test(runtime): stop a log-flush race failing the macOS release build - #136
Conversation
TestNativeDeployTarGzExecAndStop failed the macOS release build for v0.17.4 with "captured logs missing stub output: \"\"" and cost the release its publish step, on a commit that had nothing to do with the native runtime. The stub writes its marker file and its stdout line as two separate events, so the marker wait above proves the binary ran but says nothing about whether the rotating log writer has flushed yet. This was the only assertion in the file that read once instead of waiting; its thirteen siblings all use waitFor. Reproduced locally on darwin-arm64: clean, 40 consecutive runs pass, but under CPU contention the unpatched test fails in 0.43s with exactly the CI message. With the wait it passes 30 runs under the same contention. Kept the 5s budget to match the other thirteen waits — a log flush is a fast event, and a log that genuinely never arrives should still fail.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe native deployment test now polls log output until the stub’s alive message appears. It retries log errors and missing output before asserting the collected logs. ChangesNative test log capture
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This improves test reliability without introducing production behavior changes or unresolved merge risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #136 +/- ##
=======================================
Coverage 78.50% 78.50%
=======================================
Files 18 18
Lines 3797 3797
=======================================
Hits 2981 2981
Misses 587 587
Partials 229 229 🚀 New features to boost your workflow:
|
1 participant
TestNativeDeployTarGzExecAndStopfailed the macOS release build for v0.17.4 withwhich skipped
release / publishand left the tag without a release, on a commit (#135) that touched nothing in the native runtime. Re-running the identical commit passed, so it is flaky rather than a regression — but flaky here means a release silently doesn't ship.The race. The stub writes its marker file and prints its stdout line as two separate events. The test waits for the marker, which proves the binary executed, then reads the log immediately — but the log goes through
newRotatingLogWriter, which may not have flushed yet. This was the only assertion in the file that reads once; its thirteen siblings all wrap the condition inwaitFor. The empty""in the message is the tell: not wrong content, no content yet.Reproduced, not assumed. On darwin-arm64, the same platform that failed:
That also explains why it only bites a loaded shared runner and never a developer machine.
I kept the 5s budget rather than inflating it, to match the other thirteen waits — a log flush is a fast event, and a log that genuinely never arrives should still fail the test. Being straight about the limit: under pathological starvation (eight CPU hogs on a laptop) the patched test can still time out at 5s. I did not chase that, because it is not a realistic runner condition and the observed CI failure was the read-once shape at 0.45s, which this removes.
Note the two other bare
p.Logsreads in this file, inTestNativeLogsTailAndAbsent, are correct as they are — that test writes the log withos.WriteFilebefore reading and has no running process.Summary by CodeRabbit