diff --git a/CHANGELOG.md b/CHANGELOG.md index ca8f12e1..b2e7df41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ - The docs sidebar outline now lists `h3` headings as well as `h2` — per-flag and per-pattern detail (27 headings on Command line, 24 on Common patterns, 23 on Coverage) was hidden from the "On this page" navigation - The `--parallel` unsupported-OS warning no longer claims Alpine is excluded: Alpine has been a supported parallel platform since the race conditions were fixed, the message was simply never updated +### Changed +- Progress rendering moved out of `state.sh` into `console_results.sh`: `bashunit::state::print_line` and `bashunit::state::print_tap_line` are now `bashunit::console_results::print_line` / `::print_tap_line`. Rendering from the module that owns counters and the per-test payload was a layering inversion, and it was the sole remaining reason for the `state.sh → parallel.sh` call cycle that #862 broke. `state.sh` now makes no call into the renderer, parallel, or the runner, pinned by a grep-based guard so the edge cannot return through an uncovered path. These two functions were never documented API — `docs/custom-asserts.md` points custom assertions at `bashunit::assertion_passed` / `state::add_assertions_passed`, and the documented `bashunit::print_line` is an unrelated separator helper in `globals.sh` — so no alias is kept: one in `state.sh` would have recreated the very cycle this removes (#868) + ### Removed - `bin/create-pr`, a 476-line vendored copy of [Chemaclass/create-pr](https://github.com/Chemaclass/create-pr) v0.10. Nothing referenced it — not the `Makefile` targets, the workflows, or the docs, which link the upstream project rather than this copy — and having no `.sh` extension it escaped `make sa` until #863, so it sat in the tree written in a Bash 4 style the project's own rules prohibit. Use the upstream tool (#867) diff --git a/src/benchmark.sh b/src/benchmark.sh index d13c3b26..471735c4 100644 --- a/src/benchmark.sh +++ b/src/benchmark.sh @@ -112,7 +112,7 @@ function bashunit::benchmark::run_function() { if bashunit::env::is_bench_mode_enabled; then local label="$(bashunit::helper::normalize_test_function_name "$fn_name")" local line="$label [$i/$its] ${dur_ms} ms" - bashunit::state::print_line "successful" "$line" + bashunit::console_results::print_line "successful" "$line" fi done diff --git a/src/console_results.sh b/src/console_results.sh index 5485e4a8..45348949 100644 --- a/src/console_results.sh +++ b/src/console_results.sh @@ -254,7 +254,7 @@ function bashunit::console_results::print_successful_test() { full_line="$(bashunit::str::rpad "$line" "$_BASHUNIT_CONSOLE_DURATION_OUT")" fi - bashunit::state::print_line "successful" "$full_line" + bashunit::console_results::print_line "successful" "$full_line" } ## @@ -284,7 +284,7 @@ ${_BASHUNIT_COLOR_BOLD}'%s'${_BASHUNIT_COLOR_DEFAULT}\n" \ line="$line$(bashunit::console_results::test_location_suffix)" - bashunit::state::print_line "failure" "$line" + bashunit::console_results::print_line "failure" "$line" } ## @@ -381,7 +381,7 @@ $(bashunit::console_results::render_diff "$_expected_file" "$_actual_file")" line="$line$(bashunit::console_results::test_location_suffix)" - bashunit::state::print_line "failed" "$line" + bashunit::console_results::print_line "failed" "$line" } function bashunit::console_results::print_failed_snapshot_test() { @@ -404,7 +404,7 @@ function bashunit::console_results::print_failed_snapshot_test() { "$(cat "$snapshot_file")" "$actual_content")" fi - bashunit::state::print_line "failed_snapshot" "$line" + bashunit::console_results::print_line "failed_snapshot" "$line" } ## @@ -489,7 +489,7 @@ function bashunit::console_results::print_skipped_test() { line="$line$(printf "${_BASHUNIT_COLOR_FAINT} %s${_BASHUNIT_COLOR_DEFAULT}\n" "${reason}")" fi - bashunit::state::print_line "skipped" "$line" + bashunit::console_results::print_line "skipped" "$line" } function bashunit::console_results::print_incomplete_test() { @@ -503,7 +503,7 @@ function bashunit::console_results::print_incomplete_test() { line="$line$(printf "${_BASHUNIT_COLOR_FAINT} %s${_BASHUNIT_COLOR_DEFAULT}\n" "${pending}")" fi - bashunit::state::print_line "incomplete" "$line" + bashunit::console_results::print_line "incomplete" "$line" } function bashunit::console_results::print_snapshot_test() { @@ -514,7 +514,7 @@ function bashunit::console_results::print_snapshot_test() { local line line="$(printf "${_BASHUNIT_COLOR_SNAPSHOT}✎ Snapshot${_BASHUNIT_COLOR_DEFAULT}: %s\n" "${test_name}")" - bashunit::state::print_line "snapshot" "$line" + bashunit::console_results::print_line "snapshot" "$line" } function bashunit::console_results::print_risky_test() { @@ -531,7 +531,7 @@ function bashunit::console_results::print_risky_test() { full_line="$(bashunit::str::rpad "$line" "$time_display")" fi - bashunit::state::print_line "risky" "$full_line" + bashunit::console_results::print_line "risky" "$full_line" } function bashunit::console_results::print_error_test() { @@ -556,7 +556,7 @@ function bashunit::console_results::print_error_test() { line="$line$(bashunit::console_results::test_location_suffix)" - bashunit::state::print_line "error" "$line" + bashunit::console_results::print_line "error" "$line" } ## @@ -676,3 +676,122 @@ function bashunit::console_results::print_risky_tests_and_reset() { "$(bashunit::state::get_tests_risky)" "risky test" "risky tests" fi } + +## +# Emit one progress entry for a finished test, in whichever output mode is +# active (verbose line, --simple char, or TAP). +# +# Lived in state.sh until #868. Rendering from the counter module was a layering +# inversion, and it was the sole reason for the state -> parallel call cycle that +# #862 broke; keep it here so state.sh owns counters and the payload only. +# Arguments: $1 - test type, $2 - already formatted line +## +function bashunit::console_results::print_line() { + # shellcheck disable=SC2034 + local type=$1 + local line=$2 + + ((_BASHUNIT_TOTAL_TESTS_COUNT++)) || true + + bashunit::state::add_test_output "[$type]$line" + + if bashunit::env::is_no_progress_enabled; then + return + fi + + if bashunit::env::is_tap_output_enabled; then + bashunit::console_results::print_tap_line "$type" "$line" + return + fi + + if ! bashunit::env::is_simple_output_enabled; then + printf "%s\n" "$line" + return + fi + + local char + case "$type" in + successful) char="." ;; + failure) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; + failed) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; + failed_snapshot) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; + skipped) char="${_BASHUNIT_COLOR_SKIPPED}S${_BASHUNIT_COLOR_DEFAULT}" ;; + incomplete) char="${_BASHUNIT_COLOR_INCOMPLETE}I${_BASHUNIT_COLOR_DEFAULT}" ;; + snapshot) char="${_BASHUNIT_COLOR_SNAPSHOT}N${_BASHUNIT_COLOR_DEFAULT}" ;; + risky) char="${_BASHUNIT_COLOR_RISKY}R${_BASHUNIT_COLOR_DEFAULT}" ;; + error) char="${_BASHUNIT_COLOR_FAILED}E${_BASHUNIT_COLOR_DEFAULT}" ;; + *) char="?" && bashunit::log "warning" "unknown test type '$type'" ;; + esac + + if bashunit::parallel::is_enabled; then + printf "%s" "$char" + else + if ((_BASHUNIT_TOTAL_TESTS_COUNT % 50 == 0)); then + printf "%s\n" "$char" + else + printf "%s" "$char" + fi + fi +} + +function bashunit::console_results::print_tap_line() { + local type=$1 + local line=$2 + + local clean_line + clean_line=$(printf "%s" "$line" | sed 's/\x1B\[[0-9;]*[mK]//g') + local test_name="${clean_line#*: }" + test_name="${test_name%%$'\n'*}" + # Strip trailing whitespace and duration + test_name=$(printf "%s" "$test_name" | \ + sed 's/[[:space:]]*[0-9][0-9]*m\{0,1\}[[:space:]]*[0-9.]*[ms]*[[:space:]]*$//') + + case "$type" in + successful) + printf "ok %d - %s\n" "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + ;; + failure | failed | failed_snapshot | error) + printf "not ok %d - %s\n" "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + local detail_line + printf " ---\n" + while IFS= read -r detail_line; do + detail_line=$(printf "%s" "$detail_line" | sed 's/\x1B\[[0-9;]*[mK]//g') + if [ -n "$detail_line" ] \ + && [ "$(echo "$detail_line" | "$GREP" -cF "Failed:" || true)" -eq 0 ] \ + && [ "$(echo "$detail_line" | "$GREP" -cF "Error:" || true)" -eq 0 ]; then + local trimmed="${detail_line#"${detail_line%%[![:space:]]*}"}" + printf " %s\n" "$trimmed" + fi + done <<< "$clean_line" + printf " ...\n" + ;; + skipped) + local skip_name="${test_name%% *}" + local skip_reason="${test_name#"$skip_name"}" + skip_reason="${skip_reason#"${skip_reason%%[![:space:]]*}"}" + if [ -n "$skip_reason" ]; then + printf "ok %d - %s # SKIP %s\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$skip_name" "$skip_reason" + else + printf "ok %d - %s # SKIP\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + fi + ;; + incomplete) + printf "ok %d - %s # TODO incomplete\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + ;; + snapshot) + printf "ok %d - %s # snapshot\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + ;; + risky) + printf "ok %d - %s # RISKY no assertions\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + ;; + *) + printf "not ok %d - %s\n" \ + "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" + ;; + esac +} diff --git a/src/runner.sh b/src/runner.sh index a01b5f48..a0dbf75a 100755 --- a/src/runner.sh +++ b/src/runner.sh @@ -1281,7 +1281,7 @@ function bashunit::runner::run_test() { bashunit::runner::format_subshell_output "$subshell_output" subshell_output=$_BASHUNIT_RUNNER_OUTPUT_OUT if ! bashunit::env::is_failures_only_enabled; then - bashunit::state::print_line "$type" "$subshell_output" + bashunit::console_results::print_line "$type" "$subshell_output" fi fi diff --git a/src/state.sh b/src/state.sh index c582e3e6..a1e523c0 100644 --- a/src/state.sh +++ b/src/state.sh @@ -396,112 +396,3 @@ function bashunit::state::aggregate_parallel_results() { "snapshot:$total_snapshot" } -function bashunit::state::print_line() { - # shellcheck disable=SC2034 - local type=$1 - local line=$2 - - ((_BASHUNIT_TOTAL_TESTS_COUNT++)) || true - - bashunit::state::add_test_output "[$type]$line" - - if bashunit::env::is_no_progress_enabled; then - return - fi - - if bashunit::env::is_tap_output_enabled; then - bashunit::state::print_tap_line "$type" "$line" - return - fi - - if ! bashunit::env::is_simple_output_enabled; then - printf "%s\n" "$line" - return - fi - - local char - case "$type" in - successful) char="." ;; - failure) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; - failed) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; - failed_snapshot) char="${_BASHUNIT_COLOR_FAILED}F${_BASHUNIT_COLOR_DEFAULT}" ;; - skipped) char="${_BASHUNIT_COLOR_SKIPPED}S${_BASHUNIT_COLOR_DEFAULT}" ;; - incomplete) char="${_BASHUNIT_COLOR_INCOMPLETE}I${_BASHUNIT_COLOR_DEFAULT}" ;; - snapshot) char="${_BASHUNIT_COLOR_SNAPSHOT}N${_BASHUNIT_COLOR_DEFAULT}" ;; - risky) char="${_BASHUNIT_COLOR_RISKY}R${_BASHUNIT_COLOR_DEFAULT}" ;; - error) char="${_BASHUNIT_COLOR_FAILED}E${_BASHUNIT_COLOR_DEFAULT}" ;; - *) char="?" && bashunit::log "warning" "unknown test type '$type'" ;; - esac - - if bashunit::parallel::is_enabled; then - printf "%s" "$char" - else - if ((_BASHUNIT_TOTAL_TESTS_COUNT % 50 == 0)); then - printf "%s\n" "$char" - else - printf "%s" "$char" - fi - fi -} - -function bashunit::state::print_tap_line() { - local type=$1 - local line=$2 - - local clean_line - clean_line=$(printf "%s" "$line" | sed 's/\x1B\[[0-9;]*[mK]//g') - local test_name="${clean_line#*: }" - test_name="${test_name%%$'\n'*}" - # Strip trailing whitespace and duration - test_name=$(printf "%s" "$test_name" | \ - sed 's/[[:space:]]*[0-9][0-9]*m\{0,1\}[[:space:]]*[0-9.]*[ms]*[[:space:]]*$//') - - case "$type" in - successful) - printf "ok %d - %s\n" "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - ;; - failure | failed | failed_snapshot | error) - printf "not ok %d - %s\n" "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - local detail_line - printf " ---\n" - while IFS= read -r detail_line; do - detail_line=$(printf "%s" "$detail_line" | sed 's/\x1B\[[0-9;]*[mK]//g') - if [ -n "$detail_line" ] \ - && [ "$(echo "$detail_line" | "$GREP" -cF "Failed:" || true)" -eq 0 ] \ - && [ "$(echo "$detail_line" | "$GREP" -cF "Error:" || true)" -eq 0 ]; then - local trimmed="${detail_line#"${detail_line%%[![:space:]]*}"}" - printf " %s\n" "$trimmed" - fi - done <<< "$clean_line" - printf " ...\n" - ;; - skipped) - local skip_name="${test_name%% *}" - local skip_reason="${test_name#"$skip_name"}" - skip_reason="${skip_reason#"${skip_reason%%[![:space:]]*}"}" - if [ -n "$skip_reason" ]; then - printf "ok %d - %s # SKIP %s\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$skip_name" "$skip_reason" - else - printf "ok %d - %s # SKIP\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - fi - ;; - incomplete) - printf "ok %d - %s # TODO incomplete\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - ;; - snapshot) - printf "ok %d - %s # snapshot\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - ;; - risky) - printf "ok %d - %s # RISKY no assertions\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - ;; - *) - printf "not ok %d - %s\n" \ - "$_BASHUNIT_TOTAL_TESTS_COUNT" "$test_name" - ;; - esac -} diff --git a/tests/unit/console_results_test.sh b/tests/unit/console_results_test.sh index b2cb08c1..4bc7e7d1 100644 --- a/tests/unit/console_results_test.sh +++ b/tests/unit/console_results_test.sh @@ -780,3 +780,79 @@ function test_test_location_suffix_empty_when_unset() { export _BASHUNIT_TEST_LOCATION="$original" } + +# --- print_tap_line ----------------------------------------------------------- +# Each capture runs in $(...) so mutating _BASHUNIT_TOTAL_TESTS_COUNT never +# leaks into the suite's own counters. + +function test_tap_line_successful_strips_colors_and_duration() { + local line + line="$(printf '\033[32m✓ Passed\033[0m: Adds numbers 12ms')" + + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=7 + bashunit::console_results::print_tap_line "successful" "$line" + ) + + assert_same "ok 7 - Adds numbers" "$out" +} + +function test_tap_line_failure_renders_yaml_block_without_the_header() { + local line + line=$'\033[31m✗ Failed\033[0m: Broken thing\n Expected \'a\'\n but got \'b\'' + + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=3 + bashunit::console_results::print_tap_line "failed" "$line" + ) + + local expected + expected=$'not ok 3 - Broken thing\n ---\n Expected \'a\'\n but got \'b\'\n ...' + assert_same "$expected" "$out" +} + +function test_tap_line_skipped_with_reason() { + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=2 + bashunit::console_results::print_tap_line "skipped" "↷ Skipped: Needs jq jq not installed" + ) + + assert_same "ok 2 - Needs jq # SKIP jq not installed" "$out" +} + +function test_tap_line_skipped_without_reason() { + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=2 + bashunit::console_results::print_tap_line "skipped" "↷ Skipped: No reason" + ) + + assert_same "ok 2 - No reason # SKIP" "$out" +} + +function test_tap_line_incomplete_and_snapshot_and_risky_directives() { + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=5 + bashunit::console_results::print_tap_line "incomplete" "✒ Incomplete: Pending" + bashunit::console_results::print_tap_line "snapshot" "✎ Snapshot: Rendered" + bashunit::console_results::print_tap_line "risky" "△ Risky: No asserts" + ) + + local expected + expected=$'ok 5 - Pending # TODO incomplete\nok 5 - Rendered # snapshot\nok 5 - No asserts # RISKY no assertions' + assert_same "$expected" "$out" +} + +function test_tap_line_unknown_type_is_not_ok() { + local out + out=$( + _BASHUNIT_TOTAL_TESTS_COUNT=9 + bashunit::console_results::print_tap_line "mystery" "?: Something odd" + ) + + assert_same "not ok 9 - Something odd" "$out" +} diff --git a/tests/unit/custom_assertions_test.sh b/tests/unit/custom_assertions_test.sh index c4cbc637..74b07e37 100644 --- a/tests/unit/custom_assertions_test.sh +++ b/tests/unit/custom_assertions_test.sh @@ -44,10 +44,10 @@ function test_custom_assertion_with_fail_shows_correct_test_name() { # the failure message shows the test function name, not the custom assertion name local output output="$( - # Temporarily override bashunit::state::print_line to capture output + # Temporarily override bashunit::console_results::print_line to capture output _captured_output="" # shellcheck disable=SC2317,SC2329 - bashunit::state::print_line() { + bashunit::console_results::print_line() { _captured_output="$2" echo "$_captured_output" } @@ -71,7 +71,7 @@ function test_custom_assertion_with_bashunit_assertion_failed_shows_correct_test output="$( _captured_output="" # shellcheck disable=SC2317,SC2329 - bashunit::state::print_line() { + bashunit::console_results::print_line() { _captured_output="$2" echo "$_captured_output" } @@ -94,7 +94,7 @@ function test_custom_assertion_calling_assert_same_shows_correct_test_name() { output="$( _captured_output="" # shellcheck disable=SC2317,SC2329 - bashunit::state::print_line() { + bashunit::console_results::print_line() { _captured_output="$2" echo "$_captured_output" } diff --git a/tests/unit/state_test.sh b/tests/unit/state_test.sh index 54b17e5d..080eae5f 100644 --- a/tests/unit/state_test.sh +++ b/tests/unit/state_test.sh @@ -320,78 +320,17 @@ function test_decode_base64_returns_empty_for_empty_value() { assert_same "" "$(bashunit::helper::decode_base64 "")" } -# --- print_tap_line ----------------------------------------------------------- -# Each capture runs in $(...) so mutating _BASHUNIT_TOTAL_TESTS_COUNT never -# leaks into the suite's own counters. - -function test_tap_line_successful_strips_colors_and_duration() { - local line - line="$(printf '\033[32m✓ Passed\033[0m: Adds numbers 12ms')" - - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=7 - bashunit::state::print_tap_line "successful" "$line" - ) - - assert_same "ok 7 - Adds numbers" "$out" -} - -function test_tap_line_failure_renders_yaml_block_without_the_header() { - local line - line=$'\033[31m✗ Failed\033[0m: Broken thing\n Expected \'a\'\n but got \'b\'' - - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=3 - bashunit::state::print_tap_line "failed" "$line" - ) - - local expected - expected=$'not ok 3 - Broken thing\n ---\n Expected \'a\'\n but got \'b\'\n ...' - assert_same "$expected" "$out" -} - -function test_tap_line_skipped_with_reason() { - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=2 - bashunit::state::print_tap_line "skipped" "↷ Skipped: Needs jq jq not installed" - ) - - assert_same "ok 2 - Needs jq # SKIP jq not installed" "$out" -} - -function test_tap_line_skipped_without_reason() { - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=2 - bashunit::state::print_tap_line "skipped" "↷ Skipped: No reason" - ) - - assert_same "ok 2 - No reason # SKIP" "$out" -} - -function test_tap_line_incomplete_and_snapshot_and_risky_directives() { - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=5 - bashunit::state::print_tap_line "incomplete" "✒ Incomplete: Pending" - bashunit::state::print_tap_line "snapshot" "✎ Snapshot: Rendered" - bashunit::state::print_tap_line "risky" "△ Risky: No asserts" - ) - - local expected - expected=$'ok 5 - Pending # TODO incomplete\nok 5 - Rendered # snapshot\nok 5 - No asserts # RISKY no assertions' - assert_same "$expected" "$out" -} - -function test_tap_line_unknown_type_is_not_ok() { - local out - out=$( - _BASHUNIT_TOTAL_TESTS_COUNT=9 - bashunit::state::print_tap_line "mystery" "?: Something odd" - ) - - assert_same "not ok 9 - Something odd" "$out" +# --- layering ------------------------------------------------------------------ +# state.sh owns counters and the per-test payload. It rendered progress lines +# until #868, which was both a layering inversion and the sole reason for the +# state -> parallel call cycle #862 broke. Grepped rather than exercised, so the +# edge cannot come back through a path no test happens to cover. +function test_state_does_not_call_the_renderer_or_parallel() { + # `|| true`: no match is the passing case, and grep exiting 1 would sink the + # whole pipeline under --strict's pipefail. + local offenders + offenders=$({ "$GREP" -oE "bashunit::(console_results|console_header|parallel|runner)::[a-z_]+" \ + src/state.sh || true; } | sort -u | tr '\n' ' ') + + assert_same "" "${offenders% }" }