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
consttick=require('../common/tick');
66
constinitHooks=require('./init-hooks');
77
const{ checkInvocations }=require('./hook-checks');
8-
8+
consttmpdir=require('../common/tmpdir');
99
constnet=require('net');
1010

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

1414
consthooks=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
constfs=require('fs');
56
constpath=require('path');
7+
const{ debuglog }=require('util');
68

7-
functionrimrafSync(p){
8-
letst;
9-
try{
10-
st=fs.lstatSync(p);
11-
}catch(e){
12-
if(e.code==='ENOENT')
13-
return;
9+
constdebug=debuglog('test/tmpdir');
10+
11+
functionrimrafSync(pathname,{ spawn =true}={}){
12+
constst=(()=>{
13+
try{
14+
returnfs.lstatSync(pathname);
15+
}catch(e){
16+
if(fs.existsSync(pathname))
17+
thrownewError(`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-
returnrmdirSync(p,e);
26-
if(e.code!=='EISDIR')
27-
throwe;
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+
throwe;
61+
}
62+
rmdirSync(pathname,e);
2963
}
3064
}
3165

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

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

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

0 commit comments

Comments
(0)