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-repl timeout and tmpdir refresh #25425

Closed
wants to merge 1 commit into from

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Jan 9, 2019

I could not find a way to stress test this on the pi1 as armv6 (and armv7 for that matter) seems to not be a selectable target in Jenkins, although armv8/arm64 targets are?

Anyway, what prompted this was this recent CI failure on pi1-docker:

15:05:58 not ok 229 parallel/test-repl
15:05:58   ---
15:05:58   duration_ms: 7.962
15:05:58   severity: fail
15:05:58   exitcode: 1
15:05:58   stack: |-
15:05:58     out: ""
15:05:58     in: ""
15:05:58     out: "message"
15:05:58     in: "'Read, Eval, Print Loop'"
15:05:58     out: "invoke_me(987)"
15:05:58     in: "'invoked 987'"
15:05:58     out: "a = 12345"
15:05:58     in: "12345"
15:05:58     out: "{a:1}"
15:05:58     in: "{ a: 1 }"
15:05:58     out: "throw new Error('test error');"
15:05:58     in: "Thrown:"
15:05:58     in: "Error: test error"
15:05:58     out: "throw { foo: 'bar' };"
15:05:58     in: "Thrown: { foo: 'bar' }"
15:05:58     out: "function test_func() {"
15:05:58     /home/iojs/build/workspace/node-test-binary-arm/test/common/index.js:689
15:05:58     const crashOnUnhandledRejection = (err) => { throw err; };
15:05:58                                                  ^
15:05:58     
15:05:58     Error: The REPL did not reply as expected for:
15:05:58     
15:05:58     '... '
15:05:58         at Timeout.setTimeout [as _onTimeout] (/home/iojs/build/workspace/node-test-binary-arm/test/parallel/test-repl.js:854:14)
15:05:58         at listOnTimeout (timers.js:324:15)
15:05:58         at processTimers (timers.js:268:5)

The tmpdir addition was to allow running the test by itself, outside of a make test.

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

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

@mscdex mscdex added repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests. labels Jan 9, 2019
@Trott
Copy link
Member

Trott commented Jan 9, 2019

I could not find a way to stress test this on the pi1 as armv6 (and armv7 for that matter) seems to not be a selectable target in Jenkins, although armv8/arm64 targets are?

The pi1 devices have their own separate stress test Jenkins jobs. https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-binary/ and https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-fanned/

@mscdex
Copy link
Contributor Author

mscdex commented Jan 9, 2019

The pi1 devices have their own separate stress test Jenkins jobs. https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-binary/ and https://ci.nodejs.org/view/All/job/node-stress-single-test-pi1-fanned/

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@Trott
Copy link
Member

Trott commented Jan 10, 2019

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@nodejs/build ^^^^^^^

@mscdex
Copy link
Contributor Author

mscdex commented Jan 10, 2019

Also it looks like the pi1-fanned job actually executes on pi2 and pi3 machines in addition to pi1.

@mscdex
Copy link
Contributor Author

mscdex commented Jan 10, 2019

@refack
Copy link
Contributor

refack commented Jan 12, 2019

Ok, those do not seem to be listed under any existing tabs except "All". Could we add them (at least the fanned job) to the "Node.js" tab alongside the main stress test job that already exists there?

@nodejs/build ^^^^^^^

Does this make sense: https://ci.nodejs.org/view/Stress/

@mscdex
Copy link
Contributor Author

mscdex commented Jan 12, 2019

Does this make sense: https://ci.nodejs.org/view/Stress/

That is helpful, yes. Thanks.

@antsmartian antsmartian added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 15, 2019
@antsmartian
Copy link
Contributor

antsmartian commented Jan 17, 2019

Landed in 7b6e9ae 🎉

pull bot pushed a commit to zys-contrib/node that referenced this pull request Jan 17, 2019
PR-URL: nodejs#25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@mscdex mscdex deleted the test-repl-fixes branch January 17, 2019 19:05
addaleax pushed a commit that referenced this pull request Jan 23, 2019
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
PR-URL: #25425
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants