Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c176015

Browse filesBrowse files
Ceres6pmarchini
authored and
ruyadorno
committed
fs: prevent unwanted dependencyOwners removal
Remove files from watcher `dependencyOwners` on file change only if it has no other owners. Co-authored-by: Pietro Marchini <pietro.marchini94@gmail.com> PR-URL: #55565 Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 363e7d5 commit c176015
Copy full SHA for c176015

3 files changed

+184-1Lines changed: 184 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/watch_mode/files_watcher.js‎

Copy file name to clipboardExpand all lines: lib/internal/watch_mode/files_watcher.js
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,10 @@ class FilesWatcher extends EventEmitter {
180180
owners.forEach((owner) => {
181181
this.#ownerDependencies.get(owner)?.forEach((dependency) => {
182182
this.#filteredFiles.delete(dependency);
183-
this.#dependencyOwners.delete(dependency);
183+
this.#dependencyOwners.get(dependency)?.delete(owner);
184+
if (this.#dependencyOwners.get(dependency)?.size === 0) {
185+
this.#dependencyOwners.delete(dependency);
186+
}
184187
});
185188
this.#filteredFiles.delete(owner);
186189
this.#dependencyOwners.delete(owner);
Collapse file

‎test/parallel/test-runner-complex-dependencies.mjs‎

Copy file name to clipboard
+126Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Flags: --expose-internals
2+
import * as common from '../common/index.mjs';
3+
import { describe, it } from 'node:test';
4+
import assert from 'node:assert';
5+
import { spawn } from 'node:child_process';
6+
import { writeFileSync } from 'node:fs';
7+
import tmpdir from '../common/tmpdir.js';
8+
9+
if (common.isIBMi)
10+
common.skip('IBMi does not support `fs.watch()`');
11+
12+
if (common.isAIX)
13+
common.skip('folder watch capability is limited in AIX.');
14+
15+
tmpdir.refresh();
16+
17+
// Set up test files and dependencies
18+
const fixtureContent = {
19+
'dependency.js': 'module.exports = {};',
20+
'test.js': `
21+
const test = require('node:test');
22+
require('./dependency.js');
23+
test('first test has ran');`,
24+
'test-2.js': `
25+
const test = require('node:test');
26+
require('./dependency.js');
27+
test('second test has ran');`,
28+
};
29+
30+
const fixturePaths = Object.fromEntries(Object.keys(fixtureContent)
31+
.map((file) => [file, tmpdir.resolve(file)]));
32+
33+
Object.entries(fixtureContent)
34+
.forEach(([file, content]) => writeFileSync(fixturePaths[file], content));
35+
36+
describe('test runner watch mode with more complex setup', () => {
37+
it('should re-run appropriate tests when dependencies change', async () => {
38+
// Start the test runner in watch mode
39+
const child = spawn(process.execPath,
40+
['--watch', '--test'],
41+
{ encoding: 'utf8', stdio: 'pipe', cwd: tmpdir.path });
42+
43+
let currentRunOutput = '';
44+
const testRuns = [];
45+
46+
const firstRunCompleted = Promise.withResolvers();
47+
const secondRunCompleted = Promise.withResolvers();
48+
const thirdRunCompleted = Promise.withResolvers();
49+
const fourthRunCompleted = Promise.withResolvers();
50+
51+
child.stdout.on('data', (data) => {
52+
const str = data.toString();
53+
currentRunOutput += str;
54+
55+
if (/duration_ms\s\d+/.test(str)) {
56+
// Test run has completed
57+
testRuns.push(currentRunOutput);
58+
currentRunOutput = '';
59+
switch (testRuns.length) {
60+
case 1:
61+
firstRunCompleted.resolve();
62+
break;
63+
case 2:
64+
secondRunCompleted.resolve();
65+
break;
66+
case 3:
67+
thirdRunCompleted.resolve();
68+
break;
69+
case 4:
70+
fourthRunCompleted.resolve();
71+
break;
72+
}
73+
}
74+
});
75+
76+
// Wait for the initial test run to complete
77+
await firstRunCompleted.promise;
78+
79+
// Modify 'dependency.js' to trigger re-run of both tests
80+
writeFileSync(fixturePaths['dependency.js'], 'module.exports = { modified: true };');
81+
82+
// Wait for the second test run to complete
83+
await secondRunCompleted.promise;
84+
85+
// Modify 'test.js' to trigger re-run of only 'test.js'
86+
writeFileSync(fixturePaths['test.js'], `
87+
const test = require('node:test');
88+
require('./dependency.js');
89+
test('first test has ran again');`);
90+
91+
// Wait for the third test run to complete
92+
await thirdRunCompleted.promise;
93+
94+
// Modify 'dependency.js' again to trigger re-run of both tests
95+
writeFileSync(fixturePaths['dependency.js'], 'module.exports = { modified: true, again: true };');
96+
97+
// Wait for the fourth test run to complete
98+
await fourthRunCompleted.promise;
99+
100+
// Kill the child process
101+
child.kill();
102+
103+
// Analyze the test runs
104+
assert.strictEqual(testRuns.length, 4);
105+
106+
// First test run - Both tests should run
107+
const firstRunOutput = testRuns[0];
108+
assert.match(firstRunOutput, /first test has ran/);
109+
assert.match(firstRunOutput, /second test has ran/);
110+
111+
// Second test run - We have modified 'dependency.js' only, so both tests should re-run
112+
const secondRunOutput = testRuns[1];
113+
assert.match(secondRunOutput, /first test has ran/);
114+
assert.match(secondRunOutput, /second test has ran/);
115+
116+
// Third test run - We have modified 'test.js' only
117+
const thirdRunOutput = testRuns[2];
118+
assert.match(thirdRunOutput, /first test has ran again/);
119+
assert.doesNotMatch(thirdRunOutput, /second test has ran/);
120+
121+
// Fourth test run - We have modified 'dependency.js' again, so both tests should re-run
122+
const fourthRunOutput = testRuns[3];
123+
assert.match(fourthRunOutput, /first test has ran again/);
124+
assert.match(fourthRunOutput, /second test has ran/);
125+
});
126+
});
Collapse file

‎test/parallel/test-watch-file-shared-dependency.mjs‎

Copy file name to clipboard
+54Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Flags: --expose-internals
2+
import * as common from '../common/index.mjs';
3+
import { describe, it } from 'node:test';
4+
import assert from 'node:assert';
5+
import tmpdir from '../common/tmpdir.js';
6+
import watcher from 'internal/watch_mode/files_watcher';
7+
import { writeFileSync } from 'node:fs';
8+
9+
if (common.isIBMi)
10+
common.skip('IBMi does not support `fs.watch()`');
11+
12+
if (common.isAIX)
13+
common.skip('folder watch capability is limited in AIX.');
14+
15+
tmpdir.refresh();
16+
17+
const { FilesWatcher } = watcher;
18+
19+
tmpdir.refresh();
20+
21+
// Set up test files and dependencies
22+
const fixtureContent = {
23+
'dependency.js': 'module.exports = {};',
24+
'test.js': 'require(\'./dependency.js\');',
25+
'test-2.js': 'require(\'./dependency.js\');',
26+
};
27+
28+
const fixturePaths = Object.fromEntries(Object.keys(fixtureContent)
29+
.map((file) => [file, tmpdir.resolve(file)]));
30+
31+
Object.entries(fixtureContent)
32+
.forEach(([file, content]) => writeFileSync(fixturePaths[file], content));
33+
34+
describe('watch file with shared dependency', () => {
35+
it('should not remove shared dependencies when unfiltering an owner', () => {
36+
const controller = new AbortController();
37+
const watcher = new FilesWatcher({ signal: controller.signal, debounce: 200 });
38+
39+
watcher.on('changed', ({ owners }) => {
40+
assert.strictEqual(owners.size, 2);
41+
assert.ok(owners.has(fixturePaths['test.js']));
42+
assert.ok(owners.has(fixturePaths['test-2.js']));
43+
controller.abort();
44+
});
45+
watcher.filterFile(fixturePaths['test.js']);
46+
watcher.filterFile(fixturePaths['test-2.js']);
47+
watcher.filterFile(fixturePaths['dependency.js'], fixturePaths['test.js']);
48+
watcher.filterFile(fixturePaths['dependency.js'], fixturePaths['test-2.js']);
49+
watcher.unfilterFilesOwnedBy([fixturePaths['test.js']]);
50+
watcher.filterFile(fixturePaths['test.js']);
51+
watcher.filterFile(fixturePaths['dependency.js'], fixturePaths['test.js']);
52+
writeFileSync(fixturePaths['dependency.js'], 'module.exports = { modified: true };');
53+
});
54+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.