From 438c9e5eb653521225d41258edd1a6db8655dd3d Mon Sep 17 00:00:00 2001 From: helly25 <6420169+helly25@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:32:58 +0000 Subject: [PATCH] Retain scratch artifacts for all runner failures --- CHANGELOG.md | 4 +++ README.md | 4 ++- bashtest/README.md | 17 +++++++++ bashtest/bashtest.sh | 34 +++++++++++++----- bashtest/tests/junit_test.sh | 49 +++++++++++++++++++++++++ tools/junit_test.py | 69 ++++++++++++++++++++++++++++++++++++ 6 files changed, 167 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d9b4d4..b72d71e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # 0.7.1 +* 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. + * Honor native Bazel test filters, sharding, and runner fail-fast. Validate filters and shard settings before hooks run, acknowledge shard support, keep reports local to each shard, and allow empty shards without masking an empty selection. diff --git a/README.md b/README.md index 52c67f4..c77969c 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,9 @@ Run one of the following commands to get detailed information on the actual bash The flags can be used on the `bazel run` and `bazel test` commands (the latter requiring `--test_arg=...`). Use `--keep-tmpdir=never|failure|always` to control scratch retention; the default is `failure`. -A retained path is printed at shutdown. Bazel itself may still remove a sandbox unless the test runs +Failure retention includes nonzero exits, failed cases or hooks, aborted runs, invalid runner settings, +and report-writing errors, even when the runner status is handled by its caller. `never` deletes +owned scratch data on failure; `always` retains it on success too. A retained path is printed at shutdown. Bazel itself may still remove a sandbox unless the test runs locally or with `--sandbox_debug`. ### Functionality diff --git a/bashtest/README.md b/bashtest/README.md index 6a089f6..68744bd 100644 --- a/bashtest/README.md +++ b/bashtest/README.md @@ -27,6 +27,23 @@ 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`. +## Scratch retention + +`--keep-tmpdir=failure` is the default. It preserves the runner-owned +`${BASHTEST_TMPDIR}` after a nonzero process exit, a failed case or lifecycle hook, +an aborted run, invalid runner settings, or a report-writing error. Handling a +nonzero `test_runner` return does not erase that failure for retention purposes. +Failed expectations outside the runner also retain their scratch data, including +failures recorded by waited descendant shells. Successful runs remove it. + +`--keep-tmpdir=never` removes owned scratch data even on failure; +`--keep-tmpdir=always` preserves it even on success. Cleanup preserves the original +nonzero process exit status, including signal-derived statuses. If creating the +scratch directory fails, cleanup leaves the caller's `${TEST_TMPDIR}` alone. +Only the runner-created directory is removed, and retained paths are printed to +stderr. Bazel may still delete its sandbox; use local execution or +`--sandbox_debug` when investigating retained artifacts. + ## Command failure handling `expect_success command args...` checks for exit status 0. diff --git a/bashtest/bashtest.sh b/bashtest/bashtest.sh index 21669d7..c959229 100644 --- a/bashtest/bashtest.sh +++ b/bashtest/bashtest.sh @@ -271,6 +271,8 @@ _BASHTEST_HAS_ERROR=0 _BASHTEST_SUGGEST_UPDATE=0 _BASHTEST_INIT_FAILED=0 _BASHTEST_DONE_FAILED=0 +_BASHTEST_RUNNER_FAILED=0 +_BASHTEST_OWNS_TMPDIR=0 _BASHTEST_XML_RUNNING=0 _BASHTEST_XML_STARTED=0 _BASHTEST_XML_ACTIVE= @@ -302,15 +304,21 @@ function _bashtest_cleanup () { echo >&2 "ERROR: Cannot write JUnit report: ${_BASHTEST_XML_PATH}" [[ "${status}" != 0 ]] || status=1 fi - local keep=0 - if [[ "${_BASHTEST_KEEP_TMPDIR}" == "always" ]] \ - || { [[ "${_BASHTEST_KEEP_TMPDIR}" == "failure" ]] && [[ "${_BASHTEST_NUM_FAIL}" != "0" ]]; }; then - keep=1 - fi - if [[ "${keep}" == "0" ]]; then - rm -rf "${BASHTEST_TMPDIR}" - else - echo >&2 "Preserved test scratch directory: ${BASHTEST_TMPDIR}" + # Scratch creation can itself fail. Never clean up an unset/unresolved path. + if [[ "${_BASHTEST_OWNS_TMPDIR}" == 1 ]]; then + local keep=0 + if [[ "${_BASHTEST_KEEP_TMPDIR}" == always ]]; then + keep=1 + elif [[ "${_BASHTEST_KEEP_TMPDIR}" == failure ]]; then + if [[ "${status}" != 0 || "${_BASHTEST_RUNNER_FAILED}" != 0 || "${_BASHTEST_NUM_FAIL}" != 0 || "${_BASHTEST_INIT_FAILED}" != 0 || "${_BASHTEST_DONE_FAILED}" != 0 || "${_BASHTEST_HAS_ERROR}" != 0 ]] || [[ -s "${_BASHTEST_FAILURE_FILE:-}" ]]; then + keep=1 + fi + fi + if [[ "${keep}" == 0 ]]; then + rm -rf "${BASHTEST_TMPDIR}" + else + echo >&2 "Preserved test scratch directory: ${BASHTEST_TMPDIR}" + fi fi exit "${status}" } @@ -323,6 +331,7 @@ trap 'exit 143' TERM mkdir -p "${TEST_TMPDIR:=/tmp/$(date "+%Y%m%dT%H%M%S")}" BASHTEST_TMPDIR="$(mktemp -d -p "${TEST_TMPDIR}")" declare -r BASHTEST_TMPDIR +_BASHTEST_OWNS_TMPDIR=1 # Descendant shells inherit this path, so expectation failures survive subshells. _BASHTEST_FAILURE_FILE="${BASHTEST_TMPDIR}/current-failure" : > "${_BASHTEST_FAILURE_FILE}" @@ -693,6 +702,7 @@ function test_runner() { if [[ "${_BASHTEST_STRICT_COMMANDS}" == 1 ]]; then _bashtest_strict_check_context if [[ "${_BASHTEST_STRICT_CONTEXT_OK}" == 0 ]]; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 'ERROR: --strict-commands requires a standalone test_runner call, outside conditional or status-masking contexts.' _bashtest_xml_case error test_runner "Strict command checking is disabled by a conditional runner call." _BASHTEST_XML_RUNNING=0 @@ -701,6 +711,7 @@ function test_runner() { fi fi if ! _bashtest_selection_init; then + _BASHTEST_RUNNER_FAILED=1 _bashtest_xml_case error test_runner "Invalid test selection or sharding configuration." _BASHTEST_XML_RUNNING=0 _bashtest_xml_write || return 1 @@ -771,6 +782,7 @@ function test_runner() { _bashtest_xml_case error test_runner "No tests were run." fi if ! _bashtest_xml_write; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 "ERROR: Cannot write JUnit report: ${_BASHTEST_XML_PATH}" return 1 fi @@ -780,10 +792,12 @@ function test_runner() { echo >&2 "FAIL: ${_BASHTEST_NUM_FAIL}" echo "----------" if [[ "${_BASHTEST_INIT_FAILED}" -ne "0" ]]; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 "ERROR: Test initialization failed." return 1 fi if [[ "${_BASHTEST_NUM_FAIL}" != "0" ]]; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 "ERROR: Some tests failed." if [[ "${_BASHTEST_SUGGEST_UPDATE}" -gt 0 ]]; then echo "" @@ -793,10 +807,12 @@ function test_runner() { fi return 1 elif [[ "${empty_shard}" == 0 && "${_BASHTEST_NUM_PASS}" == 0 ]]; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 "ERROR: No tests were run." return 2 fi if [[ "${_BASHTEST_DONE_FAILED}" != "0" ]]; then + _BASHTEST_RUNNER_FAILED=1 echo >&2 "ERROR: Test shutdown failed." return 1 fi diff --git a/bashtest/tests/junit_test.sh b/bashtest/tests/junit_test.sh index 04e5655..20ddba4 100755 --- a/bashtest/tests/junit_test.sh +++ b/bashtest/tests/junit_test.sh @@ -218,4 +218,53 @@ PROBE expect_output_not_contains INIT "${output}" } +# shellcheck disable=SC2317 # Functions are discovered by test_runner. +test::scratch_retention() { + local root probe status expected output scratch mode retained + root="$(test_tmpdir retention)" + cat > "${root}/probe.sh" <<'PROBE' +set -euo pipefail +# shellcheck disable=SC1090,SC1091,SC2154 +source "${mboworks_bashtest}" +printf '%s' "${BASHTEST_TMPDIR}" > "${SCRATCH_RECORD}" +printf evidence > "${BASHTEST_TMPDIR}/evidence" +test::test_init() { if [[ "${PROBE}" == setup ]]; then return 7; fi; } +test::body() { + case "${PROBE}" in + failure) expect_eq yes no ;; + abort) exit 42 ;; + report) rmdir "${REPORT_DIRECTORY}" ;; + esac +} +test::test_done() { if [[ "${PROBE}" == teardown ]]; then return 7; fi; } +test_runner +PROBE + for probe in pass failure setup teardown abort report; do + for mode in failure never always; do + status=0 + expected=1 + [[ "${probe}" != pass ]] || expected=0 + [[ "${probe}" != abort ]] || expected=42 + output="$(_junit_probe PROBE="${probe}" SCRATCH_RECORD="${root}/scratch.path" \ + REPORT_DIRECTORY="${root}/reports-${probe}-${mode}" GTEST_OUTPUT="xml:${root}/reports-${probe}-${mode}/result.xml" \ + "${BASH}" "${root}/probe.sh" --keep-tmpdir="${mode}" 2>&1)" || status="${?}" + expect_eq "${expected}" "${status}" + if [[ "${probe}" == report ]]; then + expect_output_contains 'Cannot write JUnit report' "${output}" + fi + scratch="$(cat "${root}/scratch.path")" + retained=0 + if [[ "${mode}" == always || ( "${mode}" == failure && "${expected}" != 0 ) ]]; then retained=1; fi + if [[ "${retained}" == 1 ]]; then + expect_success test -d "${scratch}" + expect_eq evidence "$(cat "${scratch}/evidence")" + expect_output_contains "Preserved test scratch directory: ${scratch}" "${output}" + else + expect_success test ! -e "${scratch}" + expect_output_not_contains 'Preserved test scratch directory:' "${output}" + fi + done + done +} + test_runner diff --git a/tools/junit_test.py b/tools/junit_test.py index 055f17d..0ab0511 100644 --- a/tools/junit_test.py +++ b/tools/junit_test.py @@ -387,6 +387,75 @@ def test_write_failure_after_passing_tests_remains_failure(self): self.assertIn(b'Cannot write JUnit report', result.stderr) self.assertFalse((directory / 'result.xml').exists()) + def test_scratch_retention_modes_and_original_exit_status(self): + record = self.root / 'scratch.path' + prefix = 'printf "%s" "$BASHTEST_TMPDIR" > "$SCRATCH_RECORD"\n' \ + 'printf evidence > "$BASHTEST_TMPDIR/evidence"\n' + for output in (True, False): + for mode in ('never', 'failure', 'always'): + for command, status in ((':', 0), ('return 7', 1), ('exit 42', 42)): + with self.subTest(output=output, mode=mode, command=command): + result = self.run_probe(prefix + f'test::body() {{ {command}; }}', + f'--keep-tmpdir={mode}', output=output, + SCRATCH_RECORD=str(record)) + self.assertEqual(result.returncode, status, result.stderr) + scratch = Path(record.read_text()) + retained = mode == 'always' or (mode == 'failure' and status != 0) + self.assertEqual(scratch.exists(), retained) + self.assertEqual(b'Preserved test scratch directory:' in result.stderr, retained) + if retained: + self.assertEqual((scratch / 'evidence').read_text(), 'evidence') + + def test_scratch_retained_for_hooks_aborts_and_invalid_selection(self): + record = self.root / 'scratch.path' + prefix = 'printf "%s" "$BASHTEST_TMPDIR" > "$SCRATCH_RECORD"\n' + for output in (True, False): + for body, args, status in ( + ('test::test_init() { return 3; }\ntest::body() { :; }', (), 1), + ('test::test_done() { return 3; }\ntest::body() { :; }', (), 1), + ('test::body() { exit 0; }', (), 1), + ('test::body() { echo "$UNDEFINED_RETENTION_PROBE"; }', (), 1), + ('test::body() { kill -TERM $$; }', (), 143), + ('test::body() { :; }', ('--test_filter=[',), 1), + ('test::body() { :; }', ('--test_filter=absent',), 2)): + with self.subTest(output=output, body=body, args=args): + result = self.run_probe(prefix + body, *args, output=output, + SCRATCH_RECORD=str(record)) + self.assertEqual(result.returncode, status, result.stderr) + self.assertTrue(Path(record.read_text()).is_dir()) + self.assertIn(b'Preserved test scratch directory:', result.stderr) + # A conditional caller can handle the runner status, but the diagnostic + # artifacts must survive even when the program ultimately exits 0. + result = self.run_probe(prefix + 'test::body() { :; }', '--test_filter=[', + output=False, runner='test_runner || :', SCRATCH_RECORD=str(record)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(Path(record.read_text()).is_dir()) + + def test_scratch_retained_for_report_write_errors(self): + record = self.root / 'scratch.path' + directory = self.root / 'report directory' + prefix = 'printf "%s" "$BASHTEST_TMPDIR" > "$SCRATCH_RECORD"\n' + result = self.run_probe(prefix + 'test::body() { rmdir "$REPORT_DIRECTORY"; }', + f'--gtest_output=xml:{directory}/result.xml', + SCRATCH_RECORD=str(record), REPORT_DIRECTORY=str(directory)) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertTrue(Path(record.read_text()).is_dir()) + self.assertIn(b'Cannot write JUnit report', result.stderr) + + def test_scratch_creation_failure_never_cleans_the_caller_path(self): + blocked = self.root / 'not a directory' + blocked.write_text('caller-owned') + caller = self.root / 'caller scratch' + caller.mkdir() + sentinel = caller / 'sentinel' + sentinel.write_text('preserve me') + result = self.run_probe('', output=False, TEST_TMPDIR=str(blocked), BASHTEST_TMPDIR=str(caller)) + self.assertNotEqual(result.returncode, 0) + self.assertNotIn(b'unbound variable', result.stderr) + self.assertNotIn(b'Preserved test scratch directory:', result.stderr) + self.assertEqual(blocked.read_text(), 'caller-owned') + self.assertEqual(sentinel.read_text(), 'preserve me') + def test_disabled_reports_and_invalid_output_fail_closed(self): for environment in ({'GTEST_OUTPUT': ''}, {'output': False}): result = self.run_probe('test::pass() { :; }', **environment)