From d5c1c3b5d2066c221a82396979244519642c75a1 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Fri, 14 Aug 2026 19:48:33 +0200 Subject: [PATCH 1/2] fix(cli): name the actual cause when a bootstrap file is unusable One message covered four causes and was true of one: somedir cannot read the bootstrap file (a directory IS readable) nope.sh cannot read the bootstrap file (it is not there at all) /dev/null cannot read the bootstrap file (readable, not a regular file) unread.sh cannot read the bootstrap file (accurate) The check that rejects a directory is -f, not -r, so the message sent the reader to permissions for what is really a wrong-path mistake -- and `--env tests/` or a mistyped filename are ordinary errors. Each cause now names itself. The #1247 space-split explanation still prints on top when it applies. test_a_missing_env_file_without_a_space_stays_terse asserted the old wording; its intent is the absence of that explanation, not the phrasing, so it now asserts the accurate message and says why in a comment. Closes #1262 --- CHANGELOG.md | 1 + src/main/validate.sh | 22 ++++++++- tests/acceptance/bashunit_init_test.sh | 64 +++++++++++++++++++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3779e9b0..4579220c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - The coverage HTML report handles filenames containing `|`, `<` or `&`. Rows were joined and split on `|`, so `src/a|b.sh` truncated to `a` in the index while the coverage numbers stayed right, and filenames went into the markup unescaped — `ac.sh` was parsed as a tag and leaked into the document. The escaper now lives in `src/util/str.sh`, shared with the test report instead of duplicated (#1254) - The HTML report summary counts risky and flaky tests. A run with a risky test showed `2 total` against categories summing to 1, with nothing on the page saying where the second test went — the row was there with its own CSS class, but the summary never counted it. The console and the Markdown report both report it (#1252) - The HTML report says **why** a test failed. It listed name, status and duration only, while JUnit, JSON, TAP and Markdown all carry the message — and HTML is the format opened in a browser to find out what broke. A `Failures` section now gives each failure its name, `file:line` and message; a green run gains nothing (#1251) +- The bootstrap error names the actual cause. `cannot read the bootstrap file` covered a missing path, a directory, a device and a genuinely unreadable file alike, and was true of only the last — a directory *is* readable, since the check that rejects it is `-f`, not `-r`. Pointing `--env` at a directory or mistyping a filename now say so (#1262) - `--env` with a space in the path explains itself. The flag takes `"file arg1 arg2"` and splits on the first space, so `--env "my boot.sh"` reported `cannot read the bootstrap file: 'my'` — a path the user never typed, for a file that is right there. It now says the value was split and that `BASHUNIT_BOOTSTRAP` takes the path whole; a genuinely missing file keeps the terse message (#1247) - A `--filter` that selects nothing now explains why instead of ending on a bare `No tests found`: filters match the test **function name**, case-sensitively, while the report prints a humanized title, so feeding back the name you just read (`--filter "User login"` for `test_user_login`) silently matched nothing. The run names the test it most likely meant, resolving both the capitalisation and the spaces - `bashunit doc ` says `No assertion matches ''` instead of printing nothing, which was indistinguishable from a broken install. Mirrors the existing `--custom` wording; the exit code stays 0 because `doc` is informational (#1201) diff --git a/src/main/validate.sh b/src/main/validate.sh index 0dd6a8c0..280bfd0b 100644 --- a/src/main/validate.sh +++ b/src/main/validate.sh @@ -61,8 +61,26 @@ function bashunit::main::report_unreadable_bootstrap() { local boot_file=$1 local raw=${2-} - printf "%sError: cannot read the bootstrap file: '%s'.%s\n" \ - "$_BASHUNIT_COLOR_FAILED" "$boot_file" "$_BASHUNIT_COLOR_DEFAULT" >&2 + # Name the actual cause. All three used to report "cannot read", which is true + # of only one: a directory *is* readable -- the check that rejects it is -f, + # not -r -- and for a missing path "cannot read" understates "is not there". + # Pointing --env at a directory or mistyping a filename are ordinary mistakes, + # and one message sent the reader to permissions for both (#1262). + if [ ! -e "$boot_file" ]; then + printf "%sError: the bootstrap file does not exist: '%s'.%s\n" \ + "$_BASHUNIT_COLOR_FAILED" "$boot_file" "$_BASHUNIT_COLOR_DEFAULT" >&2 + elif [ -d "$boot_file" ]; then + printf "%sError: the bootstrap path is a directory, not a file: '%s'.%s\n" \ + "$_BASHUNIT_COLOR_FAILED" "$boot_file" "$_BASHUNIT_COLOR_DEFAULT" >&2 + elif [ ! -f "$boot_file" ]; then + # Readable, but not a regular file -- /dev/null and friends. The caller + # rejects on -f, so say that rather than blame permissions. + printf "%sError: the bootstrap path is not a regular file: '%s'.%s\n" \ + "$_BASHUNIT_COLOR_FAILED" "$boot_file" "$_BASHUNIT_COLOR_DEFAULT" >&2 + else + printf "%sError: cannot read the bootstrap file: '%s'.%s\n" \ + "$_BASHUNIT_COLOR_FAILED" "$boot_file" "$_BASHUNIT_COLOR_DEFAULT" >&2 + fi if [ "$raw" != "$boot_file" ] && [ -r "$raw" ]; then printf "%s--env splits its value on the first space to pass bootstrap arguments,%s\n" \ diff --git a/tests/acceptance/bashunit_init_test.sh b/tests/acceptance/bashunit_init_test.sh index 7e48dd7f..995b9cba 100644 --- a/tests/acceptance/bashunit_init_test.sh +++ b/tests/acceptance/bashunit_init_test.sh @@ -227,7 +227,10 @@ function test_a_missing_env_file_without_a_space_stays_terse() { popd >/dev/null assert_general_error "" "" "$ec" - assert_contains "cannot read the bootstrap file" "$output" + # The point of this test is the *absence* of the space explanation, not the + # wording of the error: since #1262 a missing file says "does not exist" + # rather than "cannot read", which was only ever true of an unreadable one. + assert_contains "bootstrap file does not exist" "$output" assert_not_contains "BASHUNIT_BOOTSTRAP" "$output" } @@ -261,3 +264,62 @@ function test_a_healthy_env_flag_file_still_loads() { assert_same 0 "$ec" assert_contains "1 passed" "$output" } + +# One message covered three different causes, and was true of only one: a +# directory *is* readable -- the check that rejects it is -f, not -r -- and for +# a missing path "cannot read" understates "is not there" (#1262). +function test_a_bootstrap_that_does_not_exist_says_so() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env nope.sh t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "does not exist" "$output" +} + +function test_a_bootstrap_that_is_a_directory_says_so() { + pushd "$TMP_DIR" >/dev/null + mkdir -p boot_dir + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env boot_dir t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "is a directory" "$output" +} + +# Readable, but not a regular file. /dev/null exists everywhere the suite runs. +function test_a_bootstrap_that_is_not_a_regular_file_says_so() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env /dev/null t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "not a regular file" "$output" +} + +# The remaining case keeps the original wording, which is accurate for it. Root +# can read a mode-000 file, so ask the kernel rather than assume. +function test_an_unreadable_bootstrap_still_says_cannot_read() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + : >unread.sh + chmod 000 unread.sh + if [ -r unread.sh ]; then + chmod 644 unread.sh + popd >/dev/null + bashunit::skip "the current user can read a mode-000 file" && return + fi + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env unread.sh t_test.sh 2>&1) || true + chmod 644 unread.sh + popd >/dev/null + + assert_contains "cannot read the bootstrap file" "$output" +} From 7137dfa8c741bc2e340bb338e3b004935dc34070 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Fri, 14 Aug 2026 20:06:51 +0200 Subject: [PATCH 2/2] test(cli): move the bootstrap-diagnosis tests to their own file The unreadable-file case can only be set up with chmod, which is a no-op for root, so it skips on the Bash 3.0 CI image and runs everywhere else. That made bashunit_init_test.sh carry two skipped tests there, and bashunit_summary_output_test.sh uses that file to demonstrate --show-skipped and asserts the wording "There was 1 skipped test:". Green locally, five Bash 3.0 jobs red. Move the four tests out so the count stays one, and leave a comment at the top of the file naming the constraint and why a conditional skip is the dangerous kind -- the failure is invisible on a dev machine. --- .../bashunit_bootstrap_diagnosis_test.sh | 83 +++++++++++++++++++ tests/acceptance/bashunit_init_test.sh | 68 ++------------- 2 files changed, 92 insertions(+), 59 deletions(-) create mode 100644 tests/acceptance/bashunit_bootstrap_diagnosis_test.sh diff --git a/tests/acceptance/bashunit_bootstrap_diagnosis_test.sh b/tests/acceptance/bashunit_bootstrap_diagnosis_test.sh new file mode 100644 index 00000000..9d49f5f5 --- /dev/null +++ b/tests/acceptance/bashunit_bootstrap_diagnosis_test.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2317 + +set -euo pipefail + +# `-e/--env/--boot` reported "cannot read the bootstrap file" for four +# different causes and it was true of one: a directory *is* readable -- the +# check that rejects it is -f, not -r -- a missing path is not there at all, +# and /dev/null is readable but not a regular file (#1262). +# +# These live apart from bashunit_init_test.sh on purpose: the unreadable case +# can only be set up with chmod, which is a no-op for root, so it skips on the +# Bash 3.0 CI image -- and bashunit_summary_output_test.sh asserts that file +# has exactly one skipped test. + +BASHUNIT_PATH="$PWD/bashunit" + +function set_up() { + TMP_DIR=$(mktemp -d) +} + +function tear_down() { + rm -rf "$TMP_DIR" +} + +# One message covered three different causes, and was true of only one: a +# directory *is* readable -- the check that rejects it is -f, not -r -- and for +# a missing path "cannot read" understates "is not there" (#1262). +function test_a_bootstrap_that_does_not_exist_says_so() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env nope.sh t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "does not exist" "$output" +} + +function test_a_bootstrap_that_is_a_directory_says_so() { + pushd "$TMP_DIR" >/dev/null + mkdir -p boot_dir + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env boot_dir t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "is a directory" "$output" +} + +# Readable, but not a regular file. /dev/null exists everywhere the suite runs. +function test_a_bootstrap_that_is_not_a_regular_file_says_so() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env /dev/null t_test.sh 2>&1) || true + popd >/dev/null + + assert_contains "not a regular file" "$output" +} + +# The remaining case keeps the original wording, which is accurate for it. Root +# can read a mode-000 file, so ask the kernel rather than assume. +function test_an_unreadable_bootstrap_still_says_cannot_read() { + pushd "$TMP_DIR" >/dev/null + printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh + : >unread.sh + chmod 000 unread.sh + if [ -r unread.sh ]; then + chmod 644 unread.sh + popd >/dev/null + bashunit::skip "the current user can read a mode-000 file" && return + fi + + local output + output=$("$BASHUNIT_PATH" --no-parallel --env unread.sh t_test.sh 2>&1) || true + chmod 644 unread.sh + popd >/dev/null + + assert_contains "cannot read the bootstrap file" "$output" +} diff --git a/tests/acceptance/bashunit_init_test.sh b/tests/acceptance/bashunit_init_test.sh index 995b9cba..fcea215e 100644 --- a/tests/acceptance/bashunit_init_test.sh +++ b/tests/acceptance/bashunit_init_test.sh @@ -1,6 +1,15 @@ #!/usr/bin/env bash # shellcheck disable=SC2317 +# This file must keep exactly ONE skipped test. bashunit_summary_output_test.sh +# runs it to demonstrate --show-skipped and asserts the wording "There was 1 +# skipped test:", so a second skip turns that into "There were 2" and fails. +# +# A conditional skip is the dangerous kind: a permission test set up with +# `chmod 000` is a no-op for root, so it runs locally and skips only on the +# Bash 3.0 CI image -- green here, five jobs red there (#1264). Put those in +# their own file; bashunit_bootstrap_diagnosis_test.sh exists for that. + set -euo pipefail BASHUNIT_PATH="$PWD/bashunit" @@ -264,62 +273,3 @@ function test_a_healthy_env_flag_file_still_loads() { assert_same 0 "$ec" assert_contains "1 passed" "$output" } - -# One message covered three different causes, and was true of only one: a -# directory *is* readable -- the check that rejects it is -f, not -r -- and for -# a missing path "cannot read" understates "is not there" (#1262). -function test_a_bootstrap_that_does_not_exist_says_so() { - pushd "$TMP_DIR" >/dev/null - printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh - - local output - output=$("$BASHUNIT_PATH" --no-parallel --env nope.sh t_test.sh 2>&1) || true - popd >/dev/null - - assert_contains "does not exist" "$output" -} - -function test_a_bootstrap_that_is_a_directory_says_so() { - pushd "$TMP_DIR" >/dev/null - mkdir -p boot_dir - printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh - - local output - output=$("$BASHUNIT_PATH" --no-parallel --env boot_dir t_test.sh 2>&1) || true - popd >/dev/null - - assert_contains "is a directory" "$output" -} - -# Readable, but not a regular file. /dev/null exists everywhere the suite runs. -function test_a_bootstrap_that_is_not_a_regular_file_says_so() { - pushd "$TMP_DIR" >/dev/null - printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh - - local output - output=$("$BASHUNIT_PATH" --no-parallel --env /dev/null t_test.sh 2>&1) || true - popd >/dev/null - - assert_contains "not a regular file" "$output" -} - -# The remaining case keeps the original wording, which is accurate for it. Root -# can read a mode-000 file, so ask the kernel rather than assume. -function test_an_unreadable_bootstrap_still_says_cannot_read() { - pushd "$TMP_DIR" >/dev/null - printf 'function test_ok() { assert_same 1 1; }\n' >t_test.sh - : >unread.sh - chmod 000 unread.sh - if [ -r unread.sh ]; then - chmod 644 unread.sh - popd >/dev/null - bashunit::skip "the current user can read a mode-000 file" && return - fi - - local output - output=$("$BASHUNIT_PATH" --no-parallel --env unread.sh t_test.sh 2>&1) || true - chmod 644 unread.sh - popd >/dev/null - - assert_contains "cannot read the bootstrap file" "$output" -}