Skip to content

Commit f336236

Browse files
refackBridgeAR
authored andcommitted
test: shell out to rmdir first on Windows
cmd's `rmdir` is hardened to deal with Windows edge cases, like lingering processes, indexing, and AV checks. So we give it a try first. * Added `opts = { spawn = true }` to opt-out of spawning * test-pipeconnectwrap.js - spawning messes up async_hooks state PR-URL: #28035 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
1 parent b6bdf75 commit f336236

File tree

3 files changed

+61
-23
lines changed

3 files changed

+61
-23
lines changed

test/async-hooks/test-pipeconnectwrap.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ const assert = require('assert');
55
const tick = require('../common/tick');
66
const initHooks = require('./init-hooks');
77
const { checkInvocations } = require('./hook-checks');
8-
8+
const tmpdir = require('../common/tmpdir');
99
const net = require('net');
1010

11-
const tmpdir = require('../common/tmpdir');
12-
tmpdir.refresh();
11+
// Spawning messes up `async_hooks` state.
12+
tmpdir.refresh({ spawn: false });
1313

1414
const hooks = initHooks();
1515
hooks.enable();

test/common/README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,11 @@ The `tmpdir` module supports the use of a temporary directory for testing.
852852

853853
The realpath of the testing temporary directory.
854854

855-
### refresh()
855+
### refresh([opts])
856+
857+
* `opts` [&lt;Object>] (optional) Extra options.
858+
* `spawn` [&lt;boolean>] (default: `true`) Indicates that `refresh` is allowed
859+
to optionally spawn a subprocess.
856860

857861
Deletes and recreates the testing temporary directory.
858862

test/common/tmpdir.js

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,65 @@
11
/* eslint-disable node-core/require-common-first, node-core/required-modules */
22
'use strict';
33

4+
const { execSync } = require('child_process');
45
const fs = require('fs');
56
const path = require('path');
7+
const { debuglog } = require('util');
68

7-
function rimrafSync(p) {
8-
let st;
9-
try {
10-
st = fs.lstatSync(p);
11-
} catch (e) {
12-
if (e.code === 'ENOENT')
13-
return;
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}`, { timout: 1000 });
32+
} catch (e) {
33+
// Attempt failed. Log and carry on.
34+
debug(e);
35+
}
1436
}
1537

1638
try {
17-
if (st && st.isDirectory())
18-
rmdirSync(p, null);
39+
if (st.isDirectory())
40+
rmdirSync(pathname, null);
1941
else
20-
fs.unlinkSync(p);
42+
fs.unlinkSync(pathname);
2143
} catch (e) {
22-
if (e.code === 'ENOENT')
23-
return;
24-
if (e.code === 'EPERM')
25-
return rmdirSync(p, e);
26-
if (e.code !== 'EISDIR')
27-
throw e;
28-
rmdirSync(p, e);
44+
debug(e);
45+
switch (e.code) {
46+
case 'ENOENT':
47+
// It's not there anymore. Work is done. Exiting.
48+
return;
49+
50+
case 'EPERM':
51+
// This can happen, try again with `rmdirSync`.
52+
break;
53+
54+
case 'EISDIR':
55+
// Got 'EISDIR' even after testing `st.isDirectory()`...
56+
// Try again with `rmdirSync`.
57+
break;
58+
59+
default:
60+
throw e;
61+
}
62+
rmdirSync(pathname, e);
2963
}
3064
}
3165

@@ -62,8 +96,8 @@ if (process.env.TEST_THREAD_ID) {
6296

6397
const tmpPath = path.join(testRoot, tmpdirName);
6498

65-
function refresh() {
66-
rimrafSync(this.path);
99+
function refresh(opts = {}) {
100+
rimrafSync(this.path, opts);
67101
fs.mkdirSync(this.path);
68102
}
69103

0 commit comments

Comments
 (0)