test: add regex to assert in zlib-write-after-close#11482
Uh oh!
There was an error while loading. Please reload this page.
Conversation
…write-after-close.js
Trott commented Feb 21, 2017
Trott commented Feb 21, 2017
Nit: The first line of the commit message should be no more than 50 chars. However, whoever lands this can fix that. Feel free to fix it yourself if you'd like to save them a few keystrokes. Either way. |
SgiobairOg changed the title 
cjihrig left a comment
cjihrig left a comment There was a problem hiding this comment.
LGTM with one nit. This can be a single line change. Can you just add the regular expression without moving around the rest of the code.
SgiobairOg commented Feb 22, 2017
I can make that change @cjihrig. Would it be worthwhile to change to the fat arrow syntax used in the docs while I'm in there? What is the procedure once the change is made? New pull request? |
jasnell left a comment
jasnell left a comment There was a problem hiding this comment.
LGTM with a suggestion. Feel free to ignore, however.
| function(){ | ||
| unzip.write(out); | ||
| }, | ||
| /^Error:zlibbindingclosed$/ |
There was a problem hiding this comment.
this could be updated to use the new common.expectsError()
Trott commented Feb 22, 2017
@SgiobairOg If you want to make the change suggested by @cjihrig, make it on this branch and push it there. That will update this PR. No need to open a new PR (and in fact, please don't!). |
SgiobairOg commented Feb 23, 2017
I'm new to Jenkins. I can't seem to figure out what the actual errors are. How do I find out what I need to correct? |
lpinca commented Feb 23, 2017
@SgiobairOg There seem to be only lint issues. Run |
removed excess spaces from line 9
lpinca commented Feb 23, 2017
SgiobairOg commented Feb 23, 2017
Alright, down to one fail. Is test/arm saying that whatever I've done here doesn't work on arm-based chips? Where would I track down details on this one. |
lpinca commented Feb 23, 2017
@SgiobairOg no fails, CI is 100% green 🎉. If you click on details you'll see that tests passed. |
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
jasnell commented Feb 24, 2017
Landed in 817b28b |
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: #11482 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]>
8 participants
docs for assert.throws() allows the addition of a regular expression argument.
test/parallel/test-zlib-write-after-close.js line 9 contains an assert.throws() call which only has a single argument.
Per NodeToDo have added regex argument to the assert.throws() call to validate received error message.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test