Expanded ghost/core unit test timeouts and retries#28067
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR raises Vitest's test.testTimeout from 2000ms to 5000ms, updates inline comments about CI cold-import/isolate behavior, and changes CI reporter selection to ['default','github-actions'] (local runs remain 'dot'). Separately, the GitHub Actions job_unit-tests step is replaced with a Bash retry loop that runs Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
57b5c6f to
c305943
Compare
c305943 to
560a019
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 394-399: The retry loop prints "Unit tests attempt X failed —
retrying" even on the final (third) iteration; update the in-loop echo so it
distinguishes the last failure: inside the for loop that runs pnpm nx run-many
-t test:unit ..., check the attempt number (e.g., if attempt is less than 3) and
print "Unit tests attempt ${attempt} failed — retrying", otherwise print "Unit
tests attempt ${attempt} failed — no more retries" (or similar final-failure
wording) so logs clearly show when no further retries will occur.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 808b8af6-7997-4a4d-84e8-e08434ac53c7
📒 Files selected for processing (2)
.github/workflows/ci.ymlghost/core/vitest.config.ts
- the CI retry loop logged "attempt N failed — retrying" on every iteration, including the third, where no retry follows - the final attempt now logs a distinct "no more retries" error so the log unambiguously shows when the unit tests have given up
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #28067 +/- ##
=======================================
Coverage 73.81% 73.82%
=======================================
Files 1528 1528
Lines 129417 129417
Branches 15506 15509 +3
=======================================
+ Hits 95527 95537 +10
+ Misses 32910 32900 -10
Partials 980 980
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ghost/core's unit tests fail intermittently onmainCI. Two distinct things were found:cache-invalidationtest was a genuine timeout.testTimeoutwas 2000ms — a mocha holdover; under vitest's per-file isolation the cold-import cost makes that too tight on a loaded runner.github-actionsreporter emits no test-level annotation, so the ghost/core vitest process is exiting abnormally (a worker crash), not failing a reported test. It could not be reproduced locally (12/12 clean); it is specific to loaded CI runners.This is interim stabilization, not the root fix:
testTimeout2000ms → 5000ms — clears the timeout-flake class with no loss of coverage.retry: 2in vitest config — retries a failed test up to twice (3 attempts), absorbing transient test-level failures.dot→default—dotemits only dots, so a crash leaves no clue which file was running;defaultnames each file.github-actionsannotations are kept for clean per-failure reporting.The worker-crash root cause is expected to be addressed by the upcoming
isolate: falseinfrastructure rework, which reworks the same vitest worker/teardown code.