-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Update node prefix tests #287
Conversation
Codecov Report
@@ Coverage Diff @@
## main #287 +/- ##
==========================================
- Coverage 80.34% 80.28% -0.07%
==========================================
Files 13 13
Lines 1486 1486
Branches 555 555
==========================================
- Hits 1194 1193 -1
Misses 122 122
- Partials 170 171 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
"package.json", | ||
"test/unit/asset-fs-array-expr-node-require/asset1.txt", | ||
"test/unit/asset-fs-array-expr-node-require/input.js" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the output.js file was necessary for each test fixture 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still necessary just was renamed https://github.com/vercel/nft/blob/main/test/unit/asset-fs-array-expr-node-prefix/output.js guess it shows as a delete because the content changed more than git expects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, git didn't track it as a rename since the contents changed too much 👍
Ensures we have the test with the
node:
prefix and without, also renames thenode
prefix fixtures to make it more clear that's what is being tested.x-ref: #285 (comment)