Skip to content

Tests: let the fake sudo wait on SECONDS so one signal is logged once - #89

Merged
krishhgg merged 1 commit into
mainfrom
test/fake-sudo-seconds
Oct 12, 2026
Merged

krishhgg merged 1 commit into
mainfrom
test/fake-sudo-seconds

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

What this fixes

A flaky test failed the macOS 27 job on #85 (run 38186117753, commit d328730):

RecoveryScriptTests.swift:4026: XCTAssertEqual failed: ("3") is not equal to ("2")
RecoveryScriptTests.swift:4027: XCTAssertEqual failed: ("2") is not equal to ("1")
calls: ["sudo -n .../bin/pmset -a disablesleep 0", "sudo SIGHUP", "sudo SIGTERM", "sudo SIGHUP", "sudo SIGTERM", "sudo SIGTERM"]

testTheSupervisorOutlivesItsRunAndGroupSignalsAndHoldsTheLockUntilItReapsTheCommand sends the backstop's process group one SIGTERM and one SIGHUP. It expects the fake sudo to log each once. The fake logged both twice. The backstop was fine. The bug is in the test's fake.

Why the fake logged twice

The fake's wait loop ran $(date +%s) on every pass. Bash 3.2 starts a command substitution (the $(...) child shell) with the parent's pending traps and trap commands. The child runs pending traps before its first command. So a signal that arrives after the loop's own trap check, and before that fork, runs the trap in the child and again in the parent.

The change

  • The watchdog deadline uses bash's SECONDS counter, so the wait loop forks nothing but /bin/sleep.
  • The deadline is set before the fake publishes its pid. A test may signal the group as soon as the pid appears, and there is no longer a date for that signal to kill.
  • No assertion changed. Only the fake's text and its doc comment.

This is the same fix as #34's e52ea07 and b96f62a, copied from #34's head, so #34 merges cleanly over it.

Evidence

Local, M5 MacBook Pro, macOS 26.2 (25C56), at bb5cf2b plus this diff (the diff's hash matched before and after the runs):

Run Result
swift build --build-tests ok
The failing test alone, 5 times 5 of 5 passed
All of RecoveryScriptTests 231 tests, 0 failures

The same test also failed on this Mac in its other form, on #86's branch, which doesn't have this fix. The fake ended itself after 26 s with command.ended = watchdog, though its watchdog is 60 s. The group's SIGTERM had killed the date in deadline=$(( $(date +%s) + 60 )), so the deadline was 60 seconds after 1970. The Mac did not sleep during that run (pmset -g log). This change sets the deadline before the pid appears, with no date, so that form is gone too.

Codex (gpt-6.1-sol, xhigh, read-only) found nothing at f71900f. It reproduced the CI log's HUP, TERM, HUP, TERM, TERM pattern with a standalone bash 3.2 script, and found no other test fake with a logging trap and a command substitution in its wait loop.

CI on this PR runs main's workflow, so it tests on macOS 26 only. #85 adds the macOS 27 job.

Tests-only PR: no Mac report needed.

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge.

Summary

Changes the fake sudo watchdog to use Bash’s SECONDS instead of running date.

  • The fake sudo watchdog uses SECONDS while it waits.

No actionable issues found. Tests were not run during this review.

Reviews (1) · Last reviewed commit: "Tests: let the fake sudo wait on SECONDS..." · Reviewed by Greptile

The macOS 27 job on #85 (run 38186117753, d328730) failed
testTheSupervisorOutlivesItsRunAndGroupSignalsAndHoldsTheLockUntilItReapsTheCommand.
The test sends the backstop's process group one SIGTERM and one SIGHUP
and expects the fake sudo to log each once. It logged SIGHUP, SIGTERM,
SIGHUP, SIGTERM, then the supervisor's SIGTERM: both group signals twice.

hang_on_term's wait loop ran `$(date +%s)` in every condition. Bash 3.2
starts a command substitution with the shell's pending traps and trap
commands, and the child runs pending traps before its first command. A
signal that lands after the condition's own trap check and before that
fork runs the trap in the child and again in the shell.

The watchdog now runs on bash's SECONDS, so the wait forks nothing but
/bin/sleep, and the deadline is set before the pid is published, so a
group signal sent as soon as the pid appears finds no `date` to kill.
No assertion changed.

This is #34's e52ea07 and b96f62a, which fixed the same race there;
the text matches #34's head, so #34 merges cleanly over it.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@krishhgg
krishhgg merged commit 7c5d2ca into main Oct 12, 2026
4 checks passed
krishhgg added a commit that referenced this pull request Oct 12, 2026
Brings in #89's fake sudo change, so CI lints the fake's new text, and #85's merge.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
krishhgg added a commit to 0xRafie/Insomnia that referenced this pull request Oct 12, 2026
krishhgg#66 moved the real-Mac checklist from docs/release-validation.md to docs/releasing.md, so this PR's sleep guard read back row moves there too, under Sessions and recovery. Brings in krishhgg#85's macOS 27 CI job and krishhgg#89's test fix.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
krishhgg added a commit to D4TBot/Insomnia that referenced this pull request Oct 12, 2026
krishhgg#66 moved the real-Mac checklist from docs/release-validation.md to docs/releasing.md. This PR's two checklist changes move there: the darkening-refused item now says a macOS major other than 26 or 27, and the macOS 27 Low Power Mode item goes under Sessions and recovery. The October 10 macOS 27.0.1 results stay in release-validation.md. Brings in krishhgg#85's macOS 27 CI job and krishhgg#89's test fix.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
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.

1 participant