test: minor fixes to test-module-loading.js#12728
test: minor fixes to test-module-loading.js#12728ywalterh wants to merge 1 commit intonodejs:masterfrom
Conversation
There was a problem hiding this comment.
I see this was like this before, but is this supposed to be a comment? It will never fail as-is.
There was a problem hiding this comment.
That should probably be in test-assert.js and inside an assert.doesNotThrow(). Especially since this is a first-time contribution, I'm OK leaving it. Maybe we can open an issue for it and label it good first contribution for someone else to fix. :-D
There was a problem hiding this comment.
(Or, alternatively, I'd be OK with the line simply being removed from this test.)
There was a problem hiding this comment.
Sorry for the late response. I got confused with it a little bit too. I agree with Trott this is not gaining coverage for testing module-loading. Putting it in comment will be an example of how to use strictEqual. I'll remove it then.
There was a problem hiding this comment.
The commit has been updated to remove the unnecessary assertion. I also updated the title of the commit to be in compliance with other PRs.
Use block scope for local variables only used in a small code block. Fixed inverse arguments in some usages of Assert.strictEqual.
ywalterh
changed the title
|
@Trott Hi Rich, I've updated the commit associated with the pull request. Is there any steps I missed for this PR to go through? Again thank you for you help! |
Use block scope for local variables only used in a small code block. Fixed inverse arguments in some usages of Assert.strictEqual. PR-URL: nodejs#12728 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 133fb0c. |
Use block scope for local variables only used in a small code block. Fixed inverse arguments in some usages of Assert.strictEqual. PR-URL: nodejs#12728 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Should this be backported to |
6 participants
Use block scope for local variables only used in a small code block.
Fixed inverse arguments in some usages of Assert.strictEqual.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)