Skip to content

test(adversarial-review): give the timing tests room to start their stubs - #1699

Merged
potiuk merged 2 commits into
mainfrom
fix/adversarial-review-timeout-flake
Oct 18, 2026
Merged

potiuk merged 2 commits into
mainfrom
fix/adversarial-review-timeout-flake

Conversation

@potiuk

@potiuk potiuk commented Oct 18, 2026

Copy link
Copy Markdown
Member

Three adversarial-review runner tests measure wall-clock time against bounds that left too little room for starting the Python stub reviewers.
Under load (the pre-commit hook runs the workspace pytest members in parallel) they failed intermittently and blocked unrelated commits.

  • test_one_timeout_does_not_block_the_others: the reviewer that should answer had a 1 s budget, which its own start-up could use up. It now has 5 s; the other stub still sleeps for a minute, so it still times out.
  • test_reviewers_run_in_parallel_and_keep_order: two 1.5 s reviewers had to finish under 2.8 s (3.03 s seen). Now 3 s each under 5.5 s; one after the other they take 6 s.
  • test_a_finished_answer_is_kept_when_a_helper_lingers: under 8 s with a 10 s timeout instead of under 4 s with 5 s.

Each test still tells the behaviour apart from its failure mode. Test-only change.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

🤖 Generated with Claude Code

test_one_timeout_does_not_block_the_others ran both stub reviewers with a
one-second budget, and the one meant to answer is a Python process that
has to start first. On a loaded machine (the parallel pre-commit pytest
run) start-up alone took longer, so the answering reviewer timed out too
and the test failed. Give it five seconds; the other stub still sleeps
for a minute, so the timeout case is unchanged.

Generated-by: Claude Opus 5
Two more runner tests measured wall-clock time against bounds that left
little room for starting the Python stubs, and one failed under the
parallel pre-commit pytest run (3.03 s against 2.8 s):

- the parallel run sleeps 3 s per reviewer and must finish under 5.5 s
  (6 s one after the other), not 1.5 s under 2.8 s;
- a finished answer with a lingering helper must arrive under 8 s with a
  10 s timeout, not under 4 s with 5 s.

Each test still tells the behaviour apart from its failure mode.

Generated-by: Claude Opus 5
@github-actions github-actions Bot added family:tools tools/* substrate:review Tool substrate: second-model read of a change before publishing (read-only CLIs, merged findings) labels Oct 18, 2026
@potiuk
potiuk merged commit 77e8145 into main Oct 18, 2026
10 checks passed
@potiuk
potiuk deleted the fix/adversarial-review-timeout-flake branch October 18, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

family:tools tools/* substrate:review Tool substrate: second-model read of a change before publishing (read-only CLIs, merged findings)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant