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: move some test from sequential to parallel #6087

Closed
wants to merge 1 commit into from

Conversation

santigimeno
Copy link
Member

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

The only test with modifications is test-stdin-child-proc that was
passing when it should not because the exit code of the child process
was not being checked.

I have run the tests on Debian Jessie 64 and SmartOS and pass on both.

@claudiorodriguez claudiorodriguez added the test Issues and PRs related to the tests. label Apr 6, 2016
@claudiorodriguez
Copy link
Contributor

@santigimeno
Copy link
Member Author

/cc @nodejs/testing

@cjihrig
Copy link
Contributor

cjihrig commented Apr 7, 2016

LGTM and CI is green.

@jbergstroem
Copy link
Member

@santigimeno: can you run these tests with -J as well?

@jbergstroem
Copy link
Member

Disregard that. make test uses -J. I've spent too much time in CI :)

@jbergstroem
Copy link
Member

LGTM

@@ -0,0 +1,13 @@
'use strict';
// This tests that pausing and resuming stdin does not hang and timeout
// when done in a child process. See test/simple/test-stdin-pause-resume.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path has to be updated.

@jasnell
Copy link
Member

jasnell commented Apr 7, 2016

LGTM with nits addressed

@santigimeno
Copy link
Member Author

PR updated. Thanks!

The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.
@jasnell
Copy link
Member

jasnell commented Apr 8, 2016

New CI after update: https://ci.nodejs.org/job/node-test-pull-request/2221/

@thefourtheye
Copy link
Contributor

LGTM if CI is green.

@claudiorodriguez
Copy link
Contributor

CI green. LGTM

jasnell pushed a commit that referenced this pull request Apr 9, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@jasnell
Copy link
Member

jasnell commented Apr 9, 2016

Landed in eaab17c

@jasnell jasnell closed this Apr 9, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@MylesBorins MylesBorins mentioned this pull request Apr 20, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
This was referenced Apr 21, 2016
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
The only test with modifications is `test-stdin-child-proc` that was
passing when it should not because the exit code of the child process
was not being checked.

PR-URL: #6087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants