diff --git a/CHANGELOG.md b/CHANGELOG.md index 2176f2cc..338209dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - `--verbose` warns on Bash 3.x that coverage does not count lines run inside a subshell, so a percentage that reads lower there than on Bash 4+ explains itself (#1112) ### Changed +- 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) - `--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/docs/configuration.md b/docs/configuration.md index 82d7cdda..17010139 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -691,6 +691,11 @@ BASHUNIT_LOG_GHA=gha.log Create a report HTML file that contains information about the test results of your bashunit tests. +The page is a summary table plus one table per test file — test name, status and duration — +followed by a **Failures** section giving each failure its name, `file:line` and the assertion +message, so the artifact answers *what broke* rather than only *that something did*. A run with +no failures does not get the section. + ::: code-group ```bash [Example] BASHUNIT_REPORT_HTML=report.html diff --git a/src/reports/html.sh b/src/reports/html.sh index 41719259..264b530f 100644 --- a/src/reports/html.sh +++ b/src/reports/html.sh @@ -3,6 +3,27 @@ # HTML report writer. +## +# Escapes $1 for HTML: &, < , > and ". Multi-line input keeps its lines, which +# is what the failure messages need. +# +# awk, not ${var//&/&}: a bare `&` in a bash replacement means "the matched +# text" from 5.2 on while staying literal on 3.2, and no spelling is right +# across the supported range (#1096). The same rule applies to gsub, hence the +# escaped \\& below. One fork per call, so this is for the handful of failure +# messages -- the per-test rows are escaped in a single pass instead. +# Arguments: $1 - the text to escape +## +function bashunit::reports::__html_escape() { + printf '%s' "$1" | awk '{ + gsub(/&/, "\\&") + gsub(//, "\\>") + gsub(/"/, "\\"") + print + }' +} + function bashunit::reports::generate_report_html() { local output_file="$1" @@ -11,6 +32,11 @@ function bashunit::reports::generate_report_html() { local tests_incomplete=$(bashunit::state::get_tests_incomplete) local tests_snapshot=$(bashunit::state::get_tests_snapshot) local tests_failed=$(bashunit::state::get_tests_failed) + # Counted here because the summary otherwise showed a Total the visible + # categories could not add up to: a risky test was in the table, with its own + # CSS class, but in none of the numbers (#1252). + local tests_risky=$(bashunit::state::get_tests_risky) + local tests_flaky=$(bashunit::state::get_tests_flaky) local time=$(bashunit::clock::total_runtime_in_milliseconds) # Temporary file to store test cases by file (use mktemp for parallel safety) @@ -90,6 +116,8 @@ function bashunit::reports::generate_report_html() { echo " Incomplete" echo " Skipped" echo " Snapshot" + echo " Risky" + echo " Flaky" echo " Time (ms)" echo " " echo " " @@ -101,6 +129,8 @@ function bashunit::reports::generate_report_html() { echo " $tests_incomplete" echo " $tests_skipped" echo " $tests_snapshot" + echo " $tests_risky" + echo " $tests_flaky" echo " $time" echo " " echo " " @@ -141,6 +171,44 @@ function bashunit::reports::generate_report_html() { echo " " fi + # Why each failure failed. The report listed names and statuses only, while + # JUnit, JSON, TAP and Markdown all carry the message -- and this is the + # format people open in a browser to find out what broke (#1251). Shaped + # like the Markdown report's "Failures" section: name, file:line, message. + local any_failure=false + local j + for j in "${!_BASHUNIT_REPORTS_TEST_NAMES[@]}"; do + # `failed` is the only status that carries a message: collect.sh stores + # snapshot/incomplete/skipped/passed/risky/failed/flaky, and a runtime + # error is recorded as failed. Flaky is deliberately not here -- it + # passed, and its first-attempt message belongs in JUnit's + # , not under a heading that says Failures. + case "${_BASHUNIT_REPORTS_TEST_STATUSES[$j]:-}" in + failed) ;; + *) continue ;; + esac + + if [ "$any_failure" = false ]; then + echo "

Failures

" + any_failure=true + fi + + local f_name f_file f_line f_message + f_name=$(bashunit::reports::__html_escape "${_BASHUNIT_REPORTS_TEST_NAMES[$j]:-}") + f_file=$(bashunit::reports::__html_escape "${_BASHUNIT_REPORTS_TEST_FILES[$j]:-}") + f_line="${_BASHUNIT_REPORTS_TEST_LINES[$j]:-}" + f_message=$(bashunit::reports::__html_escape \ + "$(bashunit::reports::__strip_ansi "${_BASHUNIT_REPORTS_TEST_FAILURES[$j]:-}")") + + echo "

$f_name

" + if [ -n "$f_line" ]; then + echo "

$f_file:$f_line

" + else + echo "

$f_file

" + fi + echo "
$f_message
" + done + echo "" echo "" } >"$output_file" diff --git a/tests/acceptance/bashunit_parallel_reports_test.sh b/tests/acceptance/bashunit_parallel_reports_test.sh index 538c7b79..098b029a 100644 --- a/tests/acceptance/bashunit_parallel_reports_test.sh +++ b/tests/acceptance/bashunit_parallel_reports_test.sh @@ -35,3 +35,36 @@ function test_parallel_reports_are_not_empty() { assert_not_contains 'tests="0"' "$counts" assert_not_contains '1..0' "$counts" } + +# The HTML report's failure detail reads _BASHUNIT_REPORTS_TEST_FAILURES and +# _LINES at render time (#1251), which is the other side of the boundary this +# file exists for: those arrays are filled inside the per-test worker, so a +# section built from them is exactly the shape that came back empty in #1004. +function html_failure_detail() { # $1 = extra flags -> "

count, message count" + local dir html + dir=$(bashunit::temp_dir) + html="$dir/r.html" + + # shellcheck disable=SC2086 + NO_COLOR=1 ./bashunit --skip-env-file $1 --report-html "$html" "$FIXTURE" >/dev/null 2>&1 || true + + printf '%s %s' \ + "$("$GREP" -c '

Failures

' "$html" || true)" \ + "$("$GREP" -c 'Expected' "$html" || true)" +} + +function test_the_html_failure_section_survives_parallel() { + assert_same "$(html_failure_detail '--no-parallel')" "$(html_failure_detail '--parallel')" +} + +# The comparison above cannot stand alone: drop the section entirely and both +# modes report "0 0", so they still match and the test passes on a report that +# says nothing. Verified by mutation -- removing the section fails only this +# one. Same reason `test_parallel_reports_are_not_empty` sits beside the +# equality check above it. +function test_the_html_failure_section_is_not_empty_in_parallel() { + local detail + detail=$(html_failure_detail '--parallel') + + assert_not_same "0 0" "$detail" +} diff --git a/tests/acceptance/bashunit_report_html_escaping_test.sh b/tests/acceptance/bashunit_report_html_escaping_test.sh index fa4ee50c..15aca726 100644 --- a/tests/acceptance/bashunit_report_html_escaping_test.sh +++ b/tests/acceptance/bashunit_report_html_escaping_test.sh @@ -76,3 +76,131 @@ function test_a_plain_title_is_unchanged() { assert_contains "plain title" "$html" assert_contains 'class="passed"' "$html" } + +# The HTML report listed test names and statuses but never said why anything +# failed, while JUnit, JSON, TAP and Markdown all carry the message. It is the +# format people open in a browser to find out what broke, so it was the one +# that most needed it (#1251). +function _report_for_failing_test() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_bad() { assert_same "want" "got"; }' + printf '%s\n' 'function test_good() { assert_same 1 1; }' + } >"$WORKDIR/f_test.sh" + + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html rep.html f_test.sh >/dev/null 2>&1) || true + cat "$WORKDIR/rep.html" +} + +function test_the_report_explains_why_a_test_failed() { + local html + html="$(_report_for_failing_test)" + + assert_contains "Failures" "$html" + assert_contains "Expected" "$html" + assert_contains "want" "$html" + assert_contains "got" "$html" +} + +function test_the_failure_names_its_file_and_line() { + local html + html="$(_report_for_failing_test)" + + assert_contains "f_test.sh:2" "$html" +} + +# A green run must not grow an empty section. +function test_a_passing_run_has_no_failures_section() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_good() { assert_same 1 1; }' + } >"$WORKDIR/g_test.sh" + + local html + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html ok.html g_test.sh >/dev/null 2>&1) || true + html="$(cat "$WORKDIR/ok.html")" + + assert_not_contains "Failures" "$html" +} + +# The message is user text too, so it goes through the same escaping. +function test_a_failure_message_with_markup_is_escaped() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_markup() { assert_same "want" "got"; }' + } >"$WORKDIR/m_test.sh" + + local html + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html m.html m_test.sh >/dev/null 2>&1) || true + html="$(cat "$WORKDIR/m.html")" + + assert_not_contains "want" "$html" + assert_contains "<b>want</b>" "$html" +} + +# The summary counted Passed/Failed/Incomplete/Skipped/Snapshot but not Risky, +# so a run with a risky test showed a Total the visible categories could not +# add up to -- 2 total against 1 passed and four zeros. The row was there, with +# the `.risky` class the stylesheet defines, but nothing counted it. The console +# and the Markdown report both report it (#1252). +function test_the_summary_counts_a_risky_test() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_pass() { assert_same 1 1; }' + printf '%s\n' 'function test_risky() { echo noise; }' + } >"$WORKDIR/r_test.sh" + + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html r.html r_test.sh >/dev/null 2>&1) || true + local html + html="$(cat "$WORKDIR/r.html")" + + assert_contains "Risky" "$html" +} + +# Flaky already reconciles -- it sits inside the pass total -- but the console +# and Markdown both surface the number, so the HTML summary should too. +function test_the_summary_reports_flaky() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_pass() { assert_same 1 1; }' + } >"$WORKDIR/p_test.sh" + + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html p.html p_test.sh >/dev/null 2>&1) || true + local html + html="$(cat "$WORKDIR/p.html")" + + assert_contains "Flaky" "$html" +} + +# The invariant behind #1252: every test lands in exactly one category, so the +# categories sum to the total. Asserting the columns exist is weaker -- a column +# that is present but always renders zero passes that and fails this. +# +# Flaky is deliberately excluded from the sum: a flaky test stays inside the +# pass total, so adding it would double-count and this test would fail on a +# retry-recovered run. +function test_the_summary_categories_sum_to_the_total() { + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function test_pass() { assert_same 1 1; }' + printf '%s\n' 'function test_fail() { assert_same 1 2; }' + printf '%s\n' 'function test_skip() { bashunit::skip "why" && return; }' + printf '%s\n' 'function test_todo() { bashunit::todo "later"; }' + printf '%s\n' 'function test_risky() { echo noise; }' + } >"$WORKDIR/a_test.sh" + + (cd "$WORKDIR" && "$BASHUNIT_BIN" --no-parallel --report-html a.html a_test.sh >/dev/null 2>&1) || true + + # The summary row is the first run in the document: total, passed, + # failed, incomplete, skipped, snapshot, risky, flaky, time. + local -a cells=() + local cell + while IFS= read -r cell; do + cells[${#cells[@]}]="$cell" + done < <("$GREP" -oE '[0-9]+' "$WORKDIR/a.html" | "$GREP" -oE '[0-9]+' | head -9) + + local total="${cells[0]:-0}" + local sum=$((${cells[1]:-0} + ${cells[2]:-0} + ${cells[3]:-0} + ${cells[4]:-0} + ${cells[5]:-0} + ${cells[6]:-0})) + + assert_same "$total" "$sum" +}