Skip to content

Commit

Permalink
test: work around ENOTEMPTY when cleaning tmp dir
Browse files Browse the repository at this point in the history
Replace the homegrown rimrafsync implementation in test/common with
a call to `fs.rmdirSync(path, { recursive: true })`.

Fixes: #30620
Fixes: #30844

PR-URL: #30849
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
bnoordhuis authored and Trott committed Dec 12, 2019
1 parent 973b5c0 commit edf654d
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 50 deletions.
3 changes: 1 addition & 2 deletions test/async-hooks/test-pipeconnectwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ const { checkInvocations } = require('./hook-checks');
const tmpdir = require('../common/tmpdir');
const net = require('net');

// Spawning messes up `async_hooks` state.
tmpdir.refresh({ spawn: false });
tmpdir.refresh();

const hooks = initHooks();
hooks.enable();
Expand Down
2 changes: 1 addition & 1 deletion test/async-hooks/test-statwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const path = require('path');
if (!common.isMainThread)
common.skip('Worker bootstrapping works differently -> different async IDs');

tmpdir.refresh({ spawn: false });
tmpdir.refresh();

const file1 = path.join(tmpdir.path, 'file1');
const file2 = path.join(tmpdir.path, 'file2');
Expand Down
6 changes: 1 addition & 5 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -898,11 +898,7 @@ The `tmpdir` module supports the use of a temporary directory for testing.

The realpath of the testing temporary directory.

### refresh(\[opts\])

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

Deletes and recreates the testing temporary directory.

Expand Down
43 changes: 5 additions & 38 deletions test/common/tmpdir.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,12 @@
/* 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');
const { isMainThread } = require('worker_threads');

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}`, { timeout: 1000 });
} catch (e) {
// Attempt failed. Log and carry on.
debug(e);
}
}

fs.rmdirSync(pathname, { recursive: true, maxRetries: 5 });

if (fs.existsSync(pathname))
throw new Error(`Unable to rimraf ${pathname}`);
function rimrafSync(pathname) {
fs.rmdirSync(pathname, { maxRetries: 3, recursive: true });
}

const testRoot = process.env.NODE_TEST_DIR ?
Expand All @@ -52,8 +19,8 @@ const tmpdirName = '.tmp.' +
const tmpPath = path.join(testRoot, tmpdirName);

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

if (firstRefresh) {
Expand All @@ -70,7 +37,7 @@ function onexit() {
process.chdir(testRoot);

try {
rimrafSync(tmpPath, { spawn: false });
rimrafSync(tmpPath);
} catch (e) {
console.error('Can\'t clean tmpdir:', tmpPath);

Expand Down
4 changes: 0 additions & 4 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ test-fs-stat-bigint: PASS,FLAKY
test-net-connect-options-port: PASS,FLAKY

[$system==win32]
# https://github.com/nodejs/node/issues/30620
test-child-process-fork-exec-path: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
test-http2-client-upload: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
Expand All @@ -23,8 +21,6 @@ test-http2-compat-client-upload-reject: PASS,FLAKY
test-http2-multistream-destroy-on-read-tls: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
test-http2-pipe: PASS,FLAKY
# https://github.com/nodejs/node/issues/30844
test-module-loading-globalpaths: PASS,FLAKY
# https://github.com/nodejs/node/issues/23277
test-worker-memory: PASS,FLAKY
# https://github.com/nodejs/node/issues/30846
Expand Down

0 comments on commit edf654d

Please sign in to comment.