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: prevent flakey test on pi2 #5537

Merged
merged 1 commit into from
Mar 3, 2016
Merged

Conversation

trevnorris
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Description of change

Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: #5302

R= @orangemocha
R= @Trott

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

@cjihrig
Copy link
Contributor

cjihrig commented Mar 2, 2016

LGTM

@orangemocha
Copy link
Contributor

Thank you. Could also remove the line in test/parallel/parallel.status that marks this test as flaky?

@orangemocha
Copy link
Contributor

CI stress run in progress: https://ci.nodejs.org/job/node-stress-single-test/552/

@Trott
Copy link
Member

Trott commented Mar 2, 2016

Test change LGTM if CI is green.

What @orangemocha said about removing the line in parallel.status for this test.

@mscdex mscdex added test Issues and PRs related to the tests. process Issues and PRs related to the process subsystem. labels Mar 2, 2016
@orangemocha
Copy link
Contributor

Stress run is green! LGTM

@orangemocha
Copy link
Contributor

The regular CI run failed because it wasn't certified safe (sigh). Here's a new run: https://ci.nodejs.org/job/node-test-pull-request/1827/

@orangemocha
Copy link
Contributor

CI is green.

Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: nodejs#5302
PR-URL: nodejs#5537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
@trevnorris
Copy link
Contributor Author

Thanks. Landed in 5d7265f.

@trevnorris trevnorris closed this Mar 3, 2016
@trevnorris trevnorris merged commit 5d7265f into nodejs:master Mar 3, 2016
@trevnorris trevnorris deleted the fix-flaky branch March 3, 2016 20:54
@Fishrock123 Fishrock123 mentioned this pull request Mar 7, 2016
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: #5302
PR-URL: #5537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: #5302
PR-URL: #5537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: #5302
PR-URL: #5537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
Looping rapidly and making new connections causes problems on pi2.
Instead create a new connection when an old connection has already been
made. Running a stress test of 600 times and they all passed.

Fixes: #5302
PR-URL: #5537
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Alexis Campailla <orangemocha@nodejs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate flaky test parallel/test-process-getactivehandles
7 participants