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 flaky fs-watch tests #8115

Closed
wants to merge 3 commits into from
Closed

Conversation

santigimeno
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

test-fs-watch-recursive and test-fs-watch were both watching the
same folder: tmp/testsubdir so running them sequentially on OS X
could make test-fs-watch to fail due to events generated in the other
test. Make them watch a random directory to fix the issue.

Fixes: #8045

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 15, 2016
@santigimeno santigimeno added macos Issues and PRs related to the macOS platform / OSX. dont-land-on-v4.x labels Aug 15, 2016
@santigimeno
Copy link
Member Author

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Aug 15, 2016
@Trott
Copy link
Member

Trott commented Aug 16, 2016

LGTM

@Trott
Copy link
Member

Trott commented Aug 16, 2016

/cc @bnoordhuis

@evanlucas
Copy link
Contributor

LGTM

`test-fs-watch-recursive` and `test-fs-watch` were both watching the
same folder: `tmp/testsubdir` so running them sequentially on `OS X`
could make `test-fs-watch` to fail due to events generated in the other
test. Make them watch a random directory to fix the issue.

Fixes: nodejs#8045
@santigimeno
Copy link
Member Author

Updated so the random directory is not created twice in test-fs-watch

@santigimeno
Copy link
Member Author

@@ -78,7 +73,9 @@ setImmediate(function() {
fs.writeFileSync(filepathTwoAbs, 'pardner');
});

fs.mkdirSync(testsubdir, 0o700);
var filenameThree = 'newfile.txt';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make these const?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I was just trying to keep the modifications to a minimum. Should I use const everywhere on the test or just in the variables I moved?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just update the variables you're moving. It won't change the size of the diff.

@cjihrig
Copy link
Contributor

cjihrig commented Aug 16, 2016

LGTM

@santigimeno
Copy link
Member Author

@jasnell
Copy link
Member

jasnell commented Aug 16, 2016

LGTM

jasnell pushed a commit that referenced this pull request Aug 18, 2016
`test-fs-watch-recursive` and `test-fs-watch` were both watching the
same folder: `tmp/testsubdir` so running them sequentially on `OS X`
could make `test-fs-watch` to fail due to events generated in the other
test. Make them watch a random directory to fix the issue.

Fixes: #8045
PR-URL: #8115
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Aug 18, 2016

Landed in 5f617c5. Thank you @santigimeno !

@jasnell jasnell closed this Aug 18, 2016
evanlucas pushed a commit that referenced this pull request Aug 24, 2016
`test-fs-watch-recursive` and `test-fs-watch` were both watching the
same folder: `tmp/testsubdir` so running them sequentially on `OS X`
could make `test-fs-watch` to fail due to events generated in the other
test. Make them watch a random directory to fix the issue.

Fixes: #8045
PR-URL: #8115
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. macos Issues and PRs related to the macOS platform / OSX. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

darwin,test: flaky test sequential/test-fs-watch
7 participants