test: refactor fs-watch tests due to macOS issue

In `macOS`, fsevents generated immediately before start watching may
leak into the event callback. See: https://github.com/nodejs/node/issues/54450
for an explanation. This might be fixed at some point in `libuv` though
it may take some time (see: https://github.com/libuv/libuv/issues/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: https://github.com/nodejs/node/pull/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>
This commit is contained in:
Santiago Gimeno 2024-09-06 19:38:28 +02:00 committed by GitHub
parent bec80892b3
commit b345118e1e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 140 additions and 64 deletions

View File

@ -8,6 +8,7 @@ const { watch } = require('fs/promises');
const fs = require('fs');
const assert = require('assert');
const { join } = require('path');
const { setTimeout } = require('timers/promises');
const tmpdir = require('../common/tmpdir');
class WatchTestCase {
@ -49,6 +50,12 @@ for (const testCase of kCases) {
let interval;
async function test() {
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
await setTimeout(common.platformTimeout(100));
}
const watcher = watch(testCase[testCase.field]);
for await (const { eventType, filename } of watcher) {
clearInterval(interval);

View File

@ -31,24 +31,34 @@ const filePath = path.join(testDirectory, 'folder-3');
const childrenFile = 'file-4.txt';
const childrenAbsolutePath = path.join(filePath, childrenFile);
const childrenRelativePath = path.join(path.basename(filePath), childrenFile);
const watcher = fs.watch(testDirectory, { recursive: true });
let watcherClosed = false;
watcher.on('change', function(event, filename) {
assert.strictEqual(event, 'rename');
assert.ok(filename === path.basename(filePath) || filename === childrenRelativePath);
if (filename === childrenRelativePath) {
watcher.close();
watcherClosed = true;
}
});
function doWatch() {
const watcher = fs.watch(testDirectory, { recursive: true });
watcher.on('change', function(event, filename) {
assert.strictEqual(event, 'rename');
assert.ok(filename === path.basename(filePath) || filename === childrenRelativePath);
// Do the write with a delay to ensure that the OS is ready to notify us.
setTimeout(() => {
fs.mkdirSync(filePath);
fs.writeFileSync(childrenAbsolutePath, 'world');
}, common.platformTimeout(200));
if (filename === childrenRelativePath) {
watcher.close();
watcherClosed = true;
}
});
// Do the write with a delay to ensure that the OS is ready to notify us.
setTimeout(() => {
fs.mkdirSync(filePath);
fs.writeFileSync(childrenAbsolutePath, 'world');
}, common.platformTimeout(200));
}
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(doWatch, common.platformTimeout(100));
} else {
doWatch();
}
process.once('exit', function() {
assert(watcherClosed, 'watcher Object was not closed');

View File

@ -35,6 +35,11 @@ tmpdir.refresh();
const symlinkFolder = path.join(rootDirectory, 'symlink-folder');
fs.symlinkSync(rootDirectory, symlinkFolder);
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
await setTimeout(common.platformTimeout(100));
}
const watcher = fs.watch(rootDirectory, { recursive: true });
let watcherClosed = false;
@ -74,6 +79,12 @@ tmpdir.refresh();
const forbiddenFile = path.join(subDirectory, 'forbidden.txt');
const acceptableFile = path.join(trackingSubDirectory, 'acceptable.txt');
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
await setTimeout(common.platformTimeout(100));
}
const watcher = fs.watch(trackingSubDirectory, { recursive: true });
let watcherClosed = false;
watcher.on('change', function(event, filename) {

View File

@ -25,14 +25,24 @@ const keepalive = setTimeout(() => {
throw new Error('timed out');
}, common.platformTimeout(30_000));
const watcher = watch(tmpDir, { recursive: true }, common.mustCall((eventType, _filename) => {
clearTimeout(keepalive);
watcher.close();
assert.strictEqual(eventType, 'rename');
assert.strictEqual(join(tmpDir, _filename), filename);
}));
function doWatch() {
const watcher = watch(tmpDir, { recursive: true }, common.mustCall((eventType, _filename) => {
clearTimeout(keepalive);
watcher.close();
assert.strictEqual(eventType, 'rename');
assert.strictEqual(join(tmpDir, _filename), filename);
}));
// Do the write with a delay to ensure that the OS is ready to notify us.
setTimeout(() => {
writeFileSync(filename, 'foobar2');
}, common.platformTimeout(200));
// Do the write with a delay to ensure that the OS is ready to notify us.
setTimeout(() => {
writeFileSync(filename, 'foobar2');
}, common.platformTimeout(200));
}
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(doWatch, common.platformTimeout(100));
} else {
doWatch();
}

View File

@ -41,13 +41,7 @@ const cases = [
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
for (const testCase of cases) {
if (testCase.shouldSkip) continue;
fs.mkdirSync(testCase.dirPath);
// Long content so it's actually flushed.
const content1 = Date.now() + testCase.fileName.toLowerCase().repeat(1e4);
fs.writeFileSync(testCase.filePath, content1);
function doWatchTest(testCase) {
let interval;
const pathToWatch = testCase[testCase.field];
const watcher = fs.watch(pathToWatch);
@ -87,6 +81,23 @@ for (const testCase of cases) {
}, 100);
}
for (const testCase of cases) {
if (testCase.shouldSkip) continue;
fs.mkdirSync(testCase.dirPath);
// Long content so it's actually flushed.
const content1 = Date.now() + testCase.fileName.toLowerCase().repeat(1e4);
fs.writeFileSync(testCase.filePath, content1);
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(() => {
doWatchTest(testCase);
}, common.platformTimeout(100));
} else {
doWatchTest(testCase);
}
}
[false, 1, {}, [], null, undefined].forEach((input) => {
assert.throws(
() => fs.watch(input, common.mustNotCall()),

View File

@ -86,10 +86,7 @@ watcher.on('stop', common.mustCall());
// Omitting AIX. It works but not reliably.
if (common.isLinux || common.isMacOS || common.isWindows) {
const dir = tmpdir.resolve('watch');
fs.mkdir(dir, common.mustCall(function(err) {
if (err) assert.fail(err);
function doWatch() {
const handle = fs.watch(dir, common.mustCall(function(eventType, filename) {
clearInterval(interval);
handle.close();
@ -101,5 +98,15 @@ if (common.isLinux || common.isMacOS || common.isWindows) {
if (err) assert.fail(err);
}));
}, 1);
}
fs.mkdir(dir, common.mustSucceed(() => {
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(doWatch, common.platformTimeout(100));
} else {
doWatch();
}
}));
}

View File

@ -38,14 +38,24 @@ const filepath = path.join(testsubdir, 'watch.txt');
fs.mkdirSync(testsubdir, 0o700);
const watcher = fs.watch(testDir, { persistent: true }, (event, filename) => {
// This function may be called with the directory depending on timing but
// must not be called with the file..
assert.strictEqual(filename, 'testsubdir');
});
setTimeout(() => {
fs.writeFileSync(filepath, 'test');
}, 100);
setTimeout(() => {
watcher.close();
}, 500);
function doWatch() {
const watcher = fs.watch(testDir, { persistent: true }, (event, filename) => {
// This function may be called with the directory depending on timing but
// must not be called with the file..
assert.strictEqual(filename, 'testsubdir');
});
setTimeout(() => {
fs.writeFileSync(filepath, 'test');
}, 100);
setTimeout(() => {
watcher.close();
}, 500);
}
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(doWatch, common.platformTimeout(100));
} else {
doWatch();
}

View File

@ -95,24 +95,34 @@ function repeat(fn) {
const testsubdir = fs.mkdtempSync(testDir + path.sep);
const filepath = path.join(testsubdir, 'newfile.txt');
const watcher =
fs.watch(testsubdir, common.mustCall(function(event, filename) {
const renameEv = common.isSunOS || common.isAIX ? 'change' : 'rename';
assert.strictEqual(event, renameEv);
if (expectFilePath) {
assert.strictEqual(filename, 'newfile.txt');
} else {
assert.strictEqual(filename, null);
}
clearInterval(interval);
watcher.close();
}));
function doWatch() {
const watcher =
fs.watch(testsubdir, common.mustCall(function(event, filename) {
const renameEv = common.isSunOS || common.isAIX ? 'change' : 'rename';
assert.strictEqual(event, renameEv);
if (expectFilePath) {
assert.strictEqual(filename, 'newfile.txt');
} else {
assert.strictEqual(filename, null);
}
clearInterval(interval);
watcher.close();
}));
const interval = repeat(() => {
fs.rmSync(filepath, { force: true });
const fd = fs.openSync(filepath, 'w');
fs.closeSync(fd);
});
const interval = repeat(() => {
fs.rmSync(filepath, { force: true });
const fd = fs.openSync(filepath, 'w');
fs.closeSync(fd);
});
}
if (common.isMacOS) {
// On macOS delay watcher start to avoid leaking previous events.
// Refs: https://github.com/libuv/libuv/pull/4503
setTimeout(doWatch, common.platformTimeout(100));
} else {
doWatch();
}
}
// https://github.com/joyent/node/issues/2293 - non-persistent watcher should