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
Stabilize watch tests #4318
Stabilize watch tests #4318
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#stabilize-watch-tests or load it into the REPL: |
ef1808d
to
cf3de24
Compare
Codecov Report
@@ Coverage Diff @@
## master #4318 +/- ##
=======================================
Coverage 98.44% 98.44%
=======================================
Files 205 205
Lines 7314 7314
Branches 2084 2084
=======================================
Hits 7200 7200
Misses 55 55
Partials 59 59 Continue to review full report at Codecov.
|
cf3de24
to
1a84b57
Compare
The node12/macos race condition test failure in this PR "correctly rewrites change event during build delay" happens roughly 20% of the time on github CI. Although not a proper fix, it might be helped by the timeout increase at the end of the patch #4300 (comment). I don't know why Lines 309 to 315 in 2a899d5
The implementation of |
might be time to replace I'll have a look into a PR. |
There are two predominant flaky tests on macos. This particular watch test
This is the other macos failure that occurs less than 10% of the time on macos/node12, which oddly is reported as 2 test failures by mocha when running
My initial suspicion that it was due to |
For the second watch failure above, this hack appears to yield positive results on an overloaded macos box: --- a/test/watch/index.js
+++ b/test/watch/index.js
@@ -309,29 +309,29 @@ describe('rollup.watch', () => {
it('correctly rewrites change event during build delay', async () => {
const WATCHED_ID = path.resolve('test/_tmp/input/watched');
const MAIN_ID = path.resolve('test/_tmp/input/main.js');
let lastEvent = null;
await sander.copydir('test/watch/samples/watch-files').to('test/_tmp/input');
await wait(100);
watcher = rollup.watch({
input: 'test/_tmp/input/main.js',
output: {
file: 'test/_tmp/output/bundle.js',
format: 'cjs',
exports: 'auto'
},
watch: {
- buildDelay: 300,
+ buildDelay: 600,
chokidar: {
atomic: false
}
},
plugins: {
buildStart() {
this.addWatchFile(WATCHED_ID);
},
watchChange(id, { event }) {
if (id === WATCHED_ID) {
assert.strictEqual(lastEvent, null);
lastEvent = event;
}
} |
I added the increased timeout. This is not fixed yet, but I will move it to master as it definitely appears to be an improvement. |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
#4300
Description
This attempts to follow ideas in #4300 (comment) and others to stabilize the watch tests especially on MacOS.