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 fs.readFile('/dev/stdin') tests #2265

Merged
merged 1 commit into from
Jul 29, 2015

Conversation

bnoordhuis
Copy link
Member

The tests were creating the temp fixture file in both the parent
and the child process, leading to interesting race conditions on
the slower buildbots.

R=@Fishrock123

CI: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/21/
Previous ARM-only run: https://jenkins-iojs.nodesource.com/job/iojs+pr+arm/192/

[Note to self: add link to https://github.com//issues/2261 in commit log.]

@brendanashworth brendanashworth added the test Issues and PRs related to the tests. label Jul 29, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Jul 29, 2015

LGTM

@rvagg
Copy link
Member

rvagg commented Jul 29, 2015

lgtm

also, FYI these tests started failing after I put the build directories on an NFS share, changing the I/O profile of the disks

@Fishrock123
Copy link
Contributor

Ah nice catch, LGTM

The tests were creating the temp fixture file in both the parent
and the child process, leading to interesting race conditions on
the slower buildbots.

Rod notes that the tests started failing after putting the build
directory on a NFS mount.

Fixes: nodejs#2261
PR-URL: nodejs#2265
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@bnoordhuis bnoordhuis force-pushed the fix-readfile-pipe-tests branch from 96da915 to bc733f7 Compare July 29, 2015 12:00
@bnoordhuis bnoordhuis closed this Jul 29, 2015
@bnoordhuis bnoordhuis deleted the fix-readfile-pipe-tests branch July 29, 2015 12:00
@bnoordhuis bnoordhuis merged commit bc733f7 into nodejs:master Jul 29, 2015
@rvagg rvagg mentioned this pull request Aug 4, 2015
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.

5 participants