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

Investigate flaky test-dgram-udp6-send-default-host on OS X #6577

Closed
Trott opened this issue May 4, 2016 · 13 comments
Closed

Investigate flaky test-dgram-udp6-send-default-host on OS X #6577

Trott opened this issue May 4, 2016 · 13 comments
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.

Comments

@Trott
Copy link
Member

Trott commented May 4, 2016

  • Version: master branch
  • Platform: OS X
  • Subsystem: test? dgram?

Failure in CI on test of unrelated change: https://ci.nodejs.org/job/node-test-commit-osx/3210/nodes=osx1010/tapTestReport/test.tap-213/

not ok 213 test-dgram-udp6-send-default-host.js
# 
# assert.js:90
#   throw new assert.AssertionError({
#   ^
# AssertionError: message was received correctly
#     at Socket.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1010/test/parallel/test-dgram-udp6-send-default-host.js:29:10)
#     at emitTwo (events.js:106:13)
#     at Socket.emit (events.js:191:7)
#     at UDP.onMessage (dgram.js:532:8)
  ---
  duration_ms: 0.145

/cc @nodejs/testing @addaleax

@Trott Trott added dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests. macos Issues and PRs related to the macOS platform / OSX. labels May 4, 2016
@Trott
Copy link
Member Author

Trott commented May 4, 2016

/cc @mcollina

@Trott
Copy link
Member Author

Trott commented May 4, 2016

@santigimeno
Copy link
Member

Have seen this failure locally a couple of times though I cannot reproduce it consistently :/.
Maybe use assert.strictEqual here would help to find out what's going on.

@Trott
Copy link
Member Author

Trott commented May 4, 2016

@santigimeno Yup, I had the same thought and am putting in the PR for that right now...

Trott added a commit to Trott/io.js that referenced this issue May 4, 2016
Previously, the assertion changed in this commit would print the same
message no matter which of the four test cases failed. The assert has
been changed so that it will indicate which test case failed.

Refs: nodejs#6577
@Trott
Copy link
Member Author

Trott commented May 4, 2016

#6581

@mcollina
Copy link
Member

mcollina commented May 5, 2016

This is fairly unexpected, as the ipv4 equivalent of this test is not flaky.

Is this flaky only on Mac? Hopefully #6581 should tell what's happening. I have never seen it fail here.

santigimeno pushed a commit to santigimeno/node that referenced this issue May 5, 2016
Previously, the assertion changed in this commit would print the same
message no matter which of the four test cases failed. The assert has
been changed so that it will indicate which test case failed.

Refs: nodejs#6577
@santigimeno
Copy link
Member

It looks like the problem is that sometimes the messages are not received in order, which makes sense being UDP. See output from https://ci.nodejs.org/job/node-stress-single-test/nodes=osx1010/690 :

not ok 1 test-dgram-udp6-send-default-host.js
# 
# assert.js:90
#   throw new assert.AssertionError({
#   ^
# AssertionError: 'hello' === 'yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
#     at Socket.<anonymous> (/Users/iojs/build/workspace/node-stress-single-test/nodes/osx1010/test/parallel/test-dgram-udp6-send-default-host.js:29:10)
#     at emitTwo (events.js:106:13)
#     at Socket.emit (events.js:191:7)
#     at UDP.onMessage (dgram.js:532:8)
  ---
  duration_ms: 0.139

@addaleax
Copy link
Member

addaleax commented May 5, 2016

Could it be that the kernel decides to reorder the packets here for some reason? That’s allowed, although I can’t see why it would. But it would explain why the test is flaky and why this is an OS-specific issue.

@santigimeno
Copy link
Member

@addaleax Some comments here like this suggest that could be the reason

@addaleax
Copy link
Member

addaleax commented May 5, 2016

👍 I guess the IPv4 test has a chance of being flaky too, then?

@santigimeno
Copy link
Member

Is there an equivalent IPv4 test? I can't find it.

@addaleax
Copy link
Member

addaleax commented May 5, 2016

@santigimeno test/parallel/test-dgram-send-default-host.js

@santigimeno
Copy link
Member

@addaleax Thanks! I guess it should also be rewritten

addaleax added a commit to addaleax/node that referenced this issue May 10, 2016
Allow out of order replies in the flaky
`test-dgram{-upd6,}-send-default-host.js` files by sorting
the incoming replies after receiving them.

Fixes: nodejs#6577
PR-URL: nodejs#6607
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
addaleax added a commit to addaleax/node that referenced this issue May 10, 2016
Allow out of order replies in the flaky
`test-dgram{-upd6,}-send-default-host.js` files by sorting
the incoming replies after receiving them.

PR-URL: nodejs#6607
Fixes: nodejs#6577
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
evanlucas pushed a commit that referenced this issue May 17, 2016
Allow out of order replies in the flaky
`test-dgram{-upd6,}-send-default-host.js` files by sorting
the incoming replies after receiving them.

PR-URL: #6607
Fixes: #6577
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

4 participants