test: fix test-debug-port-numbers on OS X

[santigimeno]

Checklist

• tests and code linting passes
• the commit message follows commit guidelines

Affected core subsystem(s)

test

Description of change

According to kill(2), kill returns EPERM error if when signalling a
process group any of the members could not be signaled.

Refs: #7037

[santigimeno]

This test was failing in OS Xhttps://ci.nodejs.org/job/node-test-commit-osx/3581/nodes=osx1010/:

not ok 191 parallel/test-debug-port-numbers # # assert.js:90 # throw new assert.AssertionError({# ^ # AssertionError: 'EPERM' === 'ESRCH' # at kill (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/parallel/test-debug-port-numbers.js:47:12) # at update (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/parallel/test-debug-port-numbers.js:37:7) # at Socket.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/parallel/test-debug-port-numbers.js:21:65) # at emitOne (events.js:101:20) # at Socket.emit (events.js:188:7) # at readableAddChunk (_stream_readable.js:172:18) # at Socket.Readable.push (_stream_readable.js:130:10) # at Pipe.onread (net.js:542:20) # debug> debug> debug> debug> �< Debugger listening on port 12347 # debug> �connecting to 127.0.0.1:12347 ... ok # debug> �< Debugger listening on port 12348 # debug> �connecting to 127.0.0.1:12348 ...�< Debugger listening on port 12346 # debug> �connecting to 127.0.0.1:12346 ...�< Debugger listening on port 12349 # debug> �connecting to 127.0.0.1:12349 ... ok # debug> ok # debug> ok # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # �break in test/parallel/test-debug-port-numbers.js:1 

[santigimeno]

Stress test without this change fails: https://ci.nodejs.org/job/node-stress-single-test/749.
Stress test with this change passes: https://ci.nodejs.org/job/node-stress-single-test/750.

[bnoordhuis]

I think the logic should be ESRCH || EPERM iff OS X, and you might as well drop the 'iff OS X' part in that case because the BSDs probably exhibit the same behavior.

I checked the xnu and libc sources and the error code seems to depend on whether POSIX compatibility mode is enabled in the libc wrapper: it returns EPERM if it is, ESRCH otherwise. That would explain why I don't see EPERM with 10.8.

[cjihrig]

LGTM. I think @bnoordhuis suggestion makes sense. One question though:

According to kill(2), kill returns EPERM error if when signalling a process group any of the members could not be signaled.

Does that mean that processes may be left behind?

[bnoordhuis]

@cjihrig My reading of the xnu sources is that EPERM or ESRCH are only returned when the process group is gone (i.e., empty.)

[santigimeno]

@bnoordhuis, from the FreeBSD 10.3 man: [EPERM] The sending process does not have permission to send sig to the receiving process. So I'm not sure if it applies to FreeBSD. I can add it anyway.

[bnoordhuis]

I think xnu inherits most of its signal handling logic from FreeBSD but it's possible that the POSIX compatibility stuff is a later addition. I don't think allowing EPERM on other platforms will hurt, at any rate.

[santigimeno]

Yes, it makes sense. So just to be sure... ESRCH || EPERM on OS X and FreeBSD right?

[bnoordhuis]

I'd do it unconditionally, i.e., no platform-specific checks.

[santigimeno]

PR updated per your comments. Thanks!

[santigimeno]

CI: https://ci.nodejs.org/job/node-test-pull-request/2847/

[cjihrig]

I don't think this is right. 'EPERM' || 'ESRCH' will always evaluate to EPERM

[cjihrig]

Also, assert.ok() just asserts that e.code is truthy.

[santigimeno]

You're right. I was fixing that :(

[santigimeno]

Updated

[cjihrig]

LGTM

[santigimeno]

CI: https://ci.nodejs.org/job/node-test-pull-request/2848/

[bnoordhuis]

LGTM

[santigimeno]

CI: https://ci.nodejs.org/job/node-test-commit/3579/. All green except some unrelated failures in some ARM bots. Landing

[santigimeno]

Landed in 6e148a3. Thanks!