test: check for error on Windows #2035
Conversation
Trott
added
dgram
brendanashworth
added
the
windows
|
|
||
| function noop() {} | ||
|
|
||
| if (cluster.isMaster) { | ||
| var worker1 = cluster.fork(); | ||
|
|
||
| if (isWindows) { | ||
| worker1.on('error', function(er) { |
There was a problem hiding this comment.
Maybe wrap the callback in common.mustCall(...)?
|
LGTM with a suggestion. Can you remove the console.log statements? |
| // test | ||
| if (process.platform == 'win32') | ||
| process.exit(0); | ||
| var isWindows = process.platform === 'win32'; |
There was a problem hiding this comment.
You can use common.isWindows here.
|
I agree about the |
Instead of not running the dgram-bind-shared-ports on Windows, check that it gets ENOTSUP.
|
Per feedback from @bnoordhuis and @cjihrig:
New CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/71/ |
|
Yet another CI to confirm that the smartOS failure was a flaky test and not something in this PR: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/72/ |
|
Yea, this PR only changes a test, so unless |
| worker1.kill(); | ||
| }; | ||
|
|
||
| worker1.on('error', common.mustCall(checkErrType, 1)); |
There was a problem hiding this comment.
1 is the default, you can just write it as worker1.on('error', common.mustCall(function(err) { (although it's not wrong this way.)
|
LGTM |
|
Third time is the charm on the CI run. All green: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/73/ |
Instead of not running the dgram-bind-shared-ports on Windows, check that it gets ENOTSUP. PR-URL: #2035 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Merged in 8e9089a |
Instead of not running the dgram-bind-shared-ports on Windows, check that it gets ENOTSUP. PR-URL: nodejs#2035 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Instead of not running the dgram-bind-shared-ports
on Windows, check that it gets ENOTSUP.
CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/70/