From 0e5be1a252c0c0b1d48813d41b8d03d5bf1dfeff Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Wed, 17 Apr 2024 22:23:40 -0700 Subject: [PATCH 01/19] chore: try to reduce windows shim test flakes --- .gitignore | 1 + DEPENDENCIES.md | 1 + libtap-settings.js | 7 + package-lock.json | 356 +++++++++++++++--- package.json | 4 +- scripts/template-oss/root.js | 1 + .../test/lib/utils/exit-handler.js.test.cjs | 12 +- test/bin/windows-shims.js | 26 +- test/lib/utils/exit-handler.js | 28 +- 9 files changed, 347 insertions(+), 89 deletions(-) create mode 100644 libtap-settings.js diff --git a/.gitignore b/.gitignore index 2a10ae9bd8888..5944e2633a27b 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,7 @@ tap-testdir*/ !/docs/ !/index.js !/lib/ +!/libtap-settings.js !/LICENSE* !/map.js !/node_modules/ diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index aa1b42f7bad7f..c4a34f98a8c7c 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -548,6 +548,7 @@ graph LR; npm-->remark-gfm; npm-->remark-github; npm-->remark; + npm-->rimraf; npm-->semver; npm-->sigstore-tuf["@sigstore/tuf"]; npm-->spawk; diff --git a/libtap-settings.js b/libtap-settings.js new file mode 100644 index 0000000000000..31d00333f01bf --- /dev/null +++ b/libtap-settings.js @@ -0,0 +1,7 @@ +// use rimraf for tap recursive directory removal, so that +// the windows tests don't flake with fs errors +const { rimraf } = require('rimraf') +module.exports = { + rmdirRecursiveSync: path => rimraf.sync(path), + rmdirRecursive: (path, cb) => rimraf(path).then(() => cb(), cb), +} diff --git a/package-lock.json b/package-lock.json index ae6ee87388514..475b645c99765 100644 --- a/package-lock.json +++ b/package-lock.json @@ -183,6 +183,7 @@ "remark": "^14.0.2", "remark-gfm": "^3.0.1", "remark-github": "^11.2.4", + "rimraf": "^5.0.5", "spawk": "^1.7.1", "tap": "^16.3.9" }, @@ -1787,6 +1788,63 @@ "node": "^12.13.0 || ^14.15.0 || >=16.0.0" } }, + "node_modules/@npmcli/move-file/node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dev": true, + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/@npmcli/move-file/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "dev": true, + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/@npmcli/move-file/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, + "node_modules/@npmcli/move-file/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/@npmcli/name-from-folder": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@npmcli/name-from-folder/-/name-from-folder-2.0.0.tgz", @@ -5328,6 +5386,67 @@ "node": "^10.12.0 || >=12.0.0" } }, + "node_modules/flat-cache/node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dev": true, + "peer": true, + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/flat-cache/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "dev": true, + "peer": true, + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/flat-cache/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "peer": true, + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, + "node_modules/flat-cache/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "peer": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/flatted": { "version": "3.3.1", "resolved": "https://registry.npmjs.org/flatted/-/flatted-3.3.1.tgz", @@ -6928,6 +7047,48 @@ "node": ">=8" } }, + "node_modules/istanbul-lib-processinfo/node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dev": true, + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/istanbul-lib-processinfo/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "dev": true, + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/istanbul-lib-processinfo/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, "node_modules/istanbul-lib-processinfo/node_modules/p-map": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/p-map/-/p-map-3.0.0.tgz", @@ -6940,6 +7101,21 @@ "node": ">=8" } }, + "node_modules/istanbul-lib-processinfo/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/istanbul-lib-report": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/istanbul-lib-report/-/istanbul-lib-report-3.0.1.tgz", @@ -8293,6 +8469,63 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, + "node_modules/licensee/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/licensee/node_modules/rimraf/node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dev": true, + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, + "node_modules/licensee/node_modules/rimraf/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "dev": true, + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/licensee/node_modules/rimraf/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, "node_modules/licensee/node_modules/signal-exit": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", @@ -10401,6 +10634,21 @@ "node": ">=8" } }, + "node_modules/nyc/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/nyc/node_modules/signal-exit": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", @@ -11858,62 +12106,23 @@ } }, "node_modules/rimraf": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", - "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "version": "5.0.5", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-5.0.5.tgz", + "integrity": "sha512-CqDakW+hMe/Bz202FPEymy68P+G50RfMQK+Qo5YUqc9SPipvbGjCGKd0RSKEelbsfQuw3g5NZDSrlZZAJurH1A==", "dev": true, "dependencies": { - "glob": "^7.1.3" + "glob": "^10.3.7" }, "bin": { - "rimraf": "bin.js" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" - } - }, - "node_modules/rimraf/node_modules/brace-expansion": { - "version": "1.1.11", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", - "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", - "dev": true, - "dependencies": { - "balanced-match": "^1.0.0", - "concat-map": "0.0.1" - } - }, - "node_modules/rimraf/node_modules/glob": { - "version": "7.2.3", - "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", - "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", - "dev": true, - "dependencies": { - "fs.realpath": "^1.0.0", - "inflight": "^1.0.4", - "inherits": "2", - "minimatch": "^3.1.1", - "once": "^1.3.0", - "path-is-absolute": "^1.0.0" + "rimraf": "dist/esm/bin.mjs" }, "engines": { - "node": "*" + "node": ">=14" }, "funding": { "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/rimraf/node_modules/minimatch": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", - "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", - "dev": true, - "dependencies": { - "brace-expansion": "^1.1.7" - }, - "engines": { - "node": "*" - } - }, "node_modules/rrweb-cssom": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/rrweb-cssom/-/rrweb-cssom-0.6.0.tgz", @@ -12282,6 +12491,16 @@ "node": ">=8" } }, + "node_modules/spawn-wrap/node_modules/brace-expansion": { + "version": "1.1.11", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", + "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", + "dev": true, + "dependencies": { + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" + } + }, "node_modules/spawn-wrap/node_modules/foreground-child": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/foreground-child/-/foreground-child-2.0.0.tgz", @@ -12295,6 +12514,53 @@ "node": ">=8.0.0" } }, + "node_modules/spawn-wrap/node_modules/glob": { + "version": "7.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", + "integrity": "sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==", + "dev": true, + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^3.1.1", + "once": "^1.3.0", + "path-is-absolute": "^1.0.0" + }, + "engines": { + "node": "*" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/spawn-wrap/node_modules/minimatch": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", + "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", + "dev": true, + "dependencies": { + "brace-expansion": "^1.1.7" + }, + "engines": { + "node": "*" + } + }, + "node_modules/spawn-wrap/node_modules/rimraf": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", + "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, + "dependencies": { + "glob": "^7.1.3" + }, + "bin": { + "rimraf": "bin.js" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/spawn-wrap/node_modules/signal-exit": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", diff --git a/package.json b/package.json index 7090dd882e276..4fadcf2cbccd6 100644 --- a/package.json +++ b/package.json @@ -214,6 +214,7 @@ "remark": "^14.0.2", "remark-gfm": "^3.0.1", "remark-github": "^11.2.4", + "rimraf": "^5.0.5", "spawk": "^1.7.1", "tap": "^16.3.9" }, @@ -254,7 +255,8 @@ "--exclude", "tap-snapshots/**" ], - "test-ignore": "^(docs|smoke-tests|mock-globals|mock-registry|workspaces)/" + "test-ignore": "^(docs|smoke-tests|mock-globals|mock-registry|workspaces)/", + "libtap-settings": "./libtap-settings.js" }, "templateOSS": { "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.", diff --git a/scripts/template-oss/root.js b/scripts/template-oss/root.js index fb2d964a3f993..a1a28e60f4b79 100644 --- a/scripts/template-oss/root.js +++ b/scripts/template-oss/root.js @@ -66,6 +66,7 @@ module.exports = { '/.mailmap', '/.licensee.json', '/.gitattributes', + '/libtap-settings.js', ], ignorePaths: [ '/node_modules/.bin/', diff --git a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs index cfb3034b3b06a..3fa13309d6249 100644 --- a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs +++ b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs @@ -23,9 +23,9 @@ XX timing npm:load:configScope Completed in {TIME}ms XX timing npm:load Completed in {TIME}ms XX verbose stack Error: Unknown error XX verbose cwd {CWD}/prefix -XX verbose Foo 1.0.0 -XX verbose node v1.0.0 -XX verbose npm v1.0.0 +XX verbose {OS} +XX verbose {NODE-VERSION} +XX verbose npm {NPM-VERSION} XX error code ECODE XX error ERR SUMMARY Unknown error XX error ERR DETAIL Unknown error @@ -54,9 +54,9 @@ timing npm:load:configScope Completed in {TIME}ms timing npm:load Completed in {TIME}ms verbose stack Error: Unknown error verbose cwd {CWD}/prefix -verbose Foo 1.0.0 -verbose node v1.0.0 -verbose npm v1.0.0 +verbose {OS} +verbose {NODE-VERSION} +verbose npm {NPM-VERSION} error code ECODE error ERR SUMMARY Unknown error error ERR DETAIL Unknown error diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 2abe5013bf10b..5e4f28127e1b3 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,7 +1,7 @@ const t = require('tap') const { spawnSync } = require('child_process') -const { resolve, join, extname, basename, sep } = require('path') -const { copyFileSync, readFileSync, chmodSync, readdirSync, rmSync, statSync } = require('fs') +const { resolve, join, extname, basename } = require('path') +const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') const Diff = require('diff') const { sync: which } = require('which') const { version } = require('../../package.json') @@ -20,12 +20,6 @@ const SHIMS = readNonJsFiles(BIN) const NODE_GYP = readNonJsFiles(join(BIN, 'node-gyp-bin')) const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))] -// windows requires each segment of a command path to be quoted when using shell: true -const quotePath = (cmd) => cmd - .split(sep) - .map(p => p.includes(' ') ? `"${p}"` : p) - .join(sep) - t.test('shim contents', t => { // these scripts should be kept in sync so this tests the contents of each // and does a diff to ensure the only differences between them are necessary @@ -82,6 +76,7 @@ t.test('node-gyp', t => { t.test('run shims', t => { const path = t.testdir({ ...SHIMS, + 'node.exe': readFileSync(process.execPath), // simulate the state where one version of npm is installed // with node, but we should load the globally installed one 'global-prefix': { @@ -105,26 +100,17 @@ t.test('run shims', t => { }, }) - // hacky fix to decrease flakes of this test from `NOTEMPTY: directory not empty, rmdir` - // this should get better in tap@18 and we can try removing it then - copyFileSync(process.execPath, join(path, 'node.exe')) - t.teardown(async () => { - rmSync(join(path, 'node.exe')) - await new Promise(res => setTimeout(res, 100)) - // this is superstition - rmSync(join(path, 'node.exe'), { force: true }) - }) - const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { if (cmd.endsWith('bash.exe')) { // only cygwin *requires* the -l, but the others are ok with it args.unshift('-l') } - const result = spawnSync(cmd, args, { + const result = spawnSync(`"${cmd}"`, args, { // don't hit the registry for the update check env: { PATH: path, npm_config_update_notifier: 'false' }, cwd: path, windowsHide: true, + shell: true, ...opts, }) if (stdioString) { @@ -235,9 +221,7 @@ t.test('run shims', t => { args.push(bin) break case 'pwsh.exe': - cmd = quotePath(cmd) args.push(`${bin}.ps1`) - opts.shell = true break default: throw new Error('unknown shell') diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index da2fe3d6ce6ed..84c96cb2fbca6 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -1,6 +1,7 @@ const fs = require('node:fs') const { join, resolve } = require('node:path') const EventEmitter = require('node:events') +const os = require('node:os') const t = require('tap') const fsMiniPass = require('fs-minipass') const { output, time } = require('proc-log') @@ -8,6 +9,7 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const mockGlobals = require('@npmcli/mock-globals') const { cleanCwd, cleanDate } = require('../../fixtures/clean-snapshot') const tmock = require('../../fixtures/tmock') +const { version: NPM_VERSION } = require('../../../package.json') const pick = (obj, ...keys) => keys.reduce((acc, key) => { acc[key] = obj[key] @@ -31,6 +33,10 @@ t.cleanSnapshot = (path) => cleanDate(cleanCwd(path)) .replace(/.*silly logfile.*cleaning.*\n/gm, '') .replace(/(Completed in )\d+(ms)/g, '$1{TIME}$2') .replace(/(removing )\d+( files)/g, '$1${NUM}2') + .replaceAll(`node ${process.version}`, '{NODE-VERSION}') + .replaceAll(`node ${process.version}`, '{NODE-VERSION}') + .replaceAll(`${os.type()} ${os.release()}`, '{OS}') + .replaceAll(`v${NPM_VERSION}`, '{NPM-VERSION}') // cut off process from script so that it won't quit the test runner // while trying to run through the myriad of cases. need to make it @@ -39,9 +45,9 @@ t.cleanSnapshot = (path) => cleanDate(cleanCwd(path)) mockGlobals(t, { process: Object.assign(new EventEmitter(), { // these are process properties that are needed in the running code and tests - ...pick(process, 'execPath', 'stdout', 'stderr', 'stdin', 'cwd', 'chdir', 'env', 'umask'), + // eslint-disable-next-line max-len + ...pick(process, 'version', 'execPath', 'stdout', 'stderr', 'stdin', 'cwd', 'chdir', 'env', 'umask'), argv: ['/node', ...process.argv.slice(1)], - version: 'v1.0.0', kill: () => {}, reallyExit: (code) => process.exit(code), pid: 123456, @@ -55,14 +61,9 @@ mockGlobals(t, { const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => { const errors = [] - const { npm, logMocks, ...rest } = await loadMockNpm(t, { + const { npm, ...rest } = await loadMockNpm(t, { ...opts, - mocks: { - '{ROOT}/package.json': { - version: '1.0.0', - }, - ...mocks, - }, + mocks, config: (dirs) => ({ loglevel: 'notice', ...(typeof config === 'function' ? config(dirs) : config), @@ -85,11 +86,6 @@ const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => { }, }, }), - 'node:os': { - type: () => 'Foo', - release: () => '1.0.0', - }, - ...logMocks, ...mocks, }) @@ -491,7 +487,7 @@ t.test('timing with no error', async (t) => { t.match(timingFileData, { metadata: { command: [], - version: '1.0.0', + version: npm.version, logfiles: [String], }, timers: { @@ -528,7 +524,7 @@ t.test('unfinished timers', async (t) => { t.match(timingFileData, { metadata: { command: [], - version: '1.0.0', + version: npm.version, logfiles: [String], }, timers: { From c04ae41a704fa6f39a9bac24ee014b5b3c7ae366 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 13:48:03 -0700 Subject: [PATCH 02/19] remove extra line --- test/lib/utils/exit-handler.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index 84c96cb2fbca6..19bfacbef6050 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -34,7 +34,6 @@ t.cleanSnapshot = (path) => cleanDate(cleanCwd(path)) .replace(/(Completed in )\d+(ms)/g, '$1{TIME}$2') .replace(/(removing )\d+( files)/g, '$1${NUM}2') .replaceAll(`node ${process.version}`, '{NODE-VERSION}') - .replaceAll(`node ${process.version}`, '{NODE-VERSION}') .replaceAll(`${os.type()} ${os.release()}`, '{OS}') .replaceAll(`v${NPM_VERSION}`, '{NPM-VERSION}') From 797ac8e26341fe059fcedc8b7e5360e906338c0e Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 13:57:00 -0700 Subject: [PATCH 03/19] move stuff around --- test/lib/utils/exit-handler.js | 54 +++++++++++++++------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index 19bfacbef6050..af92611de0594 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -360,28 +360,25 @@ t.test('no logs dir', async (t) => { }) t.test('timers fail to write', async (t) => { - // we want the fs.writeFileSync in the Timers class to fail - const mockTimers = tmock(t, '{LIB}/utils/timers.js', { - 'node:fs': { - ...fs, - writeFileSync: (file, ...rest) => { - if (file.includes('LOGS_DIR')) { - throw new Error('err') - } - - return fs.writeFileSync(file, ...rest) - }, - }, - }) - const { exitHandler, logs } = await mockExitHandler(t, { config: (dirs) => ({ 'logs-dir': resolve(dirs.prefix, 'LOGS_DIR'), timing: true, }), mocks: { - // note, this is relative to test/fixtures/mock-npm.js not this file - '{LIB}/utils/timers.js': mockTimers, + // we want the fs.writeFileSync in the Timers class to fail + '{LIB}/utils/timers.js': tmock(t, '{LIB}/utils/timers.js', { + 'node:fs': { + ...fs, + writeFileSync: (file, ...rest) => { + if (file.includes('LOGS_DIR')) { + throw new Error('err') + } + + return fs.writeFileSync(file, ...rest) + }, + }, + }), }, }) @@ -391,25 +388,22 @@ t.test('timers fail to write', async (t) => { }) t.test('log files fail to write', async (t) => { - // we want the fsMiniPass.WriteStreamSync in the LogFile class to fail - const mockLogFile = tmock(t, '{LIB}/utils/log-file.js', { - 'fs-minipass': { - ...fsMiniPass, - WriteStreamSync: (file, ...rest) => { - if (file.includes('LOGS_DIR')) { - throw new Error('err') - } - }, - }, - }) - const { exitHandler, logs } = await mockExitHandler(t, { config: (dirs) => ({ 'logs-dir': resolve(dirs.prefix, 'LOGS_DIR'), }), mocks: { - // note, this is relative to test/fixtures/mock-npm.js not this file - '{LIB}/utils/log-file.js': mockLogFile, + // we want the fsMiniPass.WriteStreamSync in the LogFile class to fail + '{LIB}/utils/log-file.js': tmock(t, '{LIB}/utils/log-file.js', { + 'fs-minipass': { + ...fsMiniPass, + WriteStreamSync: (file, ...rest) => { + if (file.includes('LOGS_DIR')) { + throw new Error('err') + } + }, + }, + }), }, }) From 2f03b64e3cf9d8c6abe79e63b127c0efbd978f0c Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 14:09:36 -0700 Subject: [PATCH 04/19] do i need to chmod node too? --- test/bin/windows-shims.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 5e4f28127e1b3..e5f540f05f765 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -100,6 +100,10 @@ t.test('run shims', t => { }, }) + for (const shim of ['node.exe', ...Object.keys(SHIMS)]) { + chmodSync(join(path, shim), 0o755) + } + const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { if (cmd.endsWith('bash.exe')) { // only cygwin *requires* the -l, but the others are ok with it @@ -145,10 +149,6 @@ t.test('run shims', t => { } } - for (const shim of Object.keys(SHIMS)) { - chmodSync(join(path, shim), 0o755) - } - const { ProgramFiles = '/', SystemRoot = '/', NYC_CONFIG, WINDOWS_SHIMS_TEST } = process.env const skipDefault = WINDOWS_SHIMS_TEST || process.platform === 'win32' ? null : 'test not relevant on platform' From 451aaa5c154ad472f74f6fadbf0b0a717fbdbd4d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 14:15:55 -0700 Subject: [PATCH 05/19] comment --- test/bin/windows-shims.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index e5f540f05f765..22bf3bf0d0da9 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -100,6 +100,7 @@ t.test('run shims', t => { }, }) + // make sure to chmod node too for (const shim of ['node.exe', ...Object.keys(SHIMS)]) { chmodSync(join(path, shim), 0o755) } From f502e5094109ec1cb79947022b962bd8bf4ec8fc Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 14:20:34 -0700 Subject: [PATCH 06/19] another comment --- test/bin/windows-shims.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 22bf3bf0d0da9..8ebcb50e5dfbb 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -101,6 +101,7 @@ t.test('run shims', t => { }) // make sure to chmod node too + // you are seeing this comment because i need to make sure this works for (const shim of ['node.exe', ...Object.keys(SHIMS)]) { chmodSync(join(path, shim), 0o755) } From 5a5b4ee7a9adcb036e2902e40ace7807b1c284dc Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 15:05:34 -0700 Subject: [PATCH 07/19] try this --- test/bin/windows-shims.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 8ebcb50e5dfbb..9dae097fbd172 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -3,6 +3,7 @@ const { spawnSync } = require('child_process') const { resolve, join, extname, basename } = require('path') const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') const Diff = require('diff') +const { windows: rimrafWindows } = require('rimraf') const { sync: which } = require('which') const { version } = require('../../package.json') @@ -14,6 +15,14 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { return acc }, {}) +const rimrafWindowsForever = async (p) => { + try { + return await rimrafWindows(p, { force: true }) + } catch { + return rimrafWindowsForever(p) + } +} + const ROOT = resolve(__dirname, '../..') const BIN = join(ROOT, 'bin') const SHIMS = readNonJsFiles(BIN) @@ -106,6 +115,12 @@ t.test('run shims', t => { chmodSync(join(path, shim), 0o755) } + t.teardown(async () => { + // this runs before taps testdir teardown so we run forever until + // this file is successfully deleted + await rimrafWindowsForever(join(path, 'node.exe')) + }) + const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { if (cmd.endsWith('bash.exe')) { // only cygwin *requires* the -l, but the others are ok with it From 4f52720d428903fcdd594435231d988a8327b245 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 15:20:46 -0700 Subject: [PATCH 08/19] more --- test/bin/windows-shims.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 9dae097fbd172..817a758bbac8c 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,7 +1,9 @@ const t = require('tap') +const timers = require('timers/promises') const { spawnSync } = require('child_process') const { resolve, join, extname, basename } = require('path') const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') +const { access } = require('fs/promises') const Diff = require('diff') const { windows: rimrafWindows } = require('rimraf') const { sync: which } = require('which') @@ -15,11 +17,26 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { return acc }, {}) -const rimrafWindowsForever = async (p) => { +const rimrafWindowsForever = async (t, p, tries = 0) => { try { - return await rimrafWindows(p, { force: true }) - } catch { - return rimrafWindowsForever(p) + t.comment(`rimraf ${p}`) + await rimrafWindows(p, { force: true }) + t.comment(`rimraf:complete ${p}`) + + const hasAccess = await access(p).then(() => ({ err: false })).catch(err => ({ err })) + if (hasAccess.err) { + t.comment(`access ${hasAccess.err}`) + return + } + + throw new Error('Still has access') + } catch (err) { + t.comment(`rimraf:error ${err}`) + if (tries >= 100) { + throw err + } + await timers.setImmediate() + return rimrafWindowsForever(t, p, tries++) } } @@ -118,7 +135,7 @@ t.test('run shims', t => { t.teardown(async () => { // this runs before taps testdir teardown so we run forever until // this file is successfully deleted - await rimrafWindowsForever(join(path, 'node.exe')) + await rimrafWindowsForever(t, join(path, 'node.exe')) }) const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { From d3d35bbd8f0103fb3482863cb8a555ca86653676 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 15:32:02 -0700 Subject: [PATCH 09/19] more access comments --- test/bin/windows-shims.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 817a758bbac8c..b63a190118ade 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -3,7 +3,7 @@ const timers = require('timers/promises') const { spawnSync } = require('child_process') const { resolve, join, extname, basename } = require('path') const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') -const { access } = require('fs/promises') +const fsp = require('fs/promises') const Diff = require('diff') const { windows: rimrafWindows } = require('rimraf') const { sync: which } = require('which') @@ -21,15 +21,14 @@ const rimrafWindowsForever = async (t, p, tries = 0) => { try { t.comment(`rimraf ${p}`) await rimrafWindows(p, { force: true }) - t.comment(`rimraf:complete ${p}`) + t.comment(`rimraf:complete`) - const hasAccess = await access(p).then(() => ({ err: false })).catch(err => ({ err })) - if (hasAccess.err) { - t.comment(`access ${hasAccess.err}`) - return + const access = await fsp.access(p).then(() => true).catch(err => err) + if (access === true) { + throw new Error('access=true') } - throw new Error('Still has access') + t.comment(`access ${access}`) } catch (err) { t.comment(`rimraf:error ${err}`) if (tries >= 100) { From 8052053ba0c57e962ef6e80d7537bc176f8b4a4b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 15:44:29 -0700 Subject: [PATCH 10/19] more --- test/bin/windows-shims.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index b63a190118ade..83f3bd077829d 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -19,21 +19,25 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { const rimrafWindowsForever = async (t, p, tries = 0) => { try { - t.comment(`rimraf ${p}`) + t.comment(`rimraf ${tries} ${p}`) await rimrafWindows(p, { force: true }) t.comment(`rimraf:complete`) const access = await fsp.access(p).then(() => true).catch(err => err) + t.comment(`access ${access}`) + if (access === true) { throw new Error('access=true') + } else if (access.code === 'EPERM') { + throw new Error('access=EPERM, try again') } - - t.comment(`access ${access}`) } catch (err) { t.comment(`rimraf:error ${err}`) + if (tries >= 100) { - throw err + return } + await timers.setImmediate() return rimrafWindowsForever(t, p, tries++) } @@ -125,15 +129,11 @@ t.test('run shims', t => { }, }) - // make sure to chmod node too - // you are seeing this comment because i need to make sure this works - for (const shim of ['node.exe', ...Object.keys(SHIMS)]) { + for (const shim of Object.keys(SHIMS)) { chmodSync(join(path, shim), 0o755) } t.teardown(async () => { - // this runs before taps testdir teardown so we run forever until - // this file is successfully deleted await rimrafWindowsForever(t, join(path, 'node.exe')) }) From f98c1edc8e6030545a0a13466ee154104e33f0f8 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 15:50:05 -0700 Subject: [PATCH 11/19] try again --- test/bin/windows-shims.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 83f3bd077829d..c71792f55ff69 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -34,7 +34,7 @@ const rimrafWindowsForever = async (t, p, tries = 0) => { } catch (err) { t.comment(`rimraf:error ${err}`) - if (tries >= 100) { + if (tries >= 1000) { return } From 23288d167349cedb7d3e50391c72adaad41e3df7 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:08:00 -0700 Subject: [PATCH 12/19] less comments --- test/bin/windows-shims.js | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index c71792f55ff69..612fd9336eb3c 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -19,27 +19,19 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { const rimrafWindowsForever = async (t, p, tries = 0) => { try { - t.comment(`rimraf ${tries} ${p}`) await rimrafWindows(p, { force: true }) - t.comment(`rimraf:complete`) - - const access = await fsp.access(p).then(() => true).catch(err => err) - t.comment(`access ${access}`) - - if (access === true) { - throw new Error('access=true') - } else if (access.code === 'EPERM') { - throw new Error('access=EPERM, try again') + const access = await fsp.access(p).then(() => ({ code: true })).catch(err => err) + if (access.code === true || access.code === 'EPERM') { + throw new Error(`access=${access.code}, try again`) + } else { + return { result: 'success', tries } } } catch (err) { - t.comment(`rimraf:error ${err}`) - if (tries >= 1000) { - return + return { result: 'error', tries } } - - await timers.setImmediate() - return rimrafWindowsForever(t, p, tries++) + await timers.setTimeout(10) + return rimrafWindowsForever(t, p, ++tries) } } @@ -134,7 +126,8 @@ t.test('run shims', t => { } t.teardown(async () => { - await rimrafWindowsForever(t, join(path, 'node.exe')) + const res = await rimrafWindowsForever(t, join(path, 'node.exe')) + t.comment(`${res.status} rimraf node.exe in ${res.tries}`) }) const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { From 55baf3a44d7f090f942fc645084744fa82e0d0c3 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:13:25 -0700 Subject: [PATCH 13/19] comment in fn --- test/bin/windows-shims.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 612fd9336eb3c..f4cc8d1ad8422 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -17,22 +17,23 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { return acc }, {}) -const rimrafWindowsForever = async (t, p, tries = 0) => { +const rimrafWindowsForever = async (t, p, tries = 1) => { + let status try { await rimrafWindows(p, { force: true }) const access = await fsp.access(p).then(() => ({ code: true })).catch(err => err) if (access.code === true || access.code === 'EPERM') { throw new Error(`access=${access.code}, try again`) - } else { - return { result: 'success', tries } } - } catch (err) { - if (tries >= 1000) { - return { result: 'error', tries } + status = 'success' + } catch { + if (tries <= 1000) { + await timers.setTimeout(10) + return rimrafWindowsForever(t, p, ++tries) } - await timers.setTimeout(10) - return rimrafWindowsForever(t, p, ++tries) + status = 'error' } + t.comment(`rimraf ${p} ${status} ${tries}`) } const ROOT = resolve(__dirname, '../..') @@ -126,8 +127,7 @@ t.test('run shims', t => { } t.teardown(async () => { - const res = await rimrafWindowsForever(t, join(path, 'node.exe')) - t.comment(`${res.status} rimraf node.exe in ${res.tries}`) + await rimrafWindowsForever(t, join(path, 'node.exe')) }) const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { From 8234642b41f1e23230e321073050d49927c8edcd Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:19:41 -0700 Subject: [PATCH 14/19] use move remove to see if it is as reliable --- test/bin/windows-shims.js | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index f4cc8d1ad8422..950e5583f0939 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,11 +1,9 @@ const t = require('tap') -const timers = require('timers/promises') const { spawnSync } = require('child_process') const { resolve, join, extname, basename } = require('path') const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') -const fsp = require('fs/promises') const Diff = require('diff') -const { windows: rimrafWindows } = require('rimraf') +const { moveRemove } = require('rimraf') const { sync: which } = require('which') const { version } = require('../../package.json') @@ -17,25 +15,6 @@ const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { return acc }, {}) -const rimrafWindowsForever = async (t, p, tries = 1) => { - let status - try { - await rimrafWindows(p, { force: true }) - const access = await fsp.access(p).then(() => ({ code: true })).catch(err => err) - if (access.code === true || access.code === 'EPERM') { - throw new Error(`access=${access.code}, try again`) - } - status = 'success' - } catch { - if (tries <= 1000) { - await timers.setTimeout(10) - return rimrafWindowsForever(t, p, ++tries) - } - status = 'error' - } - t.comment(`rimraf ${p} ${status} ${tries}`) -} - const ROOT = resolve(__dirname, '../..') const BIN = join(ROOT, 'bin') const SHIMS = readNonJsFiles(BIN) @@ -127,7 +106,7 @@ t.test('run shims', t => { } t.teardown(async () => { - await rimrafWindowsForever(t, join(path, 'node.exe')) + await moveRemove(join(path, 'node.exe')) }) const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { From 36bf69506ca6f3e5df9e921cffedcfd469cdf72a Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:26:22 -0700 Subject: [PATCH 15/19] try moveRemove again --- test/bin/windows-shims.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 950e5583f0939..719c03a3b81ef 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -106,6 +106,7 @@ t.test('run shims', t => { } t.teardown(async () => { + // try again await moveRemove(join(path, 'node.exe')) }) From 1dfa420f4f083923814ed402cb066779600e9d50 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:31:27 -0700 Subject: [PATCH 16/19] try moveRemove again again --- test/bin/windows-shims.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 719c03a3b81ef..3ae0545810820 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -106,7 +106,7 @@ t.test('run shims', t => { } t.teardown(async () => { - // try again + // try again again await moveRemove(join(path, 'node.exe')) }) From 65552b652319d1f2fc455df5729660dc4a5205bc Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:42:23 -0700 Subject: [PATCH 17/19] try without libtap now that its just doing moveRemove on one file --- .gitignore | 1 - libtap-settings.js | 7 ------- package.json | 3 +-- scripts/template-oss/root.js | 1 - test/bin/windows-shims.js | 9 +++++---- 5 files changed, 6 insertions(+), 15 deletions(-) delete mode 100644 libtap-settings.js diff --git a/.gitignore b/.gitignore index 5944e2633a27b..2a10ae9bd8888 100644 --- a/.gitignore +++ b/.gitignore @@ -27,7 +27,6 @@ tap-testdir*/ !/docs/ !/index.js !/lib/ -!/libtap-settings.js !/LICENSE* !/map.js !/node_modules/ diff --git a/libtap-settings.js b/libtap-settings.js deleted file mode 100644 index 31d00333f01bf..0000000000000 --- a/libtap-settings.js +++ /dev/null @@ -1,7 +0,0 @@ -// use rimraf for tap recursive directory removal, so that -// the windows tests don't flake with fs errors -const { rimraf } = require('rimraf') -module.exports = { - rmdirRecursiveSync: path => rimraf.sync(path), - rmdirRecursive: (path, cb) => rimraf(path).then(() => cb(), cb), -} diff --git a/package.json b/package.json index 4fadcf2cbccd6..3189a5adf59b6 100644 --- a/package.json +++ b/package.json @@ -255,8 +255,7 @@ "--exclude", "tap-snapshots/**" ], - "test-ignore": "^(docs|smoke-tests|mock-globals|mock-registry|workspaces)/", - "libtap-settings": "./libtap-settings.js" + "test-ignore": "^(docs|smoke-tests|mock-globals|mock-registry|workspaces)/" }, "templateOSS": { "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.", diff --git a/scripts/template-oss/root.js b/scripts/template-oss/root.js index a1a28e60f4b79..fb2d964a3f993 100644 --- a/scripts/template-oss/root.js +++ b/scripts/template-oss/root.js @@ -66,7 +66,6 @@ module.exports = { '/.mailmap', '/.licensee.json', '/.gitattributes', - '/libtap-settings.js', ], ignorePaths: [ '/node_modules/.bin/', diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 3ae0545810820..72376ab456055 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -105,10 +105,11 @@ t.test('run shims', t => { chmodSync(join(path, shim), 0o755) } - t.teardown(async () => { - // try again again - await moveRemove(join(path, 'node.exe')) - }) + // The removal of this fixture causes this test for fail when done with + // the default tap removal. Using rimraf's `moveRemove` seems to make this + // work reliably. Don't remove this line in the future without making sure + // this test passes the full windows suite at least 3 consecutive times. + t.teardown(() => moveRemove(join(path, 'node.exe'))) const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { if (cmd.endsWith('bash.exe')) { From 35b2c56e920683fd5632a1305fb2a4527f1dd0ca Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 16:50:52 -0700 Subject: [PATCH 18/19] one more time for good luck --- test/bin/windows-shims.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 72376ab456055..ee40d67c2f46c 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -101,10 +101,6 @@ t.test('run shims', t => { }, }) - for (const shim of Object.keys(SHIMS)) { - chmodSync(join(path, shim), 0o755) - } - // The removal of this fixture causes this test for fail when done with // the default tap removal. Using rimraf's `moveRemove` seems to make this // work reliably. Don't remove this line in the future without making sure @@ -156,6 +152,10 @@ t.test('run shims', t => { } } + for (const shim of Object.keys(SHIMS)) { + chmodSync(join(path, shim), 0o755) + } + const { ProgramFiles = '/', SystemRoot = '/', NYC_CONFIG, WINDOWS_SHIMS_TEST } = process.env const skipDefault = WINDOWS_SHIMS_TEST || process.platform === 'win32' ? null : 'test not relevant on platform' From 5430f76ed7eb04eba47c2b34345a65bd5f1f8fd9 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 18 Apr 2024 18:11:21 -0700 Subject: [PATCH 19/19] Update windows-shims.js Co-authored-by: Gar --- test/bin/windows-shims.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index ee40d67c2f46c..71f12dc8e1cdd 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -101,7 +101,7 @@ t.test('run shims', t => { }, }) - // The removal of this fixture causes this test for fail when done with + // The removal of this fixture causes this test to fail when done with // the default tap removal. Using rimraf's `moveRemove` seems to make this // work reliably. Don't remove this line in the future without making sure // this test passes the full windows suite at least 3 consecutive times.