test: refactored context type err message to regex#12596
test: refactored context type err message to regex#12596Shinobi881 wants to merge 3 commits intonodejs:masterfrom
Conversation
test/parallel/test-vm-context.js
Outdated
| /^TypeError: contextifiedSandbox argument must be an object\.$/; | ||
|
|
||
| [undefined, null, 0, 0.0, ''].forEach(function(e) { | ||
|
|
There was a problem hiding this comment.
Unnecessary whitespace here and in the same place in the second section.
cjihrig
left a comment
cjihrig
left a comment
There was a problem hiding this comment.
LGTM with a couple comments.
This should probably be generalized more to a single loop with an sandbox input value and expected error. That doesn't need to be done here though... unless you want to.
test/parallel/test-vm-context.js
Outdated
| /^TypeError: contextifiedSandbox argument must be an object\.$/; | ||
|
|
||
| [undefined, null, 0, 0.0, ''].forEach(function(e) { | ||
|
|
There was a problem hiding this comment.
Can you remove this blank line.
test/parallel/test-vm-context.js
Outdated
| /^TypeError: sandbox argument must have been converted to a context\.$/; | ||
|
|
||
| [{}, []].forEach(function(e) { | ||
|
|
There was a problem hiding this comment.
Same comment here about the blank line.
There was a problem hiding this comment.
@cjihrig that was my original thought, but i figured I'd skip the type checking and be more direct. I still have one more PR for this file and could change it then. What do you recommend?
There was a problem hiding this comment.
Instead of two separate loops, you could do the following with nested arrays:
[[undefined, nonContextualSandboxErrorMsg], etc.].forEach()
Then, you pass the first element to runInContext, and use the second element as the expected error.
bengl
added
the
code-and-learn
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in ef6a7cf |
|
Thanks for the contribution! 🎉 |
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12596 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
9 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)