Close CI gaps and wait on events instead of fixed sleeps in flaky tests - #1600
Conversation
The fuzz, wasm-handler SDK and Zed extension workspaces keep their own Cargo.lock, and three of them build root crates by path, so a root version bump or dependency change can leave one stale without any job noticing. The rust-policy job now resolves each tracked non-root lock with `cargo update --workspace --locked`, which fails on a stale lock and fetches only the registry index. The gate inventory records the step. Refs #1528 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…rter The public-organizations starter is now commented YAML and its change requests name the `casework` review authority, so the ignored native_create_recovers_after_process_exit_without_duplicate_records test failed before its first write: the fixture parsed registry.yaml as JSON, and a local start refused to activate an unbound review authority. Parse the starter as YAML and bind `casework` in the test's copied dev-clients.yaml to a declared producer client and an unused loopback endpoint. No scenario in the test submits a change request, so the binding is never contacted. Refs #1405 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The breg tutorial step filtered on a test that no longer exists, and a cargo test filter that selects nothing exits 0, so the step passed without running anything. Point it at the successor, native_create_recovers_after_process_exit_without_duplicate_records, with --exact, and fail unless the run reports exactly one pass. The two casework tutorial steps that select one ignored test by name had the same hole and get the same guard. The CI classifier test now checks the exact name, the guard, and that the named test exists. Closes #1405 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3988e3f507
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ge gap The language-server protocol tests gave each message a fixed 10 second receive timeout, so a slow runner failed while the server was still answering. receive_response now waits for the named response under one 120 second deadline for the whole wait, and reports what arrived instead when the deadline passes or the server closes stdout. The evidencectl termination test's 10 second readiness and exit deadlines become the same 120 second bound. Its polling loop is unchanged. Refs #1361 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…sses The dev supervisor tests polled with short fixed windows, and one read its prerequisite's PID marker as soon as the file existed, before the child had written it, so a loaded runner failed them. Every wait now polls its condition under one generous bound, and PID markers are read only once they hold bytes. Negative cases keep their meaning: a child that should be interrupted sleeps far past the bound, and each elapsed-time check compares against the production deadline it proves, not a test-chosen margin. Only the test module changes. Refs #1359 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
3988e3f to
31a2ad1
Compare
|
Generated by Claude Code |
31a2ad1 to
8b5f8c4
Compare
The two task-approval single-test steps filtered by bare function name, which matches any test sharing that name anywhere in the crate. Filter by the full module path with --exact instead, and cover both steps in the classifier test that already proved the BReg guard's exact name. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A bare grep -q exit gave no indication which test was missing or duplicated, just a failed step. Report the test name through ::error:: so a broken guard is actionable from the job log alone. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…, not hangs child.wait() after sending exit blocked forever if the server answered shutdown but never acted on exit, hanging the whole CI job instead of failing one test. Poll try_wait under RESPONSE_DEADLINE, then kill the process and panic with a message that names what happened. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Casework's dev tests waited on a hung child for at most 60 seconds while the language-server and evidencectl process tests allow 120. No test here depends on the shorter bound; a wider one only gives a starved runner more room before a real hang is reported. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Pull Request
Summary
Fixes CI gates that could pass without checking anything, and tests that failed on slow runners because they waited a fixed time instead of for an event.
CI (
.github/,release/scripts/)ci: check every standalone Cargo lockfile with --locked(refs Refresh the stale platform fuzz Cargo.lock and add --locked checks for fuzz and SDK workspaces #1528). The fuzz, wasm-handler SDK and Zed extension workspaces keep their ownCargo.lock, and most build root crates by path, so a root change could leave one stale unnoticed.rust-policynow runscargo update --workspace --lockedon each tracked non-root lock, which fails on a stale lock and fetches only the registry index. The gate inventory records the step.ci: run the retained example recovery test by exact name(closes CI: BReg tutorial job's retained-example recovery step runs zero tests #1405). The BReg tutorial step filtered on a test that no longer exists, and a filter that matches nothing exits 0. It now names the successor with--exactand fails unless exactly one test passes. Two casework tutorial steps get the same guard.test_ci_changes.pychecks the name, the guard, and that the test exists.Tests (
crates/)test(breg): let the native create-recovery test start the current starter(refs CI: BReg tutorial job's retained-example recovery step runs zero tests #1405). The fixture parsed the now-YAMLregistry.yamlas JSON, and a start refused the starter's unboundcaseworkreview authority. The fixture parses YAML and bindscaseworkto a declared client on an unused loopback endpoint. No scenario submits a change request, so nothing contacts it.test(evidence): bound protocol and build waits by response, not message gap(refs Flaky: Evidence tooling tests with fixed 10-second deadlines fail under load (language server protocol, evidencectl production_build) #1361). The LSP protocol tests used a fixed 10 s timeout per message.receive_responsenow waits for the named response under one 120 s deadline and reports what arrived instead if it fails. The evidencectl termination test's 10 s deadlines become 120 s. Its loop is unchanged.test(casework): wait on supervisor process events, not wall-clock guesses(refs Flaky: caseworkctl dev supervisor tests time out under parallel load #1359). Waits poll their condition under one 60 s bound instead of 1–6 s windows. PID markers are read only once they hold bytes: one test read its marker as soon as the file existed, before the child wrote it. Elapsed-time assertions compare against the production deadline they prove.Only test and CI code changes.
Evidence
rust-policyjob. All four standalone locks resolved with--lockedandLocking 0 packages.check-gates-inventory.pypasses. actionlint output is the same as onmain.test_ci_changes.pypasses (119 tests). The BReg and casework tutorial jobs pass in CI with the guard, so each named test ran and reported one pass.mainit failed withError("expected value", line: 1, column: 1), then on the unbound review authority. With this change it passes locally (50.8 s) and in the tutorial job.mainbinary failed 9 of 20:interrupted_native_prerequisite_is_killed_and_reapedexceeded its 1 s window, andkilled_guard_leaves_the_supervisor_to_clean_its_exact_service_grouphit "guarded service did not start". This branch failed 0 of 20.evidence_cards_protocolpassed all 12 tests in 771 s. Themainbinary was still stuck at the 900 s cap. Indicative, not statistical.cargo fmt --checkandcargo clippy -D warningspass on the four touched crates. The touched suites pass, except two tests that fail onmaintoo when run as root (as this container does). Both pass as an unprivileged user:registry-caseworkctl config_change_keeps_the_owner_after_created_container_cannot_be_savedregistry-evidencectl --test access revoke_leaves_the_record_untouched_when_the_private_directory_cannot_be_removedNotes
rust-policy.DCO
Signed-off-bytrailer.🤖 Generated with Claude Code
https://claude.ai/code/session_011gBjYHCSpw2Hs7LXPgE4da