chadtmiller.com/knotless

Switch repository

Loading…

What changes

Two tests in crates/knotless-serve/tests/forge.rs wait for the appview to hear of a record before reading it as another account:

• assignees_and_reviews_maintainers_ask_for: the author's ask-review answers who is asked now, read through the appview. It waits until show lists the owner's ask before asserting that the answer includes it.
• maintainers_dismiss_reviews: after the gate reviews again, merge --check sees the new review only once the appview has it. The test now waits for it, as it already did for the first review.

Why

Both failed on main at 1b7d479, on both tries, over the spaces PDS: one expected two reviewers and got one, and the other found the pull not blocked. The same tree passed on the pull's own run, on a quieter host. Neither is a fault in the code: a record another account wrote a moment ago is not read yet. Until test passes on main, nothing deploys, and every batch built on it fails the same way.

How it was checked

Left to CI: the two tests run in test's forge binary on both nets.

🤖 Generated with Claude Code

This adds two wait_until calls to forge.rs so the tests wait for the appview to hear a write before reading it as another account.

• assignees_and_reviews_maintainers_ask_for: it now polls the author's show for \nreview requested: {theirs}\n before the author's ask-review. That matches the line cli.rs prints, and it is the same pattern the nearby assignees: wait already uses. At that point the owner's ask is the only one, so the exact-line match is safe.
• maintainers_dismiss_reviews: assert!(blocked(&owner)) becomes wait_until(|| blocked(&owner)). It is the same fix as the one the test already uses after the first review.

I read the diff and the surrounding test code but did not run the tests. CI is the only check, as the body says. The waits are bounded, so a real failure would still show as a timeout panic.

Round 2. The earlier approval was at cf51e35. The head now adds a "retry the read" commit and a merge of main, and the diff has changed shape since that review:

• assignees_and_reviews_maintainers_ask_for: the author's ask-review is now retried until its answer equals {mine}\n{theirs}. Asking again for a reviewer already asked changes nothing, so retrying is safe, and the wait is bounded. This is simpler than the earlier show poll.
• maintainers_dismiss_reviews: the test waits for , dismissed\n in show before asserting on it, and the blocked(&owner) assertion becomes a wait. Both read another account's write through the appview.

I read the diff and the wait_until helper but did not run the tests, so CI is the check. One thing to watch: the final assert!(!blocked(&owner)) after the collaborator's dismissal still reads another account's write without a wait. It could flake the same way, though it's outside what this pull claims to fix.