test,fs: delay unlink in test-regress-GH-4027.js#14010
Uh oh!
There was an error while loading. Please reload this page.
Conversation
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. Fixes: nodejs#13800
There was a problem hiding this comment.
LGTM if CI is green
I confirmed that changing the test this way (and making a few other modifications so as to run in Node.js 0.8) results in a test that fails (as expected) in 0.8.9 and passes in 0.8.10 (where the bug was fixed). So that's good.
I'd prefer something that eliminates the race condition rather than accommodates it, but that may not be possible. And if it is possible, someone can do it in a subsequent PR.
Trott commented Jun 30, 2017
| fs.unwatchFile(filename); | ||
| })); | ||
| setTimeout(fs.unlinkSync,300,filename); |
jaimecbernardo commented Jul 1, 2017
Trott commented Jul 1, 2017
refack commented Jul 2, 2017
Asking for context: AFAICT the test needs to |
Trott commented Jul 2, 2017
@refack I'm not sure I understand your question, but I think this is what's going on so maybe this helps?: |
Trott commented Jul 2, 2017
(In other words, I don't think the |
AFAICT the original bug would happen if |
jaimecbernardo commented Jul 3, 2017
@refack From my understanding, the test is a regression test that is there to verify that there isn't a regression in this issue nodejs/node-v0.x-archive#4027 which is fixed in nodejs/node-v0.x-archive@39a0836, which landed in https://github.com/nodejs/node-v0.x-archive/tree/v0.8.10-release . So, @Trott ran the test in |
Trott commented Jul 3, 2017
@jaimecbernardo I think @refack groks all that. He's asking about the original bug and requirements for triggering it. I didn't look closely at it, just empirically tested to see that the test still functions as expected. So I don't have an answer for him. But that shouldn't stop this from landing. He's looking for a deep dive into the original bug, not how the test works. (And he'll correct me if I'm wrong about any of this! :-D ) |
Trott commented Jul 3, 2017
Landed in f1c890a. Thanks for the contribution! 🎉 |
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
The sequential/test-regress-GH-4027 test is flaky with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved. After increasing the delay before unlinking and calling setTimeout after watchFile, the flakiness stopped reproducing. PR-URL: #14010Fixes: #13800 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
7 participants
The sequential/test-regress-GH-4027 test is flaky on Windows CI with an increased system load, failing when the watched file is unlinked before the first state of the watched file is retrieved.
After increasing the delay before unlinking and calling
setTimeoutafterwatchFile, the flakiness stopped reproducing.Fixes: #13800
/cc @nodejs/testing
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test