chore: apps converters golden spec breaking unit tests - #41756
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (7)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
**/*.spec.ts📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
apps/meteor/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (5)📚 Learning: 2026-02-24T19:22:48.358ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-03-06T18:10:15.268ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
🔇 Additional comments (3)
WalkthroughThe golden test now loads ChangesUploads converter golden test
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 @@
## release-9.0.0 #41756 +/- ##
=================================================
- Coverage 68.84% 68.82% -0.02%
=================================================
Files 4134 4133 -1
Lines 158195 158161 -34
Branches 28021 28042 +21
=================================================
- Hits 108906 108858 -48
- Misses 44145 44155 +10
- Partials 5144 5148 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
e9d0252 to
91909a7
Compare
Proposed changes (including videos or screenshots)
Unit tests have been failing on
release-9.0.0since the develop merge on 21 Jul.Problem. A spec from develop imports the apps uploads converter → codec →
getURL→server/settings, which ends in a top-level await that mocha can't transpile to CJS. Mocha resolves every spec before running any, so this aborts the whole stage: all 2159 tests go dark rather than red.Why CI was green on both sides. Neither half breaks alone. #41054 (into
release-9.0.0, 30 Jun) added thegetURLimport to the codec, before the spec existed. #41400 (intodevelop, 17 Jul) added the spec, and develop has no/ufsremoval so its codec has nogetURLimport. They first met in the 21 Jul merge, and since they don't overlap textually there was no conflict to report. The top-level await itself predates all of this by years — it was just never reachable from a mocha spec.Fix. Load the converter through
proxyquirewithgetURLstubbed, and update the uploads expectations, which still asserted theurl/pathpass-through that #41054 intentionally removed.Test-only, no production code touched.
Issue(s)
Regression from the interaction of #41054 and #41400. No tracking issue.
Steps to test or reproduce
Run
yarn testunitfromapps/meteor— passes on this branch, fails onrelease-9.0.0.Further comments
The top-level await stays, so any future unit test importing into
server/settingsbreaks the stage the same way — aborting the run rather than reporting a failure. Worth a follow-up ondevelop.No changeset: this PR changes no behaviour.
Summary by CodeRabbit