~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

knotless-run: The gate test holds a pull under one runner and ejects it under another

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:run/gate-wait-held

What changes

a_merge_queue_waits_for_its_gate in crates/knotless-run/tests/run.rs checks the two ends of a gate's review wait with two runners rather than one wait it sits out:

  • Held: a runner whose review_wait is an hour passes over the queued pull, and the test always asserts the pull is not ejected.
  • Ejected: a second runner on the same state, whose wait is a second, passes once that second is up, and the pull is ejected, saying it waited 1s.

Why

With an 8-second wait, a loaded runner's first pass could come after the wait was up. The test then skipped its held check, and the lines in Runner::reviewed that hold a pull (queue.rs 762-764) ran in no test, so test's coverage gate failed whichever pull or batch ran the binary. The OAuth-keep pull's pre-flight was ejected for exactly this. The test also no longer sleeps out 8 seconds.

How it was checked

CI's test check runs the binary and the coverage gate.

Changeset: none

🤖 Generated with Claude Code

Approved

The test now holds the pull under a runner with a one-hour review_wait and always asserts it isn't ejected. It then drops that runner and passes a second one with a one-second wait on the same state file, and checks the ejection message says "1s". I read Runner::reviewed in queue.rs. The wait is measured from the pull's queue.created_at, not from anything in the runner's state, so the second runner sees the same wait start. The drop(l) before the new runner is safe because pass idles the runner first. The unconditional held assertion removes the timing race that skipped the coverage lines, and the test no longer sleeps for 8 seconds. I didn't run the test or read the CI checks. The change is test-only, so no docs or changeset are needed, and Changeset: none is in the body.

Merged as 675a294