Skip to content

Commit 4f82673

Browse files
santigimenotargos
authored andcommitted
test: refactor fs-watch tests due to macOS issue
In `macOS`, fsevents generated immediately before start watching may leak into the event callback. See: #54450 for an explanation. This might be fixed at some point in `libuv` though it may take some time (see: libuv/libuv#3866). This commit comes in anticipation of the soon-to-be-released `libuv@1.49.0` which was making these tests very flaky. PR-URL: #54498 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent 62953ef commit 4f82673

8 files changed

+142
-66
lines changed

test/parallel/test-fs-promises-watch.js

+7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { watch } = require('fs/promises');
88
const fs = require('fs');
99
const assert = require('assert');
1010
const { join } = require('path');
11+
const { setTimeout } = require('timers/promises');
1112
const tmpdir = require('../common/tmpdir');
1213

1314
class WatchTestCase {
@@ -49,6 +50,12 @@ for (const testCase of kCases) {
4950

5051
let interval;
5152
async function test() {
53+
if (common.isMacOS) {
54+
// On macOS delay watcher start to avoid leaking previous events.
55+
// Refs: https://github.com/libuv/libuv/pull/4503
56+
await setTimeout(common.platformTimeout(100));
57+
}
58+
5259
const watcher = watch(testCase[testCase.field]);
5360
for await (const { eventType, filename } of watcher) {
5461
clearInterval(interval);

test/parallel/test-fs-watch-recursive-add-file-to-new-folder.js

+26-16
Original file line numberDiff line numberDiff line change
@@ -31,24 +31,34 @@ const filePath = path.join(testDirectory, 'folder-3');
3131
const childrenFile = 'file-4.txt';
3232
const childrenAbsolutePath = path.join(filePath, childrenFile);
3333
const childrenRelativePath = path.join(path.basename(filePath), childrenFile);
34-
35-
const watcher = fs.watch(testDirectory, { recursive: true });
3634
let watcherClosed = false;
37-
watcher.on('change', function(event, filename) {
38-
assert.strictEqual(event, 'rename');
39-
assert.ok(filename === path.basename(filePath) || filename === childrenRelativePath);
40-
41-
if (filename === childrenRelativePath) {
42-
watcher.close();
43-
watcherClosed = true;
44-
}
45-
});
4635

47-
// Do the write with a delay to ensure that the OS is ready to notify us.
48-
setTimeout(() => {
49-
fs.mkdirSync(filePath);
50-
fs.writeFileSync(childrenAbsolutePath, 'world');
51-
}, common.platformTimeout(200));
36+
function doWatch() {
37+
const watcher = fs.watch(testDirectory, { recursive: true });
38+
watcher.on('change', function(event, filename) {
39+
assert.strictEqual(event, 'rename');
40+
assert.ok(filename === path.basename(filePath) || filename === childrenRelativePath);
41+
42+
if (filename === childrenRelativePath) {
43+
watcher.close();
44+
watcherClosed = true;
45+
}
46+
});
47+
48+
// Do the write with a delay to ensure that the OS is ready to notify us.
49+
setTimeout(() => {
50+
fs.mkdirSync(filePath);
51+
fs.writeFileSync(childrenAbsolutePath, 'world');
52+
}, common.platformTimeout(200));
53+
}
54+
55+
if (common.isMacOS) {
56+
// On macOS delay watcher start to avoid leaking previous events.
57+
// Refs: https://github.com/libuv/libuv/pull/4503
58+
setTimeout(doWatch, common.platformTimeout(100));
59+
} else {
60+
doWatch();
61+
}
5262

5363
process.once('exit', function() {
5464
assert(watcherClosed, 'watcher Object was not closed');

test/parallel/test-fs-watch-recursive-symlink.js

+11
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ tmpdir.refresh();
3535
const symlinkFolder = path.join(rootDirectory, 'symlink-folder');
3636
fs.symlinkSync(rootDirectory, symlinkFolder);
3737

38+
if (common.isMacOS) {
39+
// On macOS delay watcher start to avoid leaking previous events.
40+
// Refs: https://github.com/libuv/libuv/pull/4503
41+
await setTimeout(common.platformTimeout(100));
42+
}
3843

3944
const watcher = fs.watch(rootDirectory, { recursive: true });
4045
let watcherClosed = false;
@@ -74,6 +79,12 @@ tmpdir.refresh();
7479
const forbiddenFile = path.join(subDirectory, 'forbidden.txt');
7580
const acceptableFile = path.join(trackingSubDirectory, 'acceptable.txt');
7681

82+
if (common.isMacOS) {
83+
// On macOS delay watcher start to avoid leaking previous events.
84+
// Refs: https://github.com/libuv/libuv/pull/4503
85+
await setTimeout(common.platformTimeout(100));
86+
}
87+
7788
const watcher = fs.watch(trackingSubDirectory, { recursive: true });
7889
let watcherClosed = false;
7990
watcher.on('change', function(event, filename) {

test/parallel/test-fs-watch-recursive-sync-write.js

+21-11
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,24 @@ const keepalive = setTimeout(() => {
2525
throw new Error('timed out');
2626
}, common.platformTimeout(30_000));
2727

28-
const watcher = watch(tmpDir, { recursive: true }, common.mustCall((eventType, _filename) => {
29-
clearTimeout(keepalive);
30-
watcher.close();
31-
assert.strictEqual(eventType, 'rename');
32-
assert.strictEqual(join(tmpDir, _filename), filename);
33-
}));
34-
35-
// Do the write with a delay to ensure that the OS is ready to notify us.
36-
setTimeout(() => {
37-
writeFileSync(filename, 'foobar2');
38-
}, common.platformTimeout(200));
28+
function doWatch() {
29+
const watcher = watch(tmpDir, { recursive: true }, common.mustCall((eventType, _filename) => {
30+
clearTimeout(keepalive);
31+
watcher.close();
32+
assert.strictEqual(eventType, 'rename');
33+
assert.strictEqual(join(tmpDir, _filename), filename);
34+
}));
35+
36+
// Do the write with a delay to ensure that the OS is ready to notify us.
37+
setTimeout(() => {
38+
writeFileSync(filename, 'foobar2');
39+
}, common.platformTimeout(200));
40+
}
41+
42+
if (common.isMacOS) {
43+
// On macOS delay watcher start to avoid leaking previous events.
44+
// Refs: https://github.com/libuv/libuv/pull/4503
45+
setTimeout(doWatch, common.platformTimeout(100));
46+
} else {
47+
doWatch();
48+
}

test/parallel/test-fs-watch.js

+18-7
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,7 @@ const cases = [
4141
const tmpdir = require('../common/tmpdir');
4242
tmpdir.refresh();
4343

44-
for (const testCase of cases) {
45-
if (testCase.shouldSkip) continue;
46-
fs.mkdirSync(testCase.dirPath);
47-
// Long content so it's actually flushed.
48-
const content1 = Date.now() + testCase.fileName.toLowerCase().repeat(1e4);
49-
fs.writeFileSync(testCase.filePath, content1);
50-
44+
function doWatchTest(testCase) {
5145
let interval;
5246
const pathToWatch = testCase[testCase.field];
5347
const watcher = fs.watch(pathToWatch);
@@ -87,6 +81,23 @@ for (const testCase of cases) {
8781
}, 100);
8882
}
8983

84+
for (const testCase of cases) {
85+
if (testCase.shouldSkip) continue;
86+
fs.mkdirSync(testCase.dirPath);
87+
// Long content so it's actually flushed.
88+
const content1 = Date.now() + testCase.fileName.toLowerCase().repeat(1e4);
89+
fs.writeFileSync(testCase.filePath, content1);
90+
if (common.isMacOS) {
91+
// On macOS delay watcher start to avoid leaking previous events.
92+
// Refs: https://github.com/libuv/libuv/pull/4503
93+
setTimeout(() => {
94+
doWatchTest(testCase);
95+
}, common.platformTimeout(100));
96+
} else {
97+
doWatchTest(testCase);
98+
}
99+
}
100+
90101
[false, 1, {}, [], null, undefined].forEach((input) => {
91102
assert.throws(
92103
() => fs.watch(input, common.mustNotCall()),

test/parallel/test-fs-watchfile.js

+11-4
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,7 @@ watcher.on('stop', common.mustCall());
8686
// Omitting AIX. It works but not reliably.
8787
if (common.isLinux || common.isMacOS || common.isWindows) {
8888
const dir = tmpdir.resolve('watch');
89-
90-
fs.mkdir(dir, common.mustCall(function(err) {
91-
if (err) assert.fail(err);
92-
89+
function doWatch() {
9390
const handle = fs.watch(dir, common.mustCall(function(eventType, filename) {
9491
clearInterval(interval);
9592
handle.close();
@@ -101,5 +98,15 @@ if (common.isLinux || common.isMacOS || common.isWindows) {
10198
if (err) assert.fail(err);
10299
}));
103100
}, 1);
101+
}
102+
103+
fs.mkdir(dir, common.mustSucceed(() => {
104+
if (common.isMacOS) {
105+
// On macOS delay watcher start to avoid leaking previous events.
106+
// Refs: https://github.com/libuv/libuv/pull/4503
107+
setTimeout(doWatch, common.platformTimeout(100));
108+
} else {
109+
doWatch();
110+
}
104111
}));
105112
}

test/pummel/test-fs-watch-non-recursive.js

+21-11
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,24 @@ const filepath = path.join(testsubdir, 'watch.txt');
3838

3939
fs.mkdirSync(testsubdir, 0o700);
4040

41-
const watcher = fs.watch(testDir, { persistent: true }, (event, filename) => {
42-
// This function may be called with the directory depending on timing but
43-
// must not be called with the file..
44-
assert.strictEqual(filename, 'testsubdir');
45-
});
46-
setTimeout(() => {
47-
fs.writeFileSync(filepath, 'test');
48-
}, 100);
49-
setTimeout(() => {
50-
watcher.close();
51-
}, 500);
41+
function doWatch() {
42+
const watcher = fs.watch(testDir, { persistent: true }, (event, filename) => {
43+
// This function may be called with the directory depending on timing but
44+
// must not be called with the file..
45+
assert.strictEqual(filename, 'testsubdir');
46+
});
47+
setTimeout(() => {
48+
fs.writeFileSync(filepath, 'test');
49+
}, 100);
50+
setTimeout(() => {
51+
watcher.close();
52+
}, 500);
53+
}
54+
55+
if (common.isMacOS) {
56+
// On macOS delay watcher start to avoid leaking previous events.
57+
// Refs: https://github.com/libuv/libuv/pull/4503
58+
setTimeout(doWatch, common.platformTimeout(100));
59+
} else {
60+
doWatch();
61+
}

test/sequential/test-fs-watch.js

+27-17
Original file line numberDiff line numberDiff line change
@@ -95,24 +95,34 @@ function repeat(fn) {
9595
const testsubdir = fs.mkdtempSync(testDir + path.sep);
9696
const filepath = path.join(testsubdir, 'newfile.txt');
9797

98-
const watcher =
99-
fs.watch(testsubdir, common.mustCall(function(event, filename) {
100-
const renameEv = common.isSunOS || common.isAIX ? 'change' : 'rename';
101-
assert.strictEqual(event, renameEv);
102-
if (expectFilePath) {
103-
assert.strictEqual(filename, 'newfile.txt');
104-
} else {
105-
assert.strictEqual(filename, null);
106-
}
107-
clearInterval(interval);
108-
watcher.close();
109-
}));
98+
function doWatch() {
99+
const watcher =
100+
fs.watch(testsubdir, common.mustCall(function(event, filename) {
101+
const renameEv = common.isSunOS || common.isAIX ? 'change' : 'rename';
102+
assert.strictEqual(event, renameEv);
103+
if (expectFilePath) {
104+
assert.strictEqual(filename, 'newfile.txt');
105+
} else {
106+
assert.strictEqual(filename, null);
107+
}
108+
clearInterval(interval);
109+
watcher.close();
110+
}));
111+
112+
const interval = repeat(() => {
113+
fs.rmSync(filepath, { force: true });
114+
const fd = fs.openSync(filepath, 'w');
115+
fs.closeSync(fd);
116+
});
117+
}
110118

111-
const interval = repeat(() => {
112-
fs.rmSync(filepath, { force: true });
113-
const fd = fs.openSync(filepath, 'w');
114-
fs.closeSync(fd);
115-
});
119+
if (common.isMacOS) {
120+
// On macOS delay watcher start to avoid leaking previous events.
121+
// Refs: https://github.com/libuv/libuv/pull/4503
122+
setTimeout(doWatch, common.platformTimeout(100));
123+
} else {
124+
doWatch();
125+
}
116126
}
117127

118128
// https://github.com/joyent/node/issues/2293 - non-persistent watcher should

0 commit comments

Comments
 (0)