Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failing tests on macos mojave #21679

Closed
devsnek opened this issue Jul 6, 2018 · 23 comments
Closed

failing tests on macos mojave #21679

devsnek opened this issue Jul 6, 2018 · 23 comments
Labels
macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.

Comments

@devsnek
Copy link
Member

devsnek commented Jul 6, 2018

test-cluster-bind-privileged-port and test-cluster-shared-handle-bind-privileged-port have been consistently failing for me since i updated to mojave

@devsnek devsnek added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jul 6, 2018
@Trott
Copy link
Member

Trott commented Jul 6, 2018

Can you provide the full output of the test failures?

@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2018

=== release test-cluster-shared-handle-bind-privileged-port ===
Path: parallel/test-cluster-shared-handle-bind-privileged-port
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: listen should have failed at test/parallel/test-cluster-shared-handle-bind-privileged-port.js:42
    at Server.mustNotCall (test/common/index.js:533:12)
    at Object.onceWrapper (events.js:273:13)
    at Server.emit (events.js:187:15)
    at emitListeningNT (net.js:1369:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 1
+ 0
    at Worker.<anonymous> (test/parallel/test-cluster-shared-handle-bind-privileged-port.js:38:12)
    at Worker.<anonymous> (test/common/index.js:467:15)
    at Worker.emit (events.js:182:13)
    at ChildProcess.worker.process.once (internal/cluster/master.js:193:12)
    at Object.onceWrapper (events.js:273:13)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:237:12)
Command: out/Release/node test/parallel/test-cluster-shared-handle-bind-privileged-port.js
=== release test-cluster-bind-privileged-port ===
Path: parallel/test-cluster-bind-privileged-port
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 'EADDRINUSE'
+ 'EACCES'
    at Server.s.on.common.mustCall (test/parallel/test-cluster-bind-privileged-port.js:42:12)
    at Server.<anonymous> (test/common/index.js:467:15)
    at Server.emit (events.js:182:13)
    at listenOnMasterHandle (net.js:1403:21)
    at rr (internal/cluster/child.js:121:12)
    at Worker.send (internal/cluster/child.js:88:7)
    at process.onInternalMessage (internal/cluster/utils.js:42:8)
    at process.emit (events.js:187:15)
    at emit (internal/child_process.js:811:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 1
+ 0
    at Worker.cluster.fork.on.common.mustCall (test/parallel/test-cluster-bind-privileged-port.js:36:12)
    at Worker.<anonymous> (test/common/index.js:467:15)
    at Worker.emit (events.js:182:13)
    at ChildProcess.worker.process.once (internal/cluster/master.js:193:12)
    at Object.onceWrapper (events.js:273:13)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:237:12)
Command: out/Release/node test/parallel/test-cluster-bind-privileged-port.js
[00:00|% 100|+   0|-   2]: Done

@Trott
Copy link
Member

Trott commented Jul 6, 2018

Seems like there's something listening on port 42. Any chance it's a stray node test process that failed to exit from a previous run?

@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2018

@Trott actually there was a stray node process running, unfortunately it was not doing anything network related (it was orphaned from my repl prototype)

@Trott
Copy link
Member

Trott commented Jul 6, 2018

So the tests are still failing?

If so, what's the output of netstat -vanp tcp | grep '\.42 '?

@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2018

@Trott nothing is listening on 42, tests still fail:

$ netstat -vanp tcp | grep '\.42\b'
$ tools/test.py -J test/parallel/test-cluster-*-privileged-port.js | sed -e "s@$(pwd)@@"
=== release test-cluster-bind-privileged-port ===
Path: parallel/test-cluster-bind-privileged-port
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 'EADDRINUSE'
+ 'EACCES'
    at Server.s.on.common.mustCall (/test/parallel/test-cluster-bind-privileged-port.js:42:12)
    at Server.<anonymous> (/test/common/index.js:467:15)
    at Server.emit (events.js:182:13)
    at listenOnMasterHandle (net.js:1403:21)
    at rr (internal/cluster/child.js:121:12)
    at Worker.send (internal/cluster/child.js:88:7)
    at process.onInternalMessage (internal/cluster/utils.js:42:8)
    at process.emit (events.js:187:15)
    at emit (internal/child_process.js:811:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 1
+ 0
    at Worker.cluster.fork.on.common.mustCall (/test/parallel/test-cluster-bind-privileged-port.js:36:12)
    at Worker.<anonymous> (/test/common/index.js:467:15)
    at Worker.emit (events.js:182:13)
    at ChildProcess.worker.process.once (internal/cluster/master.js:193:12)
    at Object.onceWrapper (events.js:273:13)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:237:12)
Command: out/Release/node /test/parallel/test-cluster-bind-privileged-port.js
=== release test-cluster-shared-handle-bind-privileged-port ===
Path: parallel/test-cluster-shared-handle-bind-privileged-port
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: listen should have failed at /test/parallel/test-cluster-shared-handle-bind-privileged-port.js:42
    at Server.mustNotCall (/test/common/index.js:533:12)
    at Object.onceWrapper (events.js:273:13)
    at Server.emit (events.js:187:15)
    at emitListeningNT (net.js:1369:10)
    at process._tickCallback (internal/process/next_tick.js:63:19)
assert.js:80
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 1
+ 0
    at Worker.<anonymous> (/test/parallel/test-cluster-shared-handle-bind-privileged-port.js:38:12)
    at Worker.<anonymous> (/test/common/index.js:467:15)
    at Worker.emit (events.js:182:13)
    at ChildProcess.worker.process.once (internal/cluster/master.js:193:12)
    at Object.onceWrapper (events.js:273:13)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:237:12)
Command: out/Release/node /test/parallel/test-cluster-shared-handle-bind-privileged-port.js
[00:00|% 100|+   0|-   2]: Done
bash-4.4$

@Trott
Copy link
Member

Trott commented Jul 6, 2018

Oooooohhhh.... I see why you're getting EADDRINUSE. It's because the other test is successfully binding on port 42. You can confirm that by running the tests one at a time rather than together. They will likely both still fail but not with the EADDRINUSE that you see in one of the tests above.

Any chance at all that you're running these tests as a non-root user that nonetheless somehow has privileges to bind to port 42? I mean, based on the results, that sure seems like what's going on.

And apologies if the above is all stuff you've already figured out. :-D

@Trott
Copy link
Member

Trott commented Jul 6, 2018

Do you get ERROR: Failed to serve: Error: listen EACCES 0.0.0.0:42 when you run npx serve -l 42 /dev/null (or get an error when you otherwise try to set up a server on port 42 without using super-user)?

@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2018

@Trott no errors... that's slightly disconcerting. perhaps a rather nasty bug with permissions in mojave?

@Trott
Copy link
Member

Trott commented Jul 6, 2018

@devsnek Just to be sure, maybe let's take Node.js out of the equation? Do you get a permission error with python -m SimpleHTTPServer 42?

@devsnek
Copy link
Member Author

devsnek commented Jul 6, 2018

@Trott yeah i did some more testing on multiple accounts, etc. mojave has decided that elevation isn't a thing, i assume its a bug.

@devsnek devsnek removed the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jul 7, 2018
@devsnek
Copy link
Member Author

devsnek commented Jul 7, 2018

i'm not sure if this issue needs to stay open as the bug is with macos not our tests

@Trott
Copy link
Member

Trott commented Jul 7, 2018

It can always be re-opened if this is somehow a thing that needs to be worked around.

@Trott Trott closed this as completed Jul 7, 2018
@jdalton
Copy link
Member

jdalton commented Oct 3, 2018

I'm able to repro this same issue in macos mojave.

@devsnek
Copy link
Member Author

devsnek commented Oct 3, 2018

fwiw i tried to contact apple about this. i think we should skip these tests on macos.

@devsnek devsnek reopened this Oct 3, 2018
@Trott
Copy link
Member

Trott commented Oct 3, 2018

I don't expect it will get us any special results or anything, but I'm going to ping the one @nodejs contributor that I know works at Apple just in case they can help... /ping @gibfahn

@Trott
Copy link
Member

Trott commented Oct 3, 2018

Also: @nodejs/platform-macos

@Trott Trott added the macos Issues and PRs related to the macOS platform / OSX. label Oct 3, 2018
@joyeecheung
Copy link
Member

joyeecheung commented Oct 3, 2018

I'll remove the CI/flaky test tag since this does not (AFAIK) show up in our CI. I'll add test tag instead.

EDIT: oh, I see the CI tag has been removed

@joyeecheung joyeecheung added the test Issues and PRs related to the tests. label Oct 3, 2018
@gdams
Copy link
Member

gdams commented Oct 3, 2018

I'm going to work with @mhdawson to add Mojave to our CI.

@Agheb
Copy link
Contributor

Agheb commented Oct 12, 2018

As described in nodejs/code-and-learn#86 (comment), we're currently ignoring these test failures for test-cluster-bind-privileged-port and test-cluster-shared-handle-bind-privileged-port on macOS Mojave

@ChALkeR
Copy link
Member

ChALkeR commented Oct 12, 2018

Could we version-detect macOS Mojave, perhaps?

@devsnek
Copy link
Member Author

devsnek commented Oct 12, 2018

@ChALkeR #23550

addaleax pushed a commit that referenced this issue Oct 21, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
jasnell pushed a commit that referenced this issue Oct 21, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 30, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 26, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
rvagg pushed a commit that referenced this issue Nov 28, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 29, 2018
Refs: #21679
PR-URL: #23550
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: George Adams <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@Trott
Copy link
Member

Trott commented Jan 22, 2019

I think this should be closed (after a PR is opened to remove the known_issues test, which I'll do right now). Reportedly, Apple has decided that requiring superuser for privileged ports is ineffective security and should be dispensed with. If that is correct, then as far as Apple is concerned, this is a feature and not a bug. Node.js doesn't enforce what ports are privileged. That's up to the OS. So, not a Node.js bug.

Trott added a commit to Trott/io.js that referenced this issue Jan 22, 2019
The test was added to check for a bug in macOS Mojave, but it turns out
the issue is a macOS feature, not a bug.

Refs: nodejs#21679 (comment)
Fixes: nodejs#21679
@Trott Trott closed this as completed in 35f45ba Jan 24, 2019
addaleax pushed a commit that referenced this issue Jan 28, 2019
The test was added to check for a bug in macOS Mojave, but it turns out
the issue is a macOS feature, not a bug.

Refs: #21679 (comment)
Fixes: #21679

PR-URL: #25649
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
BethGriggs pushed a commit that referenced this issue Apr 29, 2019
The test was added to check for a bug in macOS Mojave, but it turns out
the issue is a macOS feature, not a bug.

Refs: #21679 (comment)
Fixes: #21679

PR-URL: #25649
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
BethGriggs pushed a commit that referenced this issue May 10, 2019
The test was added to check for a bug in macOS Mojave, but it turns out
the issue is a macOS feature, not a bug.

Refs: #21679 (comment)
Fixes: #21679

PR-URL: #25649
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
MylesBorins pushed a commit that referenced this issue May 16, 2019
The test was added to check for a bug in macOS Mojave, but it turns out
the issue is a macOS feature, not a bug.

Refs: #21679 (comment)
Fixes: #21679

PR-URL: #25649
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants