feat(reports): bring the HTML report level with the other formats

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 <filter>` says `No assertion matches '<filter>'` 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)

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

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//&/&amp;}: 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(/&/, "\\&amp;")
+    gsub(/</, "\\&lt;")
+    gsub(/>/, "\\&gt;")
+    gsub(/"/, "\\&quot;")
+    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 "        <th>Incomplete</th>"
     echo "        <th>Skipped</th>"
     echo "        <th>Snapshot</th>"
+    echo "        <th>Risky</th>"
+    echo "        <th>Flaky</th>"
     echo "        <th>Time (ms)</th>"
     echo "      </tr>"
     echo "    </thead>"
@@ -101,6 +129,8 @@ function bashunit::reports::generate_report_html() {
     echo "        <td>$tests_incomplete</td>"
     echo "        <td>$tests_skipped</td>"
     echo "        <td>$tests_snapshot</td>"
+    echo "        <td>$tests_risky</td>"
+    echo "        <td>$tests_flaky</td>"
     echo "        <td>$time</td>"
     echo "      </tr>"
     echo "    </tbody>"
@@ -141,6 +171,44 @@ function bashunit::reports::generate_report_html() {
       echo "  </table>"
     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
+      # <flakyFailure>, not under a heading that says Failures.
+      case "${_BASHUNIT_REPORTS_TEST_STATUSES[$j]:-}" in
+      failed) ;;
+      *) continue ;;
+      esac
+
+      if [ "$any_failure" = false ]; then
+        echo "  <h2>Failures</h2>"
+        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 "  <h3>$f_name</h3>"
+      if [ -n "$f_line" ]; then
+        echo "  <p><code>$f_file:$f_line</code></p>"
+      else
+        echo "  <p><code>$f_file</code></p>"
+      fi
+      echo "  <pre>$f_message</pre>"
+    done
+
     echo "</body>"
     echo "</html>"
   } >"$output_file"

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 -> "<h2> 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 '<h2>Failures</h2>' "$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"
+}

tests/acceptance/bashunit_report_html_escaping_test.sh

@@ -76,3 +76,131 @@ function test_a_plain_title_is_unchanged() {
   assert_contains "<td>plain title</td>" "$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 "<b>want</b>" "<i>got</i>"; }'
+  } >"$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 "<b>want</b>" "$html"
+  assert_contains "&lt;b&gt;want&lt;/b&gt;" "$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 "<th>Risky</th>" "$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 "<th>Flaky</th>" "$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 <td> 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 '<td>[0-9]+</td>' "$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"
+}