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

investigate potentially flaky test on centos #3863

Closed
jasnell opened this issue Nov 16, 2015 · 7 comments
Closed

investigate potentially flaky test on centos #3863

jasnell opened this issue Nov 16, 2015 · 7 comments
Labels
child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.

Comments

@jasnell
Copy link
Member

jasnell commented Nov 16, 2015

https://ci.nodejs.org/job/node-test-commit-linux/1215/nodes=centos5-32/console

not ok 61 test-child-process-spawnsync-input.js
#
#assert.js:89
#  throw new assert.AssertionError({
#  ^
#AssertionError: <Buffer > deepEqual <Buffer 74 68 69 73 20 69 73 20 73 74 64 6f 75 74 0a>
#    at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/centos5-32/test/parallel/test-child-process-spawnsync-input.js:98:8)
#    at Module._compile (module.js:423:26)
#    at Object.Module._extensions..js (module.js:430:10)
#    at Module.load (module.js:354:32)
#    at Function.Module._load (module.js:311:12)
#    at Function.Module.runMain (module.js:455:10)
#    at startup (node.js:138:18)
#    at node.js:974:3

I've seen this before, only on centos.

@jasnell jasnell added the test Issues and PRs related to the tests. label Nov 16, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Nov 16, 2015

I wonder if this failed assertion is even really necessary. The assertion is verifying that we can successfully get a result buffer larger than the user specified max buffer size. Maybe someone who was more involved with the project when the test was originally written can give more perspective, but it seems unnecessary to me.

@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Nov 16, 2015
@Trott
Copy link
Member

Trott commented Nov 16, 2015

Here's the commit that introduced that line of code on the test: e8df267

¯\_(ツ)_/¯

@Trott
Copy link
Member

Trott commented Nov 16, 2015

@Trott
Copy link
Member

Trott commented Nov 17, 2015

FWIW, I split out the flaky part of the test file from the rest of the test file. The flaky part still fails in the stress test, and the non-flaky part looks like it's going to complete it's run with no failures. So, at least, that would seem to rule out a side effect from the rest of the file. The issue, whatever it is, is triggered by just the narrow test at the end of the file.

@Trott
Copy link
Member

Trott commented Nov 17, 2015

The problem here seems like a race condition. Most of the time, stdout flushes before spawnSync() completes but every once in a while, either stdout does not flush or it just drops/loses stuff for whatever reason. That's my guess anyway. Not sure yet how to proceed to confirm or refute the theory...

@Trott
Copy link
Member

Trott commented Nov 17, 2015

Maybe related?: nodejs/node-v0.x-archive#8329

Trott added a commit to Trott/io.js that referenced this issue Nov 17, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: nodejs#3863
@Trott
Copy link
Member

Trott commented Nov 17, 2015

#3889 fixes this flaky test.

@Trott Trott closed this as completed in ac319c3 Nov 18, 2015
Trott added a commit that referenced this issue Dec 1, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: #3863
PR-URL: #3889
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Trott added a commit that referenced this issue Dec 4, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: #3863
PR-URL: #3889
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Trott added a commit that referenced this issue Dec 5, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: #3863
PR-URL: #3889
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Trott added a commit that referenced this issue Dec 17, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: #3863
PR-URL: #3889
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Trott added a commit that referenced this issue Dec 23, 2015
Move portion of `test-child-process-spawnsync-input.js` (that has been
flaky on CentOS in CI) to its own file. This allows us to more easily
eliminate the cause of the flakiness without affecting other unrelated
portions of the test.

Fixes: #3863
PR-URL: #3889
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
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. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

4 participants