diff --git a/CHANGELOG.md b/CHANGELOG.md index b72d71e..a3113c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # 0.7.1 +* Report source locations for failed expectations, including user helpers and + subshells. Print golden-file diffs on stderr by default and capture them in JUnit + failure reports, without duplicating diffs in verbose mode. + * Retain scratch data for hook failures, aborts, invalid runner settings, and report errors under `--keep-tmpdir=failure`, including handled runner failures. Preserve original exit statuses and avoid cleanup of unresolved scratch paths. diff --git a/README.md b/README.md index c77969c..721d986 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,10 @@ Bazel's native `--test_filter` (extended regular expression), test sharding, and `--test_runner_fail_fast` are supported. See [selection and execution controls](bashtest/README.md#filtering-sharding-and-fail-fast) for filter precedence, shard assignment, and teardown behavior. +Failed expectations include their source file and line. Golden-file failures +print unified diffs on stderr and include them in JUnit reports without requiring +`--verbose`; see [failure diagnostics](bashtest/README.md#failure-diagnostics). + ### Example 1) Write a test that sources bashtest. diff --git a/bashtest/README.md b/bashtest/README.md index 68744bd..7d6a626 100644 --- a/bashtest/README.md +++ b/bashtest/README.md @@ -27,6 +27,24 @@ The macro supports directory and manifest-only runfiles on Linux, macOS, and Windows x86-64 with an automatically provisioned MSYS2 runtime. See the root [installation requirements](../README.md#installation-and-requirements) for Windows setup and resolving data dependencies with `rlocation`. +## Failure diagnostics + +Failed expectations print `source-file:line: test failure` on stderr before their +value or command diagnostics. The location is the first assertion call outside +the framework; if an assertion is inside a user-defined helper, it points into +that helper. Locations remain available for assertions in subshells and command +substitutions, and are included in JUnit failure text and captured stderr. + +`expect_files_eq` prints its unified diff on stderr whenever files differ, without +requiring `--verbose`. The complete diff is also saved as `.diff`. +JUnit reports include the diff, so CI does not need access to a sandbox path to +show what changed. Successful comparisons and successful golden updates do not +print a failure location or diff. Verbose mode does not duplicate failure diffs. + +Strict mode's unhandled-command errors continue to identify the failed command +and exit status. They do not claim a source line: Bash 3.2's `ERR` trap metadata +can point at the function declaration rather than the failing command. + ## Scratch retention `--keep-tmpdir=failure` is the default. It preserves the runner-owned diff --git a/bashtest/bashtest.sh b/bashtest/bashtest.sh index c959229..9fabbbd 100644 --- a/bashtest/bashtest.sh +++ b/bashtest/bashtest.sh @@ -62,7 +62,8 @@ An explicit runner filter overrides that default. Sharding and `--sandbox_debug`. * -u --update `` Update golden files, assuming the ``. * -v --verbose Show additional output while tests succeed or fail. - Prints all test calls and file diffs. + Includes successful assertion details. Failure + locations and golden diffs always go to stderr. Note: If long test flags are denoted with '-'s, then '_' can also be used, e.g. '--test_filter' and '--test-filter' are both long forms for '-f'. @@ -352,6 +353,20 @@ _bashtest_record_failure() { printf 'failure\n' >> "${_BASHTEST_FAILURE_FILE}" } +# Find the first caller outside this library, preserving paths with spaces and +# the source line of the assertion call rather than the runner's invocation. +_bashtest_assertion_failure() { + _bashtest_record_failure + local depth=1 + while (( depth < ${#BASH_SOURCE[@]} )); do + if [[ "${BASH_SOURCE[depth]}" != "${BASH_SOURCE[0]}" ]]; then + printf >&2 '%s:%s: test failure\n' "${BASH_SOURCE[depth]}" "${BASH_LINENO[depth-1]}" + break + fi + ((depth+=1)) + done +} + _bashtest_reset_error() { _BASHTEST_HAS_ERROR=0 : > "${_BASHTEST_FAILURE_FILE}" @@ -472,14 +487,14 @@ _bashtest_execute() { expect_exit_code() { local expected="${1:-}" actual=0 if [[ "${#}" -lt 2 || -z "${2:-}" || ! "${expected}" =~ ^(0|[1-9][0-9]{0,2})$ ]] || ((expected > 255)); then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 'Test failure: expect_exit_code requires a status in 0..255 and a command.' return 1 fi shift "${@}" || actual="${?}" if [[ "${actual}" != "${expected}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "Test failure: expected command exit ${expected}, got ${actual}: ${*}" return 1 fi @@ -898,7 +913,7 @@ expect_files_eq() { FILE_REL_OUTPUT="$(_bashtest_relative_path "${FILE_OUTPUT}")" if [[ ! -r "${FILE_GOLDEN}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: golden file readable." @@ -906,7 +921,7 @@ expect_files_eq() { return 1 fi if [[ ! -r "${FILE_RESULT}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: result file readable." @@ -917,7 +932,7 @@ expect_files_eq() { diff -u --label "a/${FILE_REL_GOLDEN}" --label "b/${FILE_REL_GOLDEN}" \ "${FILE_GOLDEN}" "${FILE_RESULT}" > "${FILE_OUTPUT}" 2>&1 || diff_status="${?}" if [[ "${diff_status}" -gt 1 ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "ERROR: diff failed (status ${diff_status}); golden file was not updated." cat >&2 "${FILE_OUTPUT}" return 1 @@ -926,7 +941,7 @@ expect_files_eq() { if [[ -n "${_BASHTEST_UPDATE_GOLDEN}" ]]; then UPDATE_FILE="${_BASHTEST_UPDATE_GOLDEN}/${FILE_REL_GOLDEN}" if [[ ! -w "${UPDATE_FILE}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: golden file is writable." @@ -942,7 +957,7 @@ expect_files_eq() { echo "Updated golden file '${FILE_REL_GOLDEN}'." return 0 else - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: golden file gets updated." @@ -951,7 +966,7 @@ expect_files_eq() { return 1 # Technically we die, but that could be an option. fi fi - _bashtest_record_failure + _bashtest_assertion_failure _BASHTEST_SUGGEST_UPDATE=1 echo >&2 "" echo >&2 "Test failure:" @@ -959,11 +974,9 @@ expect_files_eq() { echo >&2 " Golden: '${FILE_REL_GOLDEN}'" echo >&2 " Result: '${FILE_REL_RESULT}'" echo >&2 " Diff: '${FILE_REL_OUTPUT}'" - if [[ -n "${_BASHTEST_VERBOSE}" ]]; then - echo >&2 "Diff output:" - cat "${FILE_OUTPUT}" 2>&1 - echo >&2 "----------------------------------------" - fi + echo >&2 "Diff output:" + cat >&2 "${FILE_OUTPUT}" + echo >&2 "----------------------------------------" return 1 fi if [[ -n "${_BASHTEST_VERBOSE}" ]]; then @@ -995,7 +1008,7 @@ expect_eq() { TEXT_RESULT_OUT="${TEXT_RESULT_OUT:0:47}..." fi if [[ "${TEXT_GOLDEN}" != "${TEXT_RESULT}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: strings are equal:" @@ -1033,7 +1046,7 @@ expect_ne() { TEXT_RESULT_OUT="${TEXT_RESULT_OUT:0:47}..." fi if [[ "${TEXT_GOLDEN}" == "${TEXT_RESULT}" ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: strings are different:" @@ -1071,7 +1084,7 @@ expect_contains() { fi return 0 else - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: element is present in array:" @@ -1100,7 +1113,7 @@ expect_not_contains() { fi return 0 else - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: element is NOT present in array:" @@ -1143,7 +1156,7 @@ _bashtest_ere_match() { } _bashtest_invalid_regex() { - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Invalid extended regular expression: '${1}'." @@ -1170,7 +1183,7 @@ expect_matches() { _bashtest_invalid_regex "${REGEX}" return 1 fi - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: text matches regex:" @@ -1198,7 +1211,7 @@ expect_not_matches() { local regex_status=0 _bashtest_ere_match "${REGEX}" "${TEXT}" || regex_status="${?}" if [[ "${regex_status}" == 0 ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: text does NOT match regex:" @@ -1243,7 +1256,7 @@ expect_output_contains() { fi return 0 else - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: text contains substring:" @@ -1269,7 +1282,7 @@ expect_output_not_contains() { shift TEXT_OUT="$(_bashtest_ellipsize "${TEXT}")" if [[ "${TEXT}" == *"${SUBSTR}"* ]]; then - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" echo >&2 " Expected: text does NOT contain substring:" @@ -1361,7 +1374,7 @@ expect_pcre_matches() { fi return 0 fi - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" if [[ "${rc}" == "3" ]]; then @@ -1400,7 +1413,7 @@ expect_pcre_not_matches() { fi return 0 fi - _bashtest_record_failure + _bashtest_assertion_failure echo >&2 "" echo >&2 "Test failure:" if [[ "${rc}" == "3" ]]; then diff --git a/bashtest/tests/files_test.sh b/bashtest/tests/files_test.sh index d99fad2..014a538 100755 --- a/bashtest/tests/files_test.sh +++ b/bashtest/tests/files_test.sh @@ -21,7 +21,7 @@ source "${mboworks_bashtest}" # shellcheck disable=SC2317 # Functions are discovered by test_runner. test::golden_paths_and_updates() { - local root golden result + local root golden result diagnostic status=0 line root="$(test_tmpdir golden)" mkdir -p "${root}/source/data" "${root}/output with spaces" golden="${root}/source/data/golden [1].txt" @@ -31,9 +31,12 @@ test::golden_paths_and_updates() { expect_files_eq "${golden}" "${result}" printf 'new\n' > "${result}" # This verifier owns an isolated marker for intentionally failed expectations. - if (_BASHTEST_FAILURE_FILE="${root}/expected-mismatch"; expect_files_eq "${golden}" "${result}"); then - expect_eq failure success - fi + line=$((LINENO + 1)) + diagnostic="$(_BASHTEST_FAILURE_FILE="${root}/expected-mismatch"; expect_files_eq "${golden}" "${result}" 2>&1)" || status="${?}" + expect_eq 1 "${status}" + expect_output_contains "${BASH_SOURCE[0]}:${line}: test failure" "${diagnostic}" + expect_output_contains '-old' "${diagnostic}" + expect_output_contains '+new' "${diagnostic}" expect_output_contains '+new' "$(cat "${result}.diff")" # A manifest-only lookup supplies the physical source path, while the # update destination must retain the logical workspace-relative path. diff --git a/tools/junit_test.py b/tools/junit_test.py index 0ab0511..6b82a1a 100644 --- a/tools/junit_test.py +++ b/tools/junit_test.py @@ -194,6 +194,53 @@ def test_command_assertions_work_without_strict_mode(self): self.assertEqual(result.returncode, 1, result.stderr) self.assertIsNotNone(self.report()['bad'].find('failure')) + def test_failed_expectations_report_the_assertion_source_line(self): + for command in ('expect_eq yes no', 'expect_ne same same', 'expect_success false', + 'expect_exit_code 2 true', 'expect_matches "[" text', + 'expect_not_matches "[" text'): + for output in (True, False): + with self.subTest(command=command, output=output): + result = self.run_probe(f'test::body() {{\n {command}\n return 0\n}}', output=output) + self.assertEqual(result.returncode, 1, result.stderr) + script = self.root / 'probe.sh' + line = script.read_text().splitlines().index(f' {command}') + 1 + location = f'{script}:{line}: test failure' + self.assertIn(location.encode(), result.stderr) + self.assertNotIn(location.encode(), result.stdout) + if output: + self.assertIn(location, self.report()['body'].find('failure').text) + helper = self.root / 'assertion helper with spaces.sh' + helper.write_text('helper() {\n expect_eq yes no\n}\n') + result = self.run_probe('source "$HELPER"\ntest::wrapped() { helper; }', HELPER=str(helper)) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn(f'{helper}:2: test failure'.encode(), result.stderr) + self.assertIn(f'{helper}:2: test failure', self.report()['wrapped'].find('failure').text) + + def test_subshell_assertions_keep_their_source_location(self): + for command in ('(expect_eq yes no)', 'local value; value="$(expect_eq yes no)"'): + result = self.run_probe(f'test::body() {{\n {command}\n return 0\n}}') + self.assertEqual(result.returncode, 1, result.stderr) + script = self.root / 'probe.sh' + line = script.read_text().splitlines().index(f' {command}') + 1 + self.assertIn(f'{script}:{line}: test failure', self.report()['body'].find('failure').text) + + def test_golden_diff_is_in_stderr_and_xml_without_verbose(self): + golden = self.root / 'golden with spaces.txt' + actual = self.root / 'actual with spaces.txt' + golden.write_text('old & text\n') + actual.write_text('new & text\n') + for args in ((), ('--verbose',)): + result = self.run_probe('test::golden() { expect_files_eq "$GOLDEN" "$ACTUAL"; }', + *args, GOLDEN=str(golden), ACTUAL=str(actual)) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn(b'-old & text', result.stderr) + self.assertEqual(result.stderr.count(b'+new & text'), 1) + self.assertNotIn(b'+new & text', result.stdout) + case = self.report()['golden'] + self.assertIn('-old & text', case.find('failure').text) + self.assertIn('+new & text', case.find('system-err').text) + self.assertIn('+new & text', Path(str(actual) + '.diff').read_text()) + def test_native_bazel_filter_and_explicit_override(self): body = 'test::alpha() { echo alpha_body; }\ntest::beta() { echo beta_body; }' for args, expected in (((), 'alpha'), (('--test_filter=^beta$',), 'beta'),