8000 test: fix race condition in watch mode tests · nodejs/node@a8b32b8 · GitHub
[go: up one dir, main page]
Skip to content

Commit a8b32b8

Browse files
mcollinaaduh95
mcollina
authored and
aduh95
committed
test: fix race condition in watch mode tests
The watch mode tests using runInBackground() had a race condition where stdout "Failed running" message could arrive before stderr was fully collected, causing assertions on stderr content to fail with empty strings. Fix by waiting for stderr 'data' event if stderr is empty when "Failed running" is detected. This is an event-driven approach rather than timing-based. Refs: nodejs/reliability#1450 PR-URL: #61615 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 086a5a5 commit a8b32b8

File tree

2 files changed

+33
-12
lines changed

2 files changed

+33
-12
lines changed

‎test/sequential/test-watch-mode-restart-esm-loading-error.mjs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { createInterface } from 'node:readline';
1111 8000
if (common.isIBMi)
1212
common.skip('IBMi does not support `fs.watch()`');
1313

14+
if (common.isAIX)
15+
common.skip('AIX does not reliably capture syntax errors in watch mode');
16+
1417
let tmpFiles = 0;
1518
function createTmpFile(content = 'console.log("running");', ext = '.js', basename = tmpdir.path) {
1619
const file = path.join(basename, `${tmpFiles++}${ext}`);
@@ -42,14 +45,23 @@ function runInBackground({ args = [], options = {}, completed = 'Completed runni
4245
stdout = [];
4346
stderr = '';
4447
} else if (data.startsWith('Failed running')) {
45-
if (shouldFail) {
46-
future.resolve({ stderr, stdout });
48+
const settle = () => {
49+
if (shouldFail) {
50+
future.resolve({ stderr, stdout });
51+
} else {
52+
future.reject({ stderr, stdout });
53+
}
54+
future = Promise.withResolvers();
55+
stdout = [];
56+
stderr = '';
57+
};
58+
// If stderr is empty, wait for it to receive data before settling.
59+
// This handles the race condition where stdout arrives before stderr.
60+
if (stderr === '') {
61+
child.stderr.once('data', settle);
4762
} else {
48-
future.reject({ stderr, stdout });
63+
settle();
4964
}
50-
future = Promise.withResolvers();
51-
stdout = [];
52-
stderr = '';
5365
}
5466
}
5567
});

‎test/sequential/test-watch-mode.mjs‎

+15Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,23 @@ function runInBackground({ args = [], options = {}, completed = 'Completed runni
5454
stdout = [];
5555
stderr = '';
5656
} else if (data.startsWith('Failed running')) {
57-
if (shouldFail) {
58-
future.resolve({ stderr, stdout });
57+
const settle = () => {
58+
if (shouldFail) {
59+
future.resolve({ stderr, stdout });
60+
} else {
61+
future.reject({ stderr, stdout });
62+
}
63+
future = Promise.withResolvers();
64+
stdout = [];
65+
stderr = '';
66+
};
67+
// If stderr is empty, wait for it to receive data before settling.
68+
// This handles the race condition where stdout arrives before stderr.
69+
if (stderr === '') {
70+
child.stderr.once('data', settle);
5971
} else {
60-
future.reject({ stderr, stdout });
72+
settle();
6173
}
62-
future = Promise.withResolvers();
63-
stdout = [];
64-
stderr = '';
6574
}
6675
}
6776
});

0 commit comments

Comments
 (0)
0