From edf654d43e7140f6ac50aefc5dd3f0474bfc5cfe Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Thu, 12 Dec 2019 10:05:38 +0100
Subject: [PATCH] test: work around ENOTEMPTY when cleaning tmp dir

Replace the homegrown rimrafsync implementation in test/common with
a call to `fs.rmdirSync(path, { recursive: true })`.

Fixes: https://github.com/nodejs/node/issues/30620
Fixes: https://github.com/nodejs/node/issues/30844

PR-URL: https://github.com/nodejs/node/pull/30849
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 test/async-hooks/test-pipeconnectwrap.js |  3 +-
 test/async-hooks/test-statwatcher.js     |  2 +-
 test/common/README.md                    |  6 +---
 test/common/tmpdir.js                    | 43 +++---------------------
 test/parallel/parallel.status            |  4 ---
 5 files changed, 8 insertions(+), 50 deletions(-)

diff --git a/test/async-hooks/test-pipeconnectwrap.js b/test/async-hooks/test-pipeconnectwrap.js
index b57cc63cf59e46..36980f018598f3 100644
--- a/test/async-hooks/test-pipeconnectwrap.js
+++ b/test/async-hooks/test-pipeconnectwrap.js
@@ -8,8 +8,7 @@ const { checkInvocations } = require('./hook-checks');
 const tmpdir = require('../common/tmpdir');
 const net = require('net');
 
-// Spawning messes up `async_hooks` state.
-tmpdir.refresh({ spawn: false });
+tmpdir.refresh();
 
 const hooks = initHooks();
 hooks.enable();
diff --git a/test/async-hooks/test-statwatcher.js b/test/async-hooks/test-statwatcher.js
index 48d7b0b22eedff..0b9302f21cbce5 100644
--- a/test/async-hooks/test-statwatcher.js
+++ b/test/async-hooks/test-statwatcher.js
@@ -11,7 +11,7 @@ const path = require('path');
 if (!common.isMainThread)
   common.skip('Worker bootstrapping works differently -> different async IDs');
 
-tmpdir.refresh({ spawn: false });
+tmpdir.refresh();
 
 const file1 = path.join(tmpdir.path, 'file1');
 const file2 = path.join(tmpdir.path, 'file2');
diff --git a/test/common/README.md b/test/common/README.md
index 11827d9743604a..afd054eae1fe34 100644
--- a/test/common/README.md
+++ b/test/common/README.md
@@ -898,11 +898,7 @@ The `tmpdir` module supports the use of a temporary directory for testing.
 
 The realpath of the testing temporary directory.
 
-### refresh(\[opts\])
-
-* `opts` [&lt;Object>][] (optional) Extra options.
-  * `spawn` [&lt;boolean>][] (default: `true`) Indicates that `refresh` is
-    allowed to optionally spawn a subprocess.
+### refresh()
 
 Deletes and recreates the testing temporary directory.
 
diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js
index 0b8465bb22fc73..5e1e21429fe06b 100644
--- a/test/common/tmpdir.js
+++ b/test/common/tmpdir.js
@@ -1,45 +1,12 @@
 /* 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');
 const { isMainThread } = require('worker_threads');
 
-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}`, { timeout: 1000 });
-    } catch (e) {
-      // Attempt failed. Log and carry on.
-      debug(e);
-    }
-  }
-
-  fs.rmdirSync(pathname, { recursive: true, maxRetries: 5 });
-
-  if (fs.existsSync(pathname))
-    throw new Error(`Unable to rimraf ${pathname}`);
+function rimrafSync(pathname) {
+  fs.rmdirSync(pathname, { maxRetries: 3, recursive: true });
 }
 
 const testRoot = process.env.NODE_TEST_DIR ?
@@ -52,8 +19,8 @@ const tmpdirName = '.tmp.' +
 const tmpPath = path.join(testRoot, tmpdirName);
 
 let firstRefresh = true;
-function refresh(opts = {}) {
-  rimrafSync(this.path, opts);
+function refresh() {
+  rimrafSync(this.path);
   fs.mkdirSync(this.path);
 
   if (firstRefresh) {
@@ -70,7 +37,7 @@ function onexit() {
     process.chdir(testRoot);
 
   try {
-    rimrafSync(tmpPath, { spawn: false });
+    rimrafSync(tmpPath);
   } catch (e) {
     console.error('Can\'t clean tmpdir:', tmpPath);
 
diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
index eea561b79b0ea5..dc8d0233d7da96 100644
--- a/test/parallel/parallel.status
+++ b/test/parallel/parallel.status
@@ -11,8 +11,6 @@ test-fs-stat-bigint: PASS,FLAKY
 test-net-connect-options-port: PASS,FLAKY
 
 [$system==win32]
-# https://github.com/nodejs/node/issues/30620
-test-child-process-fork-exec-path: PASS,FLAKY
 # https://github.com/nodejs/node/issues/20750
 test-http2-client-upload: PASS,FLAKY
 # https://github.com/nodejs/node/issues/20750
@@ -23,8 +21,6 @@ test-http2-compat-client-upload-reject: PASS,FLAKY
 test-http2-multistream-destroy-on-read-tls: PASS,FLAKY
 # https://github.com/nodejs/node/issues/20750
 test-http2-pipe: PASS,FLAKY
-# https://github.com/nodejs/node/issues/30844
-test-module-loading-globalpaths: PASS,FLAKY
 # https://github.com/nodejs/node/issues/23277
 test-worker-memory: PASS,FLAKY
 # https://github.com/nodejs/node/issues/30846