chore(test): collect cli output in tests#2198
chore(test): collect cli output in tests#2198rubiesonthesky wants to merge 1 commit intoJoshuaKGoldberg:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2198 +/- ##
==========================================
+ Coverage 76.52% 76.59% +0.07%
==========================================
Files 168 168
Lines 7109 7131 +22
Branches 1092 1101 +9
==========================================
+ Hits 5440 5462 +22
Misses 1663 1663
Partials 6 6
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:
|
| await runMutationTest(caseDir); | ||
| await expect(actualContent).toMatchFileSnapshot(expectedFilePath); | ||
| expect(options).toMatchSnapshot("options"); | ||
| expect.assertions(3); |
There was a problem hiding this comment.
[Refactor] 😬 I really, really dislike expect.assertions. It's super brittle: relying on implementation details cross-test. If you ever rework a test, add a utility that runs multiple assertions, etc. you end up having to change a bunch of otherwise-seemingly-arbitrary numbers. Plus it's not comprehensive - if some async logic gets added to tests and the author forgets to update a number, it can fail silently or in some future tests. Spooky.
Request: remove the expect.assertions(...) calls. If there are places that would need that logic, we can always look at them individually. (are there?)
| await expect(actualContent).toMatchFileSnapshot(expectedFilePath); | ||
| expect(options).toMatchSnapshot("options"); | ||
| expect(output).toMatchSnapshot("output"); |
There was a problem hiding this comment.
[Refactor] Having multiple assertions in a test means we only get insights from one failure at a time. If assertion 1 of 3 fails, we don't know at a glance if 2 or 3 did.
My normal trick for this kind of thing is:
| await expect(actualContent).toMatchFileSnapshot(expectedFilePath); | |
| expect(options).toMatchSnapshot("options"); | |
| expect(output).toMatchSnapshot("output"); | |
| await expect({ actualContent, options, output }).toMatchFileSnapshot(expectedFilePath); |
PR Checklist
status: accepting prsOverview
Collect CLI output in integration tests.
Added mutation stats to "log" output so we can see what kind of changes there are in integration tests.
I also added
checkTestResultfunction so I didn't have to change every test to include output snapshotting. Which meant that I had to change every test. 😅 If no snapshot is reported as unneeded, then I should have changed every test successfully.