test: attempt to fix flaky TestFailuresBelowThreshold#830
test: attempt to fix flaky TestFailuresBelowThreshold#830benhoyt wants to merge 1 commit intocanonical:masterfrom
Conversation
Note: this doesn't actually fix the TestPrevChangeIDOnThreshold issue. Partially addresses canonical#826.
There was a problem hiding this comment.
Pull request overview
This PR adjusts check timing parameters in checkstate manager tests to reduce CI flakiness related to exec checks and threshold transitions (partially addressing #826).
Changes:
- Increased exec-check
Timeoutfrom100msto1sinTestFailuresBelowThreshold. - Increased exec-check
Timeoutto1sand changedThresholdfrom3to10inTestPrevChangeIDOnThreshold.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Name: "chk1", | ||
| Override: "replace", | ||
| Period: plan.OptionalDuration{Value: 20 * time.Millisecond}, | ||
| Timeout: plan.OptionalDuration{Value: 100 * time.Millisecond}, | ||
| Threshold: 3, | ||
| Timeout: plan.OptionalDuration{Value: time.Second}, | ||
| Threshold: 10, |
There was a problem hiding this comment.
Raising the threshold from 3 to 10 changes the behavior this test exercises (time-to-threshold) and may mask the underlying race that caused PrevChangeID to be empty. A more deterministic fix is to keep the smaller threshold and adjust the waitCheck predicate for the DOWN transition to also wait until PrevChangeID == performChangeID (or at least PrevChangeID != "") before asserting, so the test waits for the state to be fully updated instead of relying on timing.
Note: this doesn't actually fix the TestPrevChangeIDOnThreshold issue.
Partially addresses #826.