fix(ci): stabilize recent dev smoke flakes#930
Conversation
|
Warning Review limit reached
More reviews will be available in 59 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR hardens test reliability by adding robust process termination logic to the Electron smoke test script and configuring a Windows-specific longer timeout for a flaky prompt-effect live test. Both changes address test stability on different execution environments. ChangesTest stability across platforms
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Code Review
This pull request introduces a closeApp helper function in the electron smoke test script to handle application teardown more robustly, and increases a test timeout in prompt-effect.test.ts. The review identifies two issues in the closeApp implementation: uncleared timeouts that can keep the Node.js event loop active, and an unconditional wait on an already exited process that causes a 5-second hang. A code suggestion is provided to resolve these issues.
49e97ed to
c2ddd87
Compare
c2ddd87 to
8cf5a57
Compare
|
Temporarily closing and reopening to retrigger required PR workflows after GitHub did not create check suites for the latest synchronize events. |
67abbef to
8cf5a57
Compare
Summary
Fixes two recent merged
devCI flakes without changing product behavior:No issue exists; this came from live investigation of the latest merged
devCI failures.Why
Recent merged
devruns showed two separate failure shapes:desktop-smokeon2a6d53dcompleted the report-problem assertions and printed a ready result, then stayed alive until GitHub cancelled the job at 30 minutes.windows-advisoryon9d1b7a8failed only becauseloop gate records same-step repeated tool errors without block or stoptimed out at 10.016s on Windows.Related Issue
None.
Human Review Status
Pending
Review Focus
Please check that the smoke-script shutdown fallback is scoped to test cleanup only, and that the timeout increase stays limited to the known slow Windows live test.
Risk Notes
No product runtime behavior changes. Platform impact considered: this touches macOS Electron smoke cleanup and Windows advisory test timing only. Manual
workflow_dispatchforwindows-advisorywas attempted after the review cleanup, but GitHub returned HTTP 500 for Actions dispatch creation; the Windows-specific path remains covered by the platform-gated test change and will be rechecked on the next dispatchable Windows advisory run.How To Verify
Screenshots or Recordings
Not applicable; no visible UI changes.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Bug Fixes
Tests