Skip to content

Teardown must not decide a test's verdict - #792

Merged
xroche merged 9 commits into
masterfrom
fix/teardown-status-773
Jul 27, 2026
Merged

Teardown must not decide a test's verdict#792
xroche merged 9 commits into
masterfrom
fix/teardown-status-773

Conversation

@xroche

@xroche xroche commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Under set -e a failing command in an EXIT trap becomes the script's exit status, so a hiccup while tearing down fixtures fails a test whose assertions all passed. That is what turned 57_local-proxy-connect.test red on the Windows x64 leg of #765: five OK lines, no FAIL, exit 1. Every EXIT trap in the suite now runs teardown with errexit off, and the signal traps keep their own trap line, since sharing set +e with HUP/INT/QUIT/PIPE/TERM would leave errexit off for the rest of a signalled run and let a torn-down test still report success.

A || true on the rm would have been smaller, but it throws away the only diagnostic, and the evidence does not say which teardown command failed: a blocked rm -rf on the Windows runner exits 1 and prints "Device or resource busy", while the log shows exit 1 and nothing at all. The sharing violation in the issue is the plausible mechanism rather than a confirmed one, so whatever it really is now prints its own error.

99_teardown-status.test pins the semantics both ways and scans the suite so a new test cannot reintroduce the shape, the leaky combined trap included. The return 0 that three cleanup() bodies ended with never protected anything, since errexit fires at the failing command before it is reached.

Closes #773

xroche and others added 9 commits July 27, 2026 10:39
Under set -e a failing command in an EXIT trap becomes the script's exit
status, so a cleanup hiccup fails a test whose assertions all passed. Run
teardown with errexit off, on EXIT only: sharing set +e with the signal
traps would leave errexit off for the rest of a signalled run, where a
torn-down test could still report success. A failing teardown command
still prints its own diagnostic, so nothing is silenced.

The return 0 that three cleanup() bodies ended with never protected
anything: errexit fires at the failing command before it is reached.

Closes #773

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <xroche@gmail.com>
…-773

Signed-off-by: Xavier Roche <xroche@gmail.com>

# Conflicts:
#	tests/Makefile.am
Numbers 99-101 were taken on master while this branch was open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <xroche@gmail.com>
…-773

Signed-off-by: Xavier Roche <xroche@gmail.com>

# Conflicts:
#	tests/Makefile.am
…-773

Signed-off-by: Xavier Roche <xroche@gmail.com>

# Conflicts:
#	tests/Makefile.am
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <xroche@gmail.com>
…-773

Signed-off-by: Xavier Roche <xroche@gmail.com>

# Conflicts:
#	tests/Makefile.am
…-773

Signed-off-by: Xavier Roche <xroche@gmail.com>

# Conflicts:
#	tests/Makefile.am
@xroche
xroche merged commit 83079b7 into master Jul 27, 2026
22 checks passed
@xroche
xroche deleted the fix/teardown-status-773 branch July 27, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A Windows sharing violation in a test's cleanup trap turns a passing test red

1 participant