Skip to content

Bats#2872

Merged
DL6ER merged 7 commits into
developmentfrom
bats
Jun 16, 2026
Merged

Bats#2872
DL6ER merged 7 commits into
developmentfrom
bats

Conversation

@yubiuser

@yubiuser yubiuser commented Apr 27, 2026

Copy link
Copy Markdown
Member

What does this PR aim to accomplish?:

This will need pi-hole/docker-base-images#156 and a new tag of the base image.

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.
  • I have read the above and my PR is ready for review. Check this box to confirm

@yubiuser yubiuser requested a review from a team as a code owner April 27, 2026 18:21
@yubiuser yubiuser mentioned this pull request Apr 27, 2026
Merged
1 task
@yubiuser

yubiuser commented Apr 27, 2026

Copy link
Copy Markdown
Member Author

Tests are expected to fail until we use a new base image. Reason is that we only install the helper libraries in run.sh when $BATS is empty, but it's not as we use the base image which had it set to /bats-core

@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

Conflicts have been resolved.

DL6ER
DL6ER reviewed May 11, 2026
View reviewed changes
Comment thread test/run.sh
@DL6ER

DL6ER commented May 11, 2026

Copy link
Copy Markdown
Member

the functions give a much nicer output if the assertion fails

So you have some examples?

@yubiuser

yubiuser commented May 11, 2026

Copy link
Copy Markdown
Member Author 
 ✗ Denied domain is blocked
   (from function `__assert_line' in file test/libs//bats-assert/src/assert_line.bash, line 252,
    from function `assert_line' in file test/libs//bats-assert/src/assert_line.bash, line 131,
    in test file test/test_suite.bats, line 69)
     `assert_line --index 0 "0.0.0.1"' failed
   
   -- line differs --
   index    : 0
   expected : 0.0.0.1
   actual   : 0.0.0.0
   --
   
   
   ═══════════════════════════════════════════════════════════════════════════════
                                 BATS TEST FAILURE DEBUG
   ═══════════════════════════════════════════════════════════════════════════════
   
      TEST DESCRIPTION:
      Denied domain is blocked
   
      COMMAND EXECUTED:
      bash -c dig denied.ftl @127.0.0.1 +short
   
      OUTPUT CAPTURED:
      0.0.0.0
   
   ═══════════════════════════════════════════════════════════════════════════════


 ✗ DNSSEC: SECURE domain is resolved
   (from function `__assert_line' in file test/libs//bats-assert/src/assert_line.bash, line 243,
    from function `assert_line' in file test/libs//bats-assert/src/assert_line.bash, line 131,
    in test file test/test_suite.bats, line 360)
     `assert_line --partial --index 3 "status: SERVFAIL"' failed
   
   -- line does not contain substring --
   index     : 3
   substring : status: SERVFAIL
   line      : ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 5000
   --
   
   
   ═══════════════════════════════════════════════════════════════════════════════
                                 BATS TEST FAILURE DEBUG
   ═══════════════════════════════════════════════════════════════════════════════
   
      TEST DESCRIPTION:
      DNSSEC: SECURE domain is resolved
   
      COMMAND EXECUTED:
      bash -c dig A a.dnssec @127.0.0.1
   
      OUTPUT CAPTURED:
   
   ; <<>> DiG 9.20.22 <<>> A a.dnssec @127.0.0.1
   ;; global options: +cmd
   ;; Got answer:
   ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 5000
   ;; flags: qr rd ra ad; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1
   
   ;; OPT PSEUDOSECTION:
   ; EDNS: version: 0, flags:; udp: 1232
   ;; QUESTION SECTION:
   ;a.dnssec.                   IN      A
   
   ;; ANSWER SECTION:
   a.dnssec.            3600    IN      A       192.168.4.1
   
   ;; Query time: 9 msec
   ;; SERVER: 127.0.0.1#53(127.0.0.1) (UDP)
   ;; WHEN: Mon May 11 20:22:57 UTC 2026
   ;; MSG SIZE  rcvd: 53
   
   ═══════════════════════════════════════════════════════════════════════════════

@github-actions

github-actions Bot commented May 11, 2026

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown

Conflicts have been resolved.

@DL6ER

DL6ER commented May 15, 2026

Copy link
Copy Markdown
Member

All CI builds failing:

[38](https://github.com/pi-hole/FTL/actions/runs/25716020768/job/75506042204?pr=2872#step:7:1841)
  Running BATS test suite...
  1..1
  not ok 1 bats-gather-tests
  # Could not find library 'bats-support' relative to test file or in BATS_LIB_PATH

The full first BATS suit isn't running at all -> no queries are generated -> python tests then fail the assertion as logical consequence.

@yubiuser

yubiuser commented May 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

This is 100% expected. From my comment above

Tests are expected to fail until we use a new base image. Reason is that we only install the helper libraries in run.sh when $BATS is empty, but it's not as we use the base image which had it set to /bats-core

Needs pi-hole/docker-base-images#156 and a fresh base image tagged.

@github-actions

github-actions Bot commented Jun 13, 2026

Copy link
Copy Markdown

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@PromoFaux

PromoFaux commented Jun 14, 2026

Copy link
Copy Markdown
Member

Needs pi-hole/docker-base-images#156 and a fresh base image tagged.

@yubiuser v2.19 base image tagged ready for use here once built.

https://github.com/pi-hole/docker-base-images/actions/runs/27511406650

yubiuser added 7 commits June 15, 2026 09:46
@yubiuser
Use bats_assert and bats_file
be7e9a0 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Add on_failure hook and remove debug logging from each test case
2f34c5f 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Use $BATS_LIB_PATH
b1022a9 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Remove left-over empty lines
de42dea 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Move on_failure_hook to separate file
47bb656 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Use build image v2.19
a95df8f 
Signed-off-by: yubiuser <github@yubiuser.dev>
@yubiuser
Adjust path
df2cca8 
Signed-off-by: yubiuser <github@yubiuser.dev>
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown

Conflicts have been resolved.

@yubiuser

yubiuser commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

Tests pass now. Ready for review.

@yubiuser yubiuser mentioned this pull request Jun 15, 2026
Open
5 tasks
DL6ER
DL6ER approved these changes Jun 16, 2026
View reviewed changes

@DL6ER DL6ER left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems clean and is definitely a nice readability win. The on_failure hook is also strictly better than the existing unconditional debug output. I will take the work on #2880 to fix it. This PR has been pushed back long enough.

Just a small comment: I had to rebuild my devcontainer for the tests to pass. I might have done that too fast, though, I cannot reconstruct the exact cause for the previous failure now. Everything is also passing locally for me now.

@DL6ER DL6ER merged commit a62e6b0 into development Jun 16, 2026
18 checks passed
@DL6ER DL6ER deleted the bats branch June 16, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DL6ER DL6ER DL6ER approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yubiuser @DL6ER @PromoFaux