test: cover the CSV importer file preparation - #42082
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: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
WalkthroughA new ChangesCsvImporter unit test coverage
Priority: ⚪ Not assessed Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Merge Risk: ⚪ Minimal · up to This change adds CSV importer test coverage without changing runtime behavior. No current merge-readiness risk is identified. Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Errors were encountered while retrieving linked issues. Errors (1)
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 @@
## develop #42082 +/- ##
===========================================
+ Coverage 69.49% 69.55% +0.05%
===========================================
Files 4291 4300 +9
Lines 171574 171709 +135
Branches 31098 31101 +3
===========================================
+ Hits 119243 119430 +187
+ Misses 47154 47079 -75
- Partials 5177 5200 +23
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
CsvImporter.prepareUsingLocalFilehad no direct tests — only an E2E happy path through the UI. This adds 24 server unit tests covering archive processing, parsing, progress reporting and post-import reconciliation, with no production code changed.csv-parse,adm-zipandaddParsedContactsrun unmocked against real zip files; only Meteor-bound boundaries are stubbed. Assertions were validated by mutation testing (61 single-line mutations of the production file, all detected).Coverage —
apps/meteor/server/lib/import/csv/CsvImporter.tsBefore, the file was never loaded by any unit spec, so it did not appear in the
nycreport at all. The 2 uncovered branches are transpiler artifacts. Full mocha suite: 2494 passing, 0 failing (2470 before this PR).Issue(s)
CORE-2665
Steps to test or reproduce
Further comments
Two pre-existing issues found while testing, neither touched here nor pinned as correct by any assertion:
CsvImporterfilters__MACOSXbut not.DS_Store(unlikeSlackImporter), andcsv-parseaccepts the binary silently, sogeneral/.DS_Storeis imported as a message; andmessagesCountcounts direct-message rows that are later skipped, socount.totalcan overshoot.No changeset: tests only, nothing user-facing.
Summary by CodeRabbit