Avoid SIGPIPE in output pattern checks - #370
Conversation
| test_excerpt "$output" | ||
| fail=$((fail + 1)) | ||
| elif printf "%s\n" "$output" | grep -qE "$pattern"; then | ||
| elif grep -qE "$pattern" <<< "$output"; then |
There was a problem hiding this comment.
Six identical copies of this pipeline survive outside this file, all under set -euo pipefail: tests/test-matrix.sh:565,633 and tests/test-static-bins.sh:93,127,161,358. They lose a matching pattern on large output exactly the way these two did, and the new lane does not reach them. Apply the same here-string to all six.
There was a problem hiding this comment.
Applied the here-string change to all six checks in 6a85186.
| if [ "${1:-}" = "--emit" ]; then | ||
| awk 'BEGIN { | ||
| print "match-begin" | ||
| for (i = 0; i < 30000; i++) |
There was a problem hiding this comment.
30000 is load-bearing: it is what pushes the emitted output past the pipe buffer so the pre-fix writer takes SIGPIPE. Nothing here says so, so a later trim to something that fits in 64 KB leaves this lane green against the bug it exists to catch. State the constraint in one line above the loop.
There was a problem hiding this comment.
Added the pipe-capacity constraint above the loop in d1275e0.
| else | ||
| run_check test-runner.sh "$pattern" --emit "$rc" | ||
| fi | ||
| [ "$pass" -eq "$want" ] && [ "$fail" -eq "$((1 - want))" ] |
There was a problem hiding this comment.
This reads the counters from the sourced lib and only sees a zero baseline because the command substitution gives each call its own subshell. Drop that subshell later and every assertion after the first compares accumulated totals instead. Name the dependency in one line so the next reader does not refactor it away.
There was a problem hiding this comment.
Documented the subshell isolation of the pass/fail counters in d1275e0.
jserv
left a comment
There was a problem hiding this comment.
Rebase latest main branch, resolve conflicts, and response to review messages.
run_check and run_pipe can reject matching output when grep exits before printf finishes writing. Under pipefail, the resulting SIGPIPE makes a successful command appear to have missed its expected pattern. Pass captured output through a here-string. Cover early and late matches, missing patterns, and nonzero command exits in make check.
The output must exceed pipe capacity for early grep exit to expose SIGPIPE. Each assertion also relies on a command-substitution subshell inheriting zero pass/fail counts without updating the parent shell.
Six captured-output checks still use echo piped to grep -q. An early match can leave echo writing to a closed pipe and report a false failure under pipefail. Use here-strings while retaining diff's expected exit status of 1. Extracted before/after paths reproduce all six early-match failures and confirm the fixes on Bash 3.2 and 5.2. Late matches, missing patterns, timeouts, and the matrix and diff exit-status checks retain their expected results. The shared runner tests and make check-format pass.
992650d to
6a85186
Compare
|
Rebased onto b54747a and resolved the conflict in mk/tests.mk. |
|
Thank @Suzu1Dev for contributing! |
A successful command with a match near the start of a large output can fail
run_check or run_pipe: grep exits early, printf receives SIGPIPE, and pipefail
reports a failed pattern check. Feed captured output through a here-string.
The regression emits a matching first line followed by 30,000 lines. Both
early-match cases fail before the fix; all eight cases pass afterward. Late
matches, missing patterns and nonzero command exits are also covered.
Validated on Apple M3 Pro, macOS 27.0 (26A5425a), SDK 26.5:
separate rerun after parallel builds stopped passes. The baseline
AArch64 matrix reports 279 pass, 0 fail, 10 skip.
Summary by cubic
Fixes flaky failures in
run_check,run_pipe, and the matrix and static-bins pattern checks when a successful command matched early in large output:grepexited before the pipe finished writing, SIGPIPE surfaced underpipefail, and the check was reported as failed. All captured-output pattern checks now read from a here-string instead of piping throughecho/printf.grep -qEchecks withgrep -qE "$pattern" <<< "$output"intests/lib/test-runner.sh,tests/test-matrix.sh, andtests/test-static-bins.sh, preservingdiff's expected exit status of 1.tests/test-runner.shcovering early matches, late matches, missing patterns, and nonzero command exits, and wires it intomake checkviamk/tests.mk; the generated output exceeds pipe capacity so the early-exit path is actually exercised.Written for commit 6a85186. Summary will update on new commits.