-
Notifications
You must be signed in to change notification settings - Fork 5.9k
feat(testing): add unit tests for constants #2701
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
Conversation
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.
Looks good! Just curious about the disappearing comment, mostly.
"wtfnode": "^0.8.4", | ||
"typescript": "^4.1.3" | ||
"typescript": "^4.1.3", | ||
"wtfnode": "^0.8.4" |
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 wonder if this was a manual change or something done automatically? Not particularly an issue, just curious 👀
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 think that happens when you add a new dependency to the top? Not sure, I did see it in one of Asher's PRs
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.
Yeah yarn
resorts the dependencies. 😄
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.
The real question is how it got out of order in the first place lol
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.
LOL I have no idea - could have been me 😂
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.
Haha idk I say we just blame yarn
.
// Things to mock | ||
// logger | ||
// location | ||
// document |
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.
Comment disappeared! no other changes in this file; curious about the change
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.
That was intentional! See commit: 164d11e
feat(testing): add unit tests for src/common/http
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.
LGTM!
This PR refactors a few lines of code in
src/node/constants.ts
and adds a few unit tests to bring code coverage for the file to 100%.Changes
constants
and addgetPackageJson
function to make it easier to test@schemastore/package
to devDeps to get PackageJson typeScreenshot
It increases our code coverage (Lines) from 49.53% to 50.09% (↑ 0.56%).

🟩 100% Line coverage (last 100) for
src/node/constants.ts
TODOS