-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
process: add coverage tests for sourceMapFromDataUrl method #30319
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.
these look like good tests to me 👍 did you confirm that they increase coverage?
@@ -116,6 +116,36 @@ function nextdir() { | |||
); | |||
} | |||
|
|||
{ |
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.
could we add a comment above both of these blocks that describes the purpose of the test, e.g.,
"base64 encoding error does not crash application".
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 took the liberty of adding two comments. LGTY?
33f0456
to
164fadf
Compare
@Nolikzero FWIW, GitHub can't link your account to your contributions. See https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork to fix. (Basically, you'll need to add the email in your commit to https://github.com/settings/emails.) |
PR-URL: #30319 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Landed in eb77321 Thanks for the contribution! 🎉 |
PR-URL: #30319 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com>
PR-URL: #30319 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com>
PR-URL: #30319 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes