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 flaky test-child-process-fork-dgram #9098

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 14, 2016

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

test dgram child_process

Description of change

test-child-process-fork-dgram is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

  • Confirm child has received the server before sending packets
  • Close the server instance on the parent or child after receiving a

Alternative to #8697

Believed/hope to fix:
#8949
#8271

`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: nodejs#8697
Fixes: nodejs#8949
Fixes: nodejs#8271
@Trott Trott added child_process Issues and PRs related to the child_process subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests. freebsd Issues and PRs related to the FreeBSD platform. macos Issues and PRs related to the macOS platform / OSX. labels Oct 14, 2016
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 14, 2016
@Trott
Copy link
Member Author

Trott commented Oct 14, 2016

/cc @gibfahn @santigimeno @mhdawson

I confirmed that the test works if the child process closes its server first and that it works if the parent process closes its server first. And I also confirmed that this test works on one of the CI FreeBSD hosts where the current one fails 100% of the time.

@Trott
Copy link
Member Author

Trott commented Oct 14, 2016

@Trott
Copy link
Member Author

Trott commented Oct 14, 2016

Small stress test (100 runs) against all available platforms for stress testing: https://ci.nodejs.org/job/node-stress-single-test/1000/

@santigimeno
Copy link
Member

LGTM if CI is green

@Trott
Copy link
Member Author

Trott commented Oct 14, 2016

CI failures are infra related, unrelated to this change.

@gibfahn
Copy link
Member

gibfahn commented Oct 15, 2016

General idea LGTM, but given that I've run this test 1000+ times on AIX and macOS 10.12 without seeing any failures, it's worth being extra-thorough about making sure this fixes the test before we mark it as non-flaky. On AIX I suspect it's load-related, not sure about FreeBSD.

@Trott
Copy link
Member Author

Trott commented Oct 15, 2016

CI stress test against master (so we can see if the CI stress test above that showed no related failures is meaningful): https://ci.nodejs.org/job/node-stress-single-test/1010/

EDIT: OK, stress test not meaningful. sigh

EDIT 2: Although I don't think it includes the version of FreeBSD that was failing reliably.

EDIT 3: Argh, yes we do, it's freebsd10-64.

@Trott
Copy link
Member Author

Trott commented Oct 18, 2016

@gibfahn I am able to make the failure happen from the command line if I log into test-osuosl-aix61-ppc64_be-2 in the project test infrastructure. It seems to fail hang (which would result in the timeouts we see) about one out of every three times.

With the modifications in this PR, the test is reliable. ✨

Trott added a commit to Trott/io.js that referenced this pull request Oct 18, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: nodejs#8697
Fixes: nodejs#8949
Fixes: nodejs#8271
PR-URL: nodejs#9098
Reviewed-By: Santiago Gimeno <[email protected]>
@Trott
Copy link
Member Author

Trott commented Oct 18, 2016

Landed in 03afecd

@Trott Trott closed this Oct 18, 2016
jasnell pushed a commit that referenced this pull request Oct 18, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: #8697
Fixes: #8949
Fixes: #8271
PR-URL: #9098
Reviewed-By: Santiago Gimeno <[email protected]>
@MylesBorins
Copy link
Contributor

@Trott safe to assume this should be backported? It isn't landing cleanly would you be willing to submit a backport?

Trott added a commit to Trott/io.js that referenced this pull request Nov 12, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: nodejs#8697
Fixes: nodejs#8949
Fixes: nodejs#8271
PR-URL: nodejs#9098
Reviewed-By: Santiago Gimeno <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Nov 12, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: nodejs#8697
Fixes: nodejs#8949
Fixes: nodejs#8271
PR-URL: nodejs#9098
Reviewed-By: Santiago Gimeno <[email protected]>
@Trott
Copy link
Member Author

Trott commented Nov 12, 2016

@thealphanerd Backports done in #9558 (to v6.x-staging) and #9559 (to v4.x-staging)

MylesBorins pushed a commit that referenced this pull request Nov 14, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: #8697
Fixes: #8949
Fixes: #8271
PR-URL: #9098
Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
`test-child-process-fork-dgram` is unreliable on some platforms,
especially FreeBSD and AIX within the project's continuous integration
testing. It has also been observed to be flaky on macos.

* Confirm child has received the server before sending packets
* Close the server instance on the parent or child after receiving a

Refs: #8697
Fixes: #8949
Fixes: #8271
PR-URL: #9098
Reviewed-By: Santiago Gimeno <[email protected]>
This was referenced Nov 22, 2016
@Trott Trott deleted the fix-dgram branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. freebsd Issues and PRs related to the FreeBSD platform. 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.

5 participants