Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f336236

Browse filesBrowse 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 <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
1 parent b6bdf75 commit f336236
Copy full SHA for f336236

File tree

Expand file treeCollapse file tree

3 files changed

+61
-23
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+61
-23
lines changed
Open diff view settings
Collapse file

‎test/async-hooks/test-pipeconnectwrap.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-pipeconnectwrap.js
+3-3Lines 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();
Collapse file

‎test/common/README.md‎

Copy file name to clipboardExpand all lines: test/common/README.md
+5-1Lines changed: 5 additions & 1 deletion
  • Display the source diff
  • Display the rich diff
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

Collapse file

‎test/common/tmpdir.js‎

Copy file name to clipboardExpand all lines: test/common/tmpdir.js
+53-19Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.