fix(cmd/XDC,internal/cmdtest): stabilize flaky unlock timeout#2236
fix(cmd/XDC,internal/cmdtest): stabilize flaky unlock timeout#2236gzliudan wants to merge 1 commit intoXinFinOrg:dev-upgradefrom
Conversation
Make cmdtest kill timeout configurable per test while keeping 30s default.\nSet 90s timeout for TestUnlockFlagAmbiguousWrongPassword to avoid intermittent startup-related timeout failures.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Stabilizes a flaky CLI unlock test by making the cmdtest child-process kill timeout configurable per test (defaulting to the historical 30s), and increasing the timeout for the known flaky case.
Changes:
- Add a
KillTimeoutfield tointernal/cmdtest.TestCmdand use it in the internal timeout-kill helper (default 30s). - Set
KillTimeout = 90sforTestUnlockFlagAmbiguousWrongPasswordto reduce intermittent startup-related timeouts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/cmdtest/test_cmd.go | Introduces per-test configurable kill timeout for expect/wait helpers, preserving 30s default. |
| cmd/XDC/accountcmd_test.go | Applies a longer kill timeout to the flaky ambiguous-unlock wrong-password test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stdin io.WriteCloser | ||
| stderr *testlogger | ||
| // KillTimeout controls how long expect/wait helpers wait before killing child. | ||
| // Zero value falls back to the historical default. |
There was a problem hiding this comment.
The doc comment says only the zero value falls back to the default, but withKillTimeout treats any non-positive duration (<= 0) as default. Either update the comment to match the behavior (non-positive) or change the condition to == 0 so negative durations retain their natural semantics (immediate timeout).
| // Zero value falls back to the historical default. | |
| // Non-positive values (<= 0) fall back to the historical default. |
Proposed changes
Make cmdtest kill timeout configurable per test while keeping 30s default.\nSet 90s timeout for TestUnlockFlagAmbiguousWrongPassword to avoid intermittent startup-related timeout failures.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that