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

test: fix test-cluster-send-handle-large-payload #14780

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Aug 12, 2017

On macOS, the parent process might not receive a message if it
is sent to soon, and then subsequent messages are also sometimes not
received.

(Is this a bug or expected operating system behavior like the
way a file watcher is returned before it's actually watching the file
system on/ macOS?)

Send a second message after a delay on macOS.

While at it, minor refactoring to the test:

  • Blank line after loading common module per test-writing guide
  • Wrap arrow function in braces where implicit return is not needed
  • Remove unnecessary unref in subprocess

Fixes: #14747

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test cluster

On macOS, the parent process might not receive a message if it
is sent to soon, and then subsequent messages are also sometimes not
received.

(Is this a bug or expected operating system behavior like the
way a file watcher is returned before it's actually watching the file
system on/ macOS?)

Send a second message after a delay on macOS.

While at it, minor refactoring to the test:

* Blank line after loading `common` module per test-writing guide
* Wrap arrow function in braces where implicit return is not needed
* Remove unnecessary unref in subprocess

Fixes: nodejs#14747
@Trott Trott added cluster Issues and PRs related to the cluster subsystem. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests. labels Aug 12, 2017
@Trott
Copy link
Member Author

Trott commented Aug 12, 2017

@nodejs/platform-macos @addaleax @matthewloring @bnoordhuis @cjihrig @mcollina

@Trott
Copy link
Member Author

Trott commented Aug 12, 2017

@Trott
Copy link
Member Author

Trott commented Aug 12, 2017

CI is green.

Stress test for current master (shows errors): https://ci.nodejs.org/job/node-stress-single-test/1372/nodes=osx1010/

Stress test for this pull request (green!): https://ci.nodejs.org/job/node-stress-single-test/1371/nodes=osx1010/

@addaleax
Copy link
Member

LGTM but I would really kind of prefer to figure out why this is failing …

@Trott
Copy link
Member Author

Trott commented Aug 12, 2017

LGTM but I would really kind of prefer to figure out why this is failing …

Would a dtruss -f of a successful run of the test be useful, or would need one of a run that deadlocks/times out?

@addaleax
Copy link
Member

@Trott I think either would be helpful, and being able to compare both kinds even more so. ;)

@Trott
Copy link
Member Author

Trott commented Aug 13, 2017

@addaleax Using current master:

Success: https://gist.github.com/Trott/e32e1a48c84a9fdba09177e90bda05d4

Failure: https://gist.github.com/Trott/fb77e052ff369e33d1b18f5ad9492ba9 (terminated with cntl-c after it hung for a while, if that matters)

@Trott
Copy link
Member Author

Trott commented Aug 14, 2017

Landed in 28a47aa

@Trott Trott closed this Aug 14, 2017
Trott added a commit to Trott/io.js that referenced this pull request Aug 14, 2017
On macOS, the parent process might not receive a message if it
is sent to soon, and then subsequent messages are also sometimes not
received.

(Is this a bug or expected operating system behavior like the
way a file watcher is returned before it's actually watching the file
system on/ macOS?)

Send a second message after a delay on macOS.

While at it, minor refactoring to the test:

* Blank line after loading `common` module per test-writing guide
* Wrap arrow function in braces where implicit return is not needed
* Remove unnecessary unref in subprocess

PR-URL: nodejs#14780
Fixes: nodejs#14747
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
On macOS, the parent process might not receive a message if it
is sent to soon, and then subsequent messages are also sometimes not
received.

(Is this a bug or expected operating system behavior like the
way a file watcher is returned before it's actually watching the file
system on/ macOS?)

Send a second message after a delay on macOS.

While at it, minor refactoring to the test:

* Blank line after loading `common` module per test-writing guide
* Wrap arrow function in braces where implicit return is not needed
* Remove unnecessary unref in subprocess

PR-URL: #14780
Fixes: #14747
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: test-cluster-send-handle-large-payload intermittent failures
3 participants