test: try to reduce flakes on Windows

test/async-hooks/test-pipeconnectwrap.js

@@ -5,11 +5,11 @@ const assert = require('assert');
 const tick = require('../common/tick');
 const initHooks = require('./init-hooks');
 const { checkInvocations } = require('./hook-checks');
-
+const tmpdir = require('../common/tmpdir');
 const net = require('net');
 
-const tmpdir = require('../common/tmpdir');
-tmpdir.refresh();
+// Spawning messes up `async_hooks` state.
+tmpdir.refresh({ spawn: false });
 
 const hooks = initHooks();
 hooks.enable();

test/common/README.md

@@ -852,7 +852,11 @@ The `tmpdir` module supports the use of a temporary directory for testing.
 
 The realpath of the testing temporary directory.
 
-### refresh()
+### refresh([opts])
+
+* `opts` [<Object>] (optional) Extra options.
+  * `spawn` [<boolean>] (default: `true`) Indicates that `refresh` is allowed
+    to optionally spawn a subprocess.
 
 Deletes and recreates the testing temporary directory.
 

test/common/tmpdir.js

@@ -1,31 +1,65 @@
 /* eslint-disable node-core/require-common-first, node-core/required-modules */
 'use strict';
 
+const { execSync } = require('child_process');
 const fs = require('fs');
 const path = require('path');
+const { debuglog } = require('util');
 
-function rimrafSync(p) {
-  let st;
-  try {
-    st = fs.lstatSync(p);
-  } catch (e) {
-    if (e.code === 'ENOENT')
-      return;
+const debug = debuglog('test/tmpdir');
+
+function rimrafSync(pathname, { spawn = true } = {}) {
+  const st = (() => {
+    try {
+      return fs.lstatSync(pathname);
+    } catch (e) {
+      if (fs.existsSync(pathname))
+        throw new Error(`Something wonky happened rimrafing ${pathname}`);
+      debug(e);
+    }
+  })();
+
+  // If (!st) then nothing to do.
+  if (!st) {
+    return;
+  }
+
+  // On Windows first try to delegate rmdir to a shell.
+  if (spawn && process.platform === 'win32' && st.isDirectory()) {
+    try {
+      // Try `rmdir` first.
+      execSync(`rmdir /q /s ${pathname}`, { timout: 1000 });
+    } catch (e) {
+      // Attempt failed. Log and carry on.
+      debug(e);
+    }
   }
 
   try {
-    if (st && st.isDirectory())
-      rmdirSync(p, null);
+    if (st.isDirectory())
+      rmdirSync(pathname, null);
     else
-      fs.unlinkSync(p);
+      fs.unlinkSync(pathname);
   } catch (e) {
-    if (e.code === 'ENOENT')
-      return;
-    if (e.code === 'EPERM')
-      return rmdirSync(p, e);
-    if (e.code !== 'EISDIR')
-      throw e;
-    rmdirSync(p, e);
+    debug(e);
+    switch (e.code) {
+      case 'ENOENT':
+        // It's not there anymore. Work is done. Exiting.
+        return;
+
+      case 'EPERM':
+        // This can happen, try again with `rmdirSync`.
+        break;
+
+      case 'EISDIR':
+        // Got 'EISDIR' even after testing `st.isDirectory()`...
+        // Try again with `rmdirSync`.
+        break;
+
+      default:
+        throw e;
+    }
+    rmdirSync(pathname, e);
   }
 }
 
@@ -55,15 +89,11 @@ const testRoot = process.env.NODE_TEST_DIR ?
 
 // Using a `.` prefixed name, which is the convention for "hidden" on POSIX,
 // gets tools to ignore it by default or by simple rules, especially eslint.
-let tmpdirName = '.tmp';
-if (process.env.TEST_THREAD_ID) {
-  tmpdirName += `.${process.env.TEST_THREAD_ID}`;
-}
-
+const tmpdirName = '.tmp.' + (process.env.TEST_THREAD_ID || '0');
 const tmpPath = path.join(testRoot, tmpdirName);
 
-function refresh() {
-  rimrafSync(this.path);
+function refresh(opts = {}) {
+  rimrafSync(this.path, opts);
   fs.mkdirSync(this.path);
 }
 

test/parallel/parallel.status

@@ -17,6 +17,8 @@ test-worker-memory: PASS,FLAKY
 # https://github.com/nodejs/node/issues/20750
 test-http2-client-upload: PASS,FLAKY
 test-http2-client-upload-reject: PASS,FLAKY
+# https://github.com/nodejs/node/issues/28106
+test-worker-debug: PASS,FLAKY
 
 [$system==linux]
 

test/parallel/test-common.js

@@ -23,8 +23,11 @@
 const common = require('../common');
 const hijackstdio = require('../common/hijackstdio');
 const fixtures = require('../common/fixtures');
+const tmpdir = require('../common/tmpdir');
 const assert = require('assert');
 const { execFile } = require('child_process');
+const { writeFileSync, existsSync } = require('fs');
+const { join } = require('path');
 
 // Test for leaked global detection
 {
@@ -124,25 +127,38 @@ const HIJACK_TEST_ARRAY = [ 'foo\n', 'bar\n', 'baz\n' ];
   assert.strictEqual(originalWrite, stream.write);
 });
 
-// hijackStderr and hijackStdout again
-// for console
-[[ 'err', 'error' ], [ 'out', 'log' ]].forEach(([ type, method ]) => {
-  hijackstdio[`hijackStd${type}`](common.mustCall(function(data) {
-    assert.strictEqual(data, 'test\n');
+// Test `tmpdir`.
+{
+  tmpdir.refresh();
+  assert.ok(/\.tmp\.\d+/.test(tmpdir.path));
+  const sentinelPath = join(tmpdir.path, 'gaga');
+  writeFileSync(sentinelPath, 'googoo');
+  tmpdir.refresh();
+  assert.strictEqual(existsSync(tmpdir.path), true);
+  assert.strictEqual(existsSync(sentinelPath), false);
+}
 
-    // throw an error
-    throw new Error(`console ${type} error`);
-  }));
+// hijackStderr and hijackStdout again for console
+// Must be last, since it uses `process.on('uncaughtException')`
+{
+  [['err', 'error'], ['out', 'log']].forEach(([type, method]) => {
+    hijackstdio[`hijackStd${type}`](common.mustCall(function(data) {
+      assert.strictEqual(data, 'test\n');
 
-  console[method]('test');
-  hijackstdio[`restoreStd${type}`]();
-});
+      // throw an error
+      throw new Error(`console ${type} error`);
+    }));
+
+    console[method]('test');
+    hijackstdio[`restoreStd${type}`]();
+  });
 
-let uncaughtTimes = 0;
-process.on('uncaughtException', common.mustCallAtLeast(function(e) {
-  assert.strictEqual(uncaughtTimes < 2, true);
-  assert.strictEqual(e instanceof Error, true);
-  assert.strictEqual(
-    e.message,
-    `console ${([ 'err', 'out' ])[uncaughtTimes++]} error`);
-}, 2));
+  let uncaughtTimes = 0;
+  process.on('uncaughtException', common.mustCallAtLeast(function(e) {
+    assert.strictEqual(uncaughtTimes < 2, true);
+    assert.strictEqual(e instanceof Error, true);
+    assert.strictEqual(
+      e.message,
+      `console ${(['err', 'out'])[uncaughtTimes++]} error`);
+  }, 2));
+}  // End of "Must be last".

test/parallel/test-worker-debug.js

@@ -91,14 +91,14 @@ class WorkerSession extends EventEmitter {
       this.emit(message.method, message);
       return;
     }
-    const callback = this._requestCallbacks.get(message.id);
-    if (callback) {
-      this._requestCallbacks.delete(message.id);
-      if (message.error)
-        callback[1](message.error.message);
-      else
-        callback[0](message.result);
-    }
+    if (!this._requestCallbacks.has(message.id))
+      return;
+    const [ resolve, reject ] = this._requestCallbacks.get(message.id);
+    this._requestCallbacks.delete(message.id);
+    if (message.error)
+      reject(new Error(message.error.message));
+    else
+      resolve(message.result);
   }
 
   async waitForBreakAfterCommand(command, script, line) {
@@ -144,7 +144,7 @@ async function testBasicWorkerDebug(session, post) {
   assert.strictEqual(waitingForDebugger, true);
   const detached = waitForWorkerDetach(session, sessionId);
   const workerSession = new WorkerSession(session, sessionId);
-  const contextEvents = Promise.all([
+  const contextEventPromises = Promise.all([
     waitForEvent(workerSession, 'Runtime.executionContextCreated'),
     waitForEvent(workerSession, 'Runtime.executionContextDestroyed')
   ]);
@@ -156,9 +156,10 @@ async function testBasicWorkerDebug(session, post) {
     'Runtime.runIfWaitingForDebugger', __filename, 1);
   await workerSession.waitForBreakAfterCommand(
     'Debugger.resume', __filename, 26);  // V8 line number is zero-based
-  assert.strictEqual(await consolePromise, workerMessage);
+  const msg = await consolePromise;
+  assert.strictEqual(msg, workerMessage);
   workerSession.post('Debugger.resume');
-  await Promise.all([worker, detached, contextEvents]);
+  await Promise.all([worker, detached, contextEventPromises]);
 }
 
 async function testNoWaitOnStart(session, post) {
@@ -252,7 +253,7 @@ async function testWaitForDisconnectInWorker(session, post) {
   sessionWithoutWaiting.disconnect();
 }
 
-async function test() {
+(async function test() {
   const session = new Session();
   session.connect();
   const post = doPost.bind(null, session);
@@ -264,11 +265,14 @@ async function test() {
   await runWorker(1);
 
   await testNoWaitOnStart(session, post);
+
   await testTwoWorkers(session, post);
+
   await testWaitForDisconnectInWorker(session, post);
 
   session.disconnect();
   console.log('Test done');
-}
-
-test();
+})().catch((err) => {
+  console.error(err);
+  process.abort();
+});