From f33623662fc68a139755502db60fcb480e695207 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sun, 2 Jun 2019 16:19:26 -0400 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/28035 Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater Reviewed-By: Benjamin Gruenbaum Reviewed-By: Sam Roberts --- test/async-hooks/test-pipeconnectwrap.js | 6 +- test/common/README.md | 6 +- test/common/tmpdir.js | 72 +++++++++++++++++------- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/test/async-hooks/test-pipeconnectwrap.js b/test/async-hooks/test-pipeconnectwrap.js index 5d3706ac44fef6..f086807e393145 100644 --- a/test/async-hooks/test-pipeconnectwrap.js +++ b/test/async-hooks/test-pipeconnectwrap.js @@ -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(); diff --git a/test/common/README.md b/test/common/README.md index 664b42aca3e66c..9c0d150b789a5b 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -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` [<Object>] (optional) Extra options. + * `spawn` [<boolean>] (default: `true`) Indicates that `refresh` is allowed + to optionally spawn a subprocess. Deletes and recreates the testing temporary directory. diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index 369c49019aa436..ced957e5547bb9 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -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); } } @@ -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); }