Skip to content

Commit c176015

Browse 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

3 files changed

Lines changed: 184 additions & 1 deletion

File tree

‎lib/internal/watch_mode/files_watcher.js‎

Lines 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);

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Flags: --expose-internals
2+
import*ascommonfrom'../common/index.mjs';
3+
import{describe,it}from'node:test';
4+
importassertfrom'node:assert';
5+
import{spawn}from'node:child_process';
6+
import{writeFileSync}from'node:fs';
7+
importtmpdirfrom'../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+
constfixtureContent={
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+
constfixturePaths=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+
constchild=spawn(process.execPath,
40+
['--watch','--test'],
41+
{encoding: 'utf8',stdio: 'pipe',cwd: tmpdir.path});
42+
43+
letcurrentRunOutput='';
44+
consttestRuns=[];
45+
46+
constfirstRunCompleted=Promise.withResolvers();
47+
constsecondRunCompleted=Promise.withResolvers();
48+
constthirdRunCompleted=Promise.withResolvers();
49+
constfourthRunCompleted=Promise.withResolvers();
50+
51+
child.stdout.on('data',(data)=>{
52+
conststr=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+
case1:
61+
firstRunCompleted.resolve();
62+
break;
63+
case2:
64+
secondRunCompleted.resolve();
65+
break;
66+
case3:
67+
thirdRunCompleted.resolve();
68+
break;
69+
case4:
70+
fourthRunCompleted.resolve();
71+
break;
72+
}
73+
}
74+
});
75+
76+
// Wait for the initial test run to complete
77+
awaitfirstRunCompleted.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+
awaitsecondRunCompleted.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+
awaitthirdRunCompleted.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+
awaitfourthRunCompleted.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+
constfirstRunOutput=testRuns[0];
108+
assert.match(firstRunOutput,/firsttesthasran/);
109+
assert.match(firstRunOutput,/secondtesthasran/);
110+
111+
// Second test run - We have modified 'dependency.js' only, so both tests should re-run
112+
constsecondRunOutput=testRuns[1];
113+
assert.match(secondRunOutput,/firsttesthasran/);
114+
assert.match(secondRunOutput,/secondtesthasran/);
115+
116+
// Third test run - We have modified 'test.js' only
117+
constthirdRunOutput=testRuns[2];
118+
assert.match(thirdRunOutput,/firsttesthasranagain/);
119+
assert.doesNotMatch(thirdRunOutput,/secondtesthasran/);
120+
121+
// Fourth test run - We have modified 'dependency.js' again, so both tests should re-run
122+
constfourthRunOutput=testRuns[3];
123+
assert.match(fourthRunOutput,/firsttesthasranagain/);
124+
assert.match(fourthRunOutput,/secondtesthasran/);
125+
});
126+
});

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Flags: --expose-internals
2+
import*ascommonfrom'../common/index.mjs';
3+
import{describe,it}from'node:test';
4+
importassertfrom'node:assert';
5+
importtmpdirfrom'../common/tmpdir.js';
6+
importwatcherfrom'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+
constfixtureContent={
23+
'dependency.js': 'module.exports = {};',
24+
'test.js': 'require(\'./dependency.js\');',
25+
'test-2.js': 'require(\'./dependency.js\');',
26+
};
27+
28+
constfixturePaths=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+
constcontroller=newAbortController();
37+
constwatcher=newFilesWatcher({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)