From 3abd71a0ea3a83f64fc0341333ddf729578158cc Mon Sep 17 00:00:00 2001
From: Moshe Atlow <moshe@atlow.co.il>
Date: Thu, 29 Sep 2022 12:30:34 +0300
Subject: [PATCH] test: deflake watch mode tests

PR-URL: https://github.com/nodejs/node/pull/44621
Backport-PR-URL: https://github.com/nodejs/node/pull/44815
Fixes: https://github.com/nodejs/node/issues/44655
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 .../fixtures/watch-mode/event_loop_blocked.js |   4 +
 test/fixtures/watch-mode/graceful-sigterm.js  |  17 --
 test/fixtures/watch-mode/infinite-loop.js     |   2 -
 test/parallel/test-watch-mode.mjs             | 237 ------------------
 test/sequential/test-watch-mode.mjs           | 223 ++++++++++++++++
 5 files changed, 227 insertions(+), 256 deletions(-)
 create mode 100644 test/fixtures/watch-mode/event_loop_blocked.js
 delete mode 100644 test/fixtures/watch-mode/graceful-sigterm.js
 delete mode 100644 test/fixtures/watch-mode/infinite-loop.js
 delete mode 100644 test/parallel/test-watch-mode.mjs
 create mode 100644 test/sequential/test-watch-mode.mjs

diff --git a/test/fixtures/watch-mode/event_loop_blocked.js b/test/fixtures/watch-mode/event_loop_blocked.js
new file mode 100644
index 00000000000000..e74b7fd389339c
--- /dev/null
+++ b/test/fixtures/watch-mode/event_loop_blocked.js
@@ -0,0 +1,4 @@
+console.log('running');
+Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0);
+console.log('don\'t show me');
+
diff --git a/test/fixtures/watch-mode/graceful-sigterm.js b/test/fixtures/watch-mode/graceful-sigterm.js
deleted file mode 100644
index d099b47b76f730..00000000000000
--- a/test/fixtures/watch-mode/graceful-sigterm.js
+++ /dev/null
@@ -1,17 +0,0 @@
-
-setInterval(() => {}, 1000);
-console.log('running');
-
-process.on('SIGTERM', () => {
-  setTimeout(() => {
-    console.log('exiting gracefully');
-    process.exit(0);
-  }, 1000);
-});
-
-process.on('SIGINT', () => {
-  setTimeout(() => {
-    console.log('exiting gracefully');
-    process.exit(0);
-  }, 1000);
-});
diff --git a/test/fixtures/watch-mode/infinite-loop.js b/test/fixtures/watch-mode/infinite-loop.js
deleted file mode 100644
index 56e92666e7cb1c..00000000000000
--- a/test/fixtures/watch-mode/infinite-loop.js
+++ /dev/null
@@ -1,2 +0,0 @@
-console.log('running');
-while(true) {};
diff --git a/test/parallel/test-watch-mode.mjs b/test/parallel/test-watch-mode.mjs
deleted file mode 100644
index 68d120d0796723..00000000000000
--- a/test/parallel/test-watch-mode.mjs
+++ /dev/null
@@ -1,237 +0,0 @@
-import * as common from '../common/index.mjs';
-import * as fixtures from '../common/fixtures.mjs';
-import tmpdir from '../common/tmpdir.js';
-import assert from 'node:assert';
-import path from 'node:path';
-import { execPath } from 'node:process';
-import { describe, it } from 'node:test';
-import { spawn } from 'node:child_process';
-import { writeFileSync, readFileSync } from 'node:fs';
-import { inspect } from 'node:util';
-import { once } from 'node:events';
-import { setTimeout } from 'node:timers/promises';
-
-if (common.isIBMi)
-  common.skip('IBMi does not support `fs.watch()`');
-
-async function spawnWithRestarts({
-  args,
-  file,
-  restarts,
-  startedPredicate,
-  restartMethod,
-}) {
-  args ??= [file];
-  const printedArgs = inspect(args.slice(args.indexOf(file)).join(' '));
-  startedPredicate ??= (data) => Boolean(data.match(new RegExp(`(Failed|Completed) running ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length);
-  restartMethod ??= () => writeFileSync(file, readFileSync(file));
-
-  let stderr = '';
-  let stdout = '';
-  let restartCount = 0;
-  let completedStart = false;
-  let finished = false;
-
-  const child = spawn(execPath, ['--watch', '--no-warnings', ...args], { encoding: 'utf8' });
-  child.stderr.on('data', (data) => {
-    stderr += data;
-  });
-  child.stdout.on('data', async (data) => {
-    if (finished) return;
-    stdout += data;
-    const restartMessages = stdout.match(new RegExp(`Restarting ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length ?? 0;
-    completedStart = completedStart || startedPredicate(data.toString());
-    if (restartMessages >= restarts && completedStart) {
-      finished = true;
-      child.kill();
-      return;
-    }
-    if (restartCount <= restartMessages && completedStart) {
-      await setTimeout(restartCount > 0 ? 1000 : 50, { ref: false }); // Prevent throttling
-      restartCount++;
-      completedStart = false;
-      restartMethod();
-    }
-  });
-
-  await Promise.race([once(child, 'exit'), once(child, 'error')]);
-  return { stderr, stdout };
-}
-
-let tmpFiles = 0;
-function createTmpFile(content = 'console.log("running");') {
-  const file = path.join(tmpdir.path, `${tmpFiles++}.js`);
-  writeFileSync(file, content);
-  return file;
-}
-
-function removeGraceMessage(stdout, file) {
-  // Remove the message in case restart took long to avoid flakiness
-  return stdout
-    .replaceAll('Waiting for graceful termination...', '')
-    .replaceAll(`Gracefully restarted ${inspect(file)}`, '');
-}
-
-tmpdir.refresh();
-
-// Warning: this suite can run safely with concurrency: true
-// only if tests do not watch/depend on the same files
-describe('watch mode', { concurrency: false, timeout: 60_0000 }, () => {
-  it('should watch changes to a file - event loop ended', async () => {
-    const file = createTmpFile();
-    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 1 });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [
-      'running', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`,
-      'running', `Completed running ${inspect(file)}`, '',
-    ].join('\n'));
-  });
-
-  it('should watch changes to a failing file', async () => {
-    const file = fixtures.path('watch-mode/failing.js');
-    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 1 });
-
-    assert.match(stderr, /Error: fails\r?\n/);
-    assert.strictEqual(stderr.match(/Error: fails\r?\n/g).length, 2);
-    assert.strictEqual(removeGraceMessage(stdout, file), [`Failed running ${inspect(file)}`, `Restarting ${inspect(file)}`,
-                                                          `Failed running ${inspect(file)}`, ''].join('\n'));
-  });
-
-  it('should not watch when running an non-existing file', async () => {
-    const file = fixtures.path('watch-mode/non-existing.js');
-    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 0, restartMethod: () => {} });
-
-    assert.match(stderr, /code: 'MODULE_NOT_FOUND'/);
-    assert.strictEqual(stdout, [`Failed running ${inspect(file)}`, ''].join('\n'));
-  });
-
-  it('should watch when running an non-existing file - when specified under --watch-path', {
-    skip: !common.isOSX && !common.isWindows
-  }, async () => {
-    const file = fixtures.path('watch-mode/subdir/non-existing.js');
-    const watched = fixtures.path('watch-mode/subdir/file.js');
-    const { stderr, stdout } = await spawnWithRestarts({
-      file,
-      args: ['--watch-path', fixtures.path('./watch-mode/subdir/'), file],
-      restarts: 1,
-      restartMethod: () => writeFileSync(watched, readFileSync(watched))
-    });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [`Failed running ${inspect(file)}`, `Restarting ${inspect(file)}`,
-                                                          `Failed running ${inspect(file)}`, ''].join('\n'));
-  });
-
-  it('should watch changes to a file - event loop blocked', async () => {
-    const file = fixtures.path('watch-mode/infinite-loop.js');
-    const { stderr, stdout } = await spawnWithRestarts({
-      file,
-      restarts: 2,
-      startedPredicate: (data) => data.startsWith('running'),
-    });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file),
-                       ['running', `Restarting ${inspect(file)}`, 'running', `Restarting ${inspect(file)}`, 'running', ''].join('\n'));
-  });
-
-  it('should watch changes to dependencies - cjs', async () => {
-    const file = fixtures.path('watch-mode/dependant.js');
-    const dependency = fixtures.path('watch-mode/dependency.js');
-    const { stderr, stdout } = await spawnWithRestarts({
-      file,
-      restarts: 1,
-      restartMethod: () => writeFileSync(dependency, readFileSync(dependency)),
-    });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [
-      '{}', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`,
-      '{}', `Completed running ${inspect(file)}`, '',
-    ].join('\n'));
-  });
-
-  it('should watch changes to dependencies - esm', async () => {
-    const file = fixtures.path('watch-mode/dependant.mjs');
-    const dependency = fixtures.path('watch-mode/dependency.mjs');
-    const { stderr, stdout } = await spawnWithRestarts({
-      file,
-      restarts: 1,
-      restartMethod: () => writeFileSync(dependency, readFileSync(dependency)),
-    });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [
-      '{}', `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`,
-      '{}', `Completed running ${inspect(file)}`, '',
-    ].join('\n'));
-  });
-
-  it('should restart multiple times', async () => {
-    const file = createTmpFile();
-    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 3 });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(stdout.match(new RegExp(`Restarting ${inspect(file).replace(/\\/g, '\\\\')}`, 'g')).length, 3);
-  });
-
-  it('should gracefully wait when restarting', { skip: common.isWindows }, async () => {
-    const file = fixtures.path('watch-mode/graceful-sigterm.js');
-    const { stderr, stdout } = await spawnWithRestarts({
-      file,
-      restarts: 1,
-      startedPredicate: (data) => data.startsWith('running'),
-    });
-
-    // This message appearing is very flaky depending on a race between the
-    // inner process and the outer process. it is acceptable for the message not to appear
-    // as long as the SIGTERM handler is respected.
-    if (stdout.includes('Waiting for graceful termination...')) {
-      assert.strictEqual(stdout, ['running', `Restarting ${inspect(file)}`, 'Waiting for graceful termination...',
-                                  'exiting gracefully', `Gracefully restarted ${inspect(file)}`, 'running', ''].join('\n'));
-    } else {
-      assert.strictEqual(stdout, ['running', `Restarting ${inspect(file)}`, 'exiting gracefully', 'running', ''].join('\n'));
-    }
-    assert.strictEqual(stderr, '');
-  });
-
-  it('should pass arguments to file', async () => {
-    const file = fixtures.path('watch-mode/parse_args.js');
-    const random = Date.now().toString();
-    const args = [file, '--random', random];
-    const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, args.join(' ')), [
-      random, `Completed running ${inspect(args.join(' '))}`, `Restarting ${inspect(args.join(' '))}`,
-      random, `Completed running ${inspect(args.join(' '))}`, '',
-    ].join('\n'));
-  });
-
-  it('should not load --require modules in main process', async () => {
-    const file = createTmpFile('');
-    const required = fixtures.path('watch-mode/process_exit.js');
-    const args = ['--require', required, file];
-    const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [
-      `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, `Completed running ${inspect(file)}`, '',
-    ].join('\n'));
-  });
-
-  it('should not load --import modules in main process', {
-    skip: 'enable once --import is backported',
-  }, async () => {
-    const file = createTmpFile('');
-    const imported = fixtures.fileURL('watch-mode/process_exit.js');
-    const args = ['--import', imported, file];
-    const { stderr, stdout } = await spawnWithRestarts({ file, args, restarts: 1 });
-
-    assert.strictEqual(stderr, '');
-    assert.strictEqual(removeGraceMessage(stdout, file), [
-      `Completed running ${inspect(file)}`, `Restarting ${inspect(file)}`, `Completed running ${inspect(file)}`, '',
-    ].join('\n'));
-  });
-});
diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs
new file mode 100644
index 00000000000000..92839814ba280a
--- /dev/null
+++ b/test/sequential/test-watch-mode.mjs
@@ -0,0 +1,223 @@
+import * as common from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import tmpdir from '../common/tmpdir.js';
+import assert from 'node:assert';
+import path from 'node:path';
+import { execPath } from 'node:process';
+import { describe, it } from 'node:test';
+import { spawn } from 'node:child_process';
+import { writeFileSync, readFileSync } from 'node:fs';
+import { inspect } from 'node:util';
+import { once } from 'node:events';
+
+if (common.isIBMi)
+  common.skip('IBMi does not support `fs.watch()`');
+
+function restart(file) {
+  // To avoid flakiness, we save the file repeatedly until test is done
+  writeFileSync(file, readFileSync(file));
+  const timer = setInterval(() => writeFileSync(file, readFileSync(file)), 1000);
+  return () => clearInterval(timer);
+}
+
+async function spawnWithRestarts({
+  args,
+  file,
+  watchedFile = file,
+  restarts = 1,
+  isReady,
+}) {
+  args ??= [file];
+  const printedArgs = inspect(args.slice(args.indexOf(file)).join(' '));
+  isReady ??= (data) => Boolean(data.match(new RegExp(`(Failed|Completed) running ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length);
+
+  let stderr = '';
+  let stdout = '';
+  let cancelRestarts;
+
+  const child = spawn(execPath, ['--watch', '--no-warnings', ...args], { encoding: 'utf8' });
+  child.stderr.on('data', (data) => {
+    stderr += data;
+  });
+  child.stdout.on('data', async (data) => {
+    stdout += data;
+    const restartsCount = stdout.match(new RegExp(`Restarting ${printedArgs.replace(/\\/g, '\\\\')}`, 'g'))?.length ?? 0;
+    if (restarts === 0 || !isReady(data.toString())) {
+      return;
+    }
+    if (restartsCount >= restarts) {
+      cancelRestarts?.();
+      child.kill();
+      return;
+    }
+    cancelRestarts ??= restart(watchedFile);
+  });
+
+  await once(child, 'exit');
+  cancelRestarts?.();
+  return { stderr, stdout };
+}
+
+let tmpFiles = 0;
+function createTmpFile(content = 'console.log("running");') {
+  const file = path.join(tmpdir.path, `${tmpFiles++}.js`);
+  writeFileSync(file, content);
+  return file;
+}
+
+function assertRestartedCorrectly({ stdout, messages: { inner, completed, restarted } }) {
+  const lines = stdout.split(/\r?\n/).filter(Boolean);
+
+  const start = [inner, completed, restarted].filter(Boolean);
+  const end = [restarted, inner, completed].filter(Boolean);
+  assert.deepStrictEqual(lines.slice(0, start.length), start);
+  assert.deepStrictEqual(lines.slice(-end.length), end);
+}
+
+tmpdir.refresh();
+
+// Warning: this suite can run safely with concurrency: true
+// only if tests do not watch/depend on the same files
+describe('watch mode', { concurrency: true, timeout: 60_0000 }, () => {
+  it('should watch changes to a file - event loop ended', async () => {
+    const file = createTmpFile();
+    const { stderr, stdout } = await spawnWithRestarts({ file });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { inner: 'running', completed: `Completed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },
+    });
+  });
+
+  it('should watch changes to a failing file', async () => {
+    const file = fixtures.path('watch-mode/failing.js');
+    const { stderr, stdout } = await spawnWithRestarts({ file });
+
+    assert.match(stderr, /Error: fails\r?\n/);
+    assert.strictEqual(stderr.match(/Error: fails\r?\n/g).length, 2);
+    assertRestartedCorrectly({
+      stdout,
+      messages: { completed: `Failed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },
+    });
+  });
+
+  it('should not watch when running an non-existing file', async () => {
+    const file = fixtures.path('watch-mode/non-existing.js');
+    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 0 });
+
+    assert.match(stderr, /code: 'MODULE_NOT_FOUND'/);
+    assert.strictEqual(stdout, `Failed running ${inspect(file)}\n`);
+  });
+
+  it('should watch when running an non-existing file - when specified under --watch-path', {
+    skip: !common.isOSX && !common.isWindows
+  }, async () => {
+    const file = fixtures.path('watch-mode/subdir/non-existing.js');
+    const watchedFile = fixtures.path('watch-mode/subdir/file.js');
+    const { stderr, stdout } = await spawnWithRestarts({
+      file,
+      watchedFile,
+      args: ['--watch-path', fixtures.path('./watch-mode/subdir/'), file],
+    });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { completed: `Failed running ${inspect(file)}`, restarted: `Restarting ${inspect(file)}` },
+    });
+  });
+
+  it('should watch changes to a file - event loop blocked', async () => {
+    const file = fixtures.path('watch-mode/event_loop_blocked.js');
+    const { stderr, stdout } = await spawnWithRestarts({
+      file,
+      isReady: (data) => data.startsWith('running'),
+    });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { inner: 'running', restarted: `Restarting ${inspect(file)}` },
+    });
+  });
+
+  it('should watch changes to dependencies - cjs', async () => {
+    const file = fixtures.path('watch-mode/dependant.js');
+    const dependency = fixtures.path('watch-mode/dependency.js');
+    const { stderr, stdout } = await spawnWithRestarts({
+      file,
+      watchedFile: dependency,
+    });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { inner: '{}', restarted: `Restarting ${inspect(file)}`, completed: `Completed running ${inspect(file)}` },
+    });
+  });
+
+  it('should watch changes to dependencies - esm', async () => {
+    const file = fixtures.path('watch-mode/dependant.mjs');
+    const dependency = fixtures.path('watch-mode/dependency.mjs');
+    const { stderr, stdout } = await spawnWithRestarts({
+      file,
+      watchedFile: dependency,
+    });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { inner: '{}', restarted: `Restarting ${inspect(file)}`, completed: `Completed running ${inspect(file)}` },
+    });
+  });
+
+  it('should restart multiple times', async () => {
+    const file = createTmpFile();
+    const { stderr, stdout } = await spawnWithRestarts({ file, restarts: 3 });
+
+    assert.strictEqual(stderr, '');
+    assert.strictEqual(stdout.match(new RegExp(`Restarting ${inspect(file).replace(/\\/g, '\\\\')}`, 'g')).length, 3);
+  });
+
+  it('should pass arguments to file', async () => {
+    const file = fixtures.path('watch-mode/parse_args.js');
+    const random = Date.now().toString();
+    const args = [file, '--random', random];
+    const { stderr, stdout } = await spawnWithRestarts({ file, args });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { inner: random, restarted: `Restarting ${inspect(args.join(' '))}`, completed: `Completed running ${inspect(args.join(' '))}` },
+    });
+  });
+
+  it('should not load --require modules in main process', async () => {
+    const file = createTmpFile('');
+    const required = fixtures.path('watch-mode/process_exit.js');
+    const args = ['--require', required, file];
+    const { stderr, stdout } = await spawnWithRestarts({ file, args });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { restarted: `Restarting ${inspect(file)}`, completed: `Completed running ${inspect(file)}` },
+    });
+  });
+
+  it('should not load --import modules in main process', {
+    skip: 'enable once --import is backported',
+  }, async () => {
+    const file = createTmpFile('');
+    const imported = fixtures.fileURL('watch-mode/process_exit.js');
+    const args = ['--import', imported, file];
+    const { stderr, stdout } = await spawnWithRestarts({ file, args });
+
+    assert.strictEqual(stderr, '');
+    assertRestartedCorrectly({
+      stdout,
+      messages: { restarted: `Restarting ${inspect(file)}`, completed: `Completed running ${inspect(file)}` },
+    });
+  });
+});