Commit 5fff46a
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: #30620
Fixes: #30844
PR-URL: #30849
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>1 parent cfe30ae commit 5fff46a
5 files changed
Lines changed: 7 additions & 46 deletions
File tree
- test
- async-hooks
- common
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -8,8 +8,7 @@ const { checkInvocations } = require('./hook-checks'); | |||
| 8 | 8 | const tmpdir = require('../common/tmpdir'); | |
| 9 | 9 | const net = require('net'); | |
| 10 | 10 | ||
| 11 | - // Spawning messes up `async_hooks` state. | ||
| 12 | - tmpdir.refresh({ spawn: false }); | ||
| 11 | + tmpdir.refresh(); | ||
| 13 | 12 | ||
| 14 | 13 | const hooks = initHooks(); | |
| 15 | 14 | hooks.enable(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -11,7 +11,7 @@ const path = require('path'); | |||
| 11 | 11 | if (!common.isMainThread) | |
| 12 | 12 | common.skip('Worker bootstrapping works differently -> different async IDs'); | |
| 13 | 13 | ||
| 14 | - tmpdir.refresh({ spawn: false }); | ||
| 14 | + tmpdir.refresh(); | ||
| 15 | 15 | ||
| 16 | 16 | const file1 = path.join(tmpdir.path, 'file1'); | |
| 17 | 17 | const file2 = path.join(tmpdir.path, 'file2'); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -898,11 +898,7 @@ The `tmpdir` module supports the use of a temporary directory for testing. | |||
| 898 | 898 | ||
| 899 | 899 | The realpath of the testing temporary directory. | |
| 900 | 900 | ||
| 901 | - ### refresh(\[opts\]) | ||
| 902 | - | ||
| 903 | - * `opts` [<Object>][] (optional) Extra options. | ||
| 904 | - * `spawn` [<boolean>][] (default: `true`) Indicates that `refresh` is | ||
| 905 | - allowed to optionally spawn a subprocess. | ||
| 901 | + ### refresh() | ||
| 906 | 902 | ||
| 907 | 903 | Deletes and recreates the testing temporary directory. | |
| 908 | 904 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,41 +1,11 @@ | |||
| 1 | 1 | /* eslint-disable node-core/require-common-first, node-core/required-modules */ | |
| 2 | 2 | 'use strict'; | |
| 3 | 3 | ||
| 4 | - const { execSync } = require('child_process'); | ||
| 5 | 4 | const fs = require('fs'); | |
| 6 | 5 | const path = require('path'); | |
| 7 | - const { debuglog } = require('util'); | ||
| 8 | 6 | ||
| 9 | - const debug = debuglog('test/tmpdir'); | ||
| 10 | - | ||
| 11 | - function rimrafSync(pathname, { spawn = true } = {}) { | ||
| 12 | - const st = (() => { | ||
| 13 | - try { | ||
| 14 | - return fs.lstatSync(pathname); | ||
| 15 | - } catch (e) { | ||
| 16 | - if (fs.existsSync(pathname)) | ||
| 17 | - throw new Error(`Something wonky happened rimrafing ${pathname}`); | ||
| 18 | - debug(e); | ||
| 19 | - } | ||
| 20 | - })(); | ||
| 21 | - | ||
| 22 | - // If (!st) then nothing to do. | ||
| 23 | - if (!st) { | ||
| 24 | - return; | ||
| 25 | - } | ||
| 26 | - | ||
| 27 | - // On Windows first try to delegate rmdir to a shell. | ||
| 28 | - if (spawn && process.platform === 'win32' && st.isDirectory()) { | ||
| 29 | - try { | ||
| 30 | - // Try `rmdir` first. | ||
| 31 | - execSync(`rmdir /q /s ${pathname}`, { timeout: 1000 }); | ||
| 32 | - } catch (e) { | ||
| 33 | - // Attempt failed. Log and carry on. | ||
| 34 | - debug(e); | ||
| 35 | - } | ||
| 36 | - } | ||
| 37 | - | ||
| 38 | - fs.rmdirSync(pathname, { recursive: true, maxRetries: 5 }); | ||
| 7 | + function rimrafSync(pathname) { | ||
| 8 | + fs.rmdirSync(pathname, { maxRetries: 3, recursive: true }); | ||
| 39 | 9 | } | |
| 40 | 10 | ||
| 41 | 11 | const testRoot = process.env.NODE_TEST_DIR ? | |
@@ -46,8 +16,8 @@ const testRoot = process.env.NODE_TEST_DIR ? | |||
| 46 | 16 | const tmpdirName = '.tmp.' + (process.env.TEST_THREAD_ID || '0'); | |
| 47 | 17 | const tmpPath = path.join(testRoot, tmpdirName); | |
| 48 | 18 | ||
| 49 | - function refresh(opts = {}) { | ||
| 50 | - rimrafSync(this.path, opts); | ||
| 19 | + function refresh() { | ||
| 20 | + rimrafSync(this.path); | ||
| 51 | 21 | fs.mkdirSync(this.path); | |
| 52 | 22 | } | |
| 53 | 23 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -11,8 +11,6 @@ test-fs-stat-bigint: PASS,FLAKY | |||
| 11 | 11 | test-net-connect-options-port: PASS,FLAKY | |
| 12 | 12 | ||
| 13 | 13 | [$system==win32] | |
| 14 | - # https://github.com/nodejs/node/issues/30620 | ||
| 15 | - test-child-process-fork-exec-path: PASS,FLAKY | ||
| 16 | 14 | # https://github.com/nodejs/node/issues/20750 | |
| 17 | 15 | test-http2-client-upload: PASS,FLAKY | |
| 18 | 16 | # https://github.com/nodejs/node/issues/20750 | |
@@ -23,8 +21,6 @@ test-http2-compat-client-upload-reject: PASS,FLAKY | |||
| 23 | 21 | test-http2-multistream-destroy-on-read-tls: PASS,FLAKY | |
| 24 | 22 | # https://github.com/nodejs/node/issues/20750 | |
| 25 | 23 | test-http2-pipe: PASS,FLAKY | |
| 26 | - # https://github.com/nodejs/node/issues/30844 | ||
| 27 | - test-module-loading-globalpaths: PASS,FLAKY | ||
| 28 | 24 | # https://github.com/nodejs/node/issues/23277 | |
| 29 | 25 | test-worker-memory: PASS,FLAKY | |
| 30 | 26 | # https://github.com/nodejs/node/issues/30846 | |
0 commit comments