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 a8b32b8

Browse filesBrowse files
mcollinaaduh95
authored andcommitted
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
Copy full SHA for a8b32b8

2 files changed

+33-12Lines changed: 33 additions & 12 deletions

File tree

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

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

Copy file name to clipboardExpand all lines: test/sequential/test-watch-mode-restart-esm-loading-error.mjs
+18-6Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { createInterface } from 'node:readline';
1111
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
});
Collapse file

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

Copy file name to clipboardExpand all lines: test/sequential/test-watch-mode.mjs
+15-6Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.