Skip to content

Commit

Permalink
test: shell out to rmdir first on Windows
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
refack committed Jun 6, 2019
1 parent 546d6cd commit 59f666c
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 23 deletions.
6 changes: 3 additions & 3 deletions test/async-hooks/test-pipeconnectwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ const assert = require('assert');
const tick = require('../common/tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const tmpdir = require('../common/tmpdir');
const net = require('net');

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

const hooks = initHooks();
hooks.enable();
Expand Down
6 changes: 5 additions & 1 deletion test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,11 @@ The `tmpdir` module supports the use of a temporary directory for testing.

The realpath of the testing temporary directory.

### refresh()
### refresh([opts])

* `opts` [&lt;Object>] (optional) Extra options.
* `spawn` [&lt;boolean>] (default: `true`) Indicates that `refresh` is allowed
to optionally spawn a subprocess.

Deletes and recreates the testing temporary directory.

Expand Down
72 changes: 53 additions & 19 deletions test/common/tmpdir.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,65 @@
/* 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');

function rimrafSync(p) {
let st;
try {
st = fs.lstatSync(p);
} catch (e) {
if (e.code === 'ENOENT')
return;
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}`, { timout: 1000 });
} catch (e) {
// Attempt failed. Log and carry on.
debug(e);
}
}

try {
if (st && st.isDirectory())
rmdirSync(p, null);
if (st.isDirectory())
rmdirSync(pathname, null);
else
fs.unlinkSync(p);
fs.unlinkSync(pathname);
} catch (e) {
if (e.code === 'ENOENT')
return;
if (e.code === 'EPERM')
return rmdirSync(p, e);
if (e.code !== 'EISDIR')
throw e;
rmdirSync(p, e);
debug(e);
switch (e.code) {
case 'ENOENT':
// It's not there anymore. Work is done. Exiting.
return;

case 'EPERM':
// This can happen, try again with `rmdirSync`.
break;

case 'EISDIR':
// Got 'EISDIR' even after testing `st.isDirectory()`...
// Try again with `rmdirSync`.
break;

default:
throw e;
}
rmdirSync(pathname, e);
}
}

Expand Down Expand Up @@ -62,8 +96,8 @@ if (process.env.TEST_THREAD_ID) {

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

function refresh() {
rimrafSync(this.path);
function refresh(opts = {}) {
rimrafSync(this.path, opts);
fs.mkdirSync(this.path);
}

Expand Down

0 comments on commit 59f666c

Please sign in to comment.