Repository navigation
fix(e2e): re-stamp automations-04's spawn claim right before the CLI runs - #783
Merged
Merged
Conversation
…runs automations-04 plants an autostart claim (this process's pid, stamped now) and expects `automations enable` to defer to it. The CLI honours a claim for 30 seconds. On the mock lane up to 64 scenarios run at once and every step blocks a runtime worker while its `rocm` child runs, so the When can start more than 30 seconds after the Given. The CLI then reads the claim as stale and spawns a daemon, which is the correct behaviour, and the scenario fails. Re-stamp the claim the Given wrote immediately before the CLI starts. Only a claim holding the harness's own pid is touched, so a claim the CLI wrote for a daemon it spawned is left alone. With a 31-second delay injected between the steps, the scenario fails as on CI without the re-stamp and passes with it. Signed-off-by: Eugene Volen <eugene.volen@amd.com>
juhovainio
approved these changes
Oct 28, 2026
juhovainio
left a comment
juhovainio
left a comment
Collaborator
There was a problem hiding this comment.
Looks good. The re-stamp is guarded by a pid match, so it only touches the claim the Given step planted and never one the CLI wrote for a spawned daemon. Plant and refresh share one writer, so the <pid> <ms> format can't drift.
Non-blocking notes:
- The refresh lives in the shared
enable_observestep; it is a no-op for any scenario that doesn't plant a harness-pid claim (see inline). - A small window remains at CLI process startup, which is acceptable. This addresses the timing symptom, not the 64-way starvation behind it, and that tradeoff (versus raising the TTL or injecting a clock) seems reasonable for a test-only fix.
I haven't re-run the 31s-sleep repro myself; CI was still running at review time.
|
|
||
| #[when("the user enables an automation watcher in observe mode")] | ||
| async fn enable_observe(world: &mut E2eWorld) { | ||
| refresh_harness_autostart_claim(world); |
Collaborator
There was a problem hiding this comment.
Nit: this refresh runs for every scenario using this shared When step. It's safe because only automations-04 plants a harness-pid claim, but a short comment here saying so would keep that coupling visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
automations-04(@id:automations-autostart-defers-during-spawn-window) failed once on the GitHub-hostedE2E testsjob (run 40677075537). It passes when run alone. The failure is in the test's timing, not in the CLI.Cause
The Given plants an autostart claim holding the harness's own pid and the current time. The When runs
rocm automations enable, which must defer to that claim instead of spawning a second daemon. The CLI honours a claim for 30 seconds (AUTOSTART_CLAIM_TTL_MSinapps/rocm/src/service_launch.rs).On the mock lane, cucumber runs up to 64 scenarios at once on a tokio runtime with one worker per CPU, and every step blocks a worker while its
rocmchild runs (run_rocmcallsCommand::output()). On a 4-core hosted runner the When can therefore start more than 30 seconds after the Given. The CLI then reads the claim as stale and spawns, which is correct, and the scenario fails with...but it reached the spawn.Change
The When re-stamps the claim immediately before starting the CLI, so the window shrinks to process startup. It only rewrites a claim that holds this harness's pid, which only this Given writes. A claim the CLI wrote for a daemon it spawned is never touched.
Verification
Reproduced in WSL by adding a temporary 31-second sleep between the Given and the CLI call: without the re-stamp the scenario fails with the same message as CI, and with it the scenario passes.
All six
automationsscenarios pass withcargo xtask e2e -- -n automations(WSL, no sleep).cargo clippy -p e2e-cucumber --all-targets -D warningsandcargo fmt --checkpass.No new scenario is needed: this fixes the step of an existing one, and no CLI behaviour changes.
If this PR fixes a bug, searched
tests/e2e-cucumber/expectations.tomlfor the fixed ticket ID and removed/narrowed any now-stale xfail rows. (No xfail row covers this scenario.)If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per
docs/architecture.md. (N/A)Every new or changed user-facing message was read against the code path that runs after it. (No user-facing messages change.)