From 6fc87bd0f8e8610f4047f279b10e8f903b3816de Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 8 May 2024 09:41:44 -0700 Subject: [PATCH 1/4] chore: disable progress on shellout exit tests These tests assert what happens if a shellout command like exec throws unexpected errors by checking what is written to stderr. Progress also gets written to stderr but is not always deterministic due to the nature of calling it via setInterval. So this disables progress for these tests so the stderr assertions can be relied on to be the same. --- test/lib/cli/exit-handler.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js index 73cc57cb9a676..a95ac75e56c14 100644 --- a/test/lib/cli/exit-handler.js +++ b/test/lib/cli/exit-handler.js @@ -652,6 +652,7 @@ t.test('do no fancy handling for shellouts', async t => { argv: ['-c', 'exit'], config: { timing: false, + progress: false, ...opts.config, }, ...opts, From b5086c049d2f967714bb789991bde8e56681394e Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 8 May 2024 09:44:26 -0700 Subject: [PATCH 2/4] chore: disable color in config tests The logging output was making bare string assertions fail --- tap-snapshots/test/lib/commands/config.js.test.cjs | 8 ++++++-- test/lib/commands/config.js | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index b86820a306029..ace89ec9a719d 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -8,6 +8,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches snapshot 1`] = ` { "cache": "{CACHE}", + "color": {COLOR}, "json": true, "projectloaded": "yes", "userloaded": "yes", @@ -29,7 +30,6 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "call": "", "cert": null, "cidr": null, - "color": {COLOR}, "commit-hooks": true, "cpu": null, "depth": null, @@ -192,7 +192,7 @@ cafile = null call = "" cert = null cidr = null -color = {COLOR} +; color = {COLOR} commit-hooks = true cpu = null depth = null @@ -345,6 +345,7 @@ projectloaded = "yes" ; "cli" config from command line options cache = "{CACHE}" +color = {COLOR} long = true ` @@ -364,6 +365,7 @@ projectloaded = "yes" ; "cli" config from command line options cache = "{CACHE}" +color = {COLOR} ; node bin location = {NODE-BIN-LOCATION} ; node version = {NODE-VERSION} @@ -378,6 +380,7 @@ exports[`test/lib/commands/config.js TAP config list with publishConfig global > ; "cli" config from command line options cache = "{CACHE}" +color = {COLOR} global = true ; node bin location = {NODE-BIN-LOCATION} @@ -393,6 +396,7 @@ exports[`test/lib/commands/config.js TAP config list with publishConfig local > ; "cli" config from command line options cache = "{CACHE}" +color = {COLOR} ; node bin location = {NODE-BIN-LOCATION} ; node version = {NODE-VERSION} diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js index 0806326e2e8e4..11ac9a7477eb1 100644 --- a/test/lib/commands/config.js +++ b/test/lib/commands/config.js @@ -37,7 +37,7 @@ const loadMockNpm = (t, opts = {}) => _loadMockNpm(t, { // Reset configs that mock npm sets by default 'fetch-retries': undefined, loglevel: undefined, - color: undefined, + color: false, }, }) From 4223f73ce1aa88caa345e73e38d1f592952b301e Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 8 May 2024 09:50:05 -0700 Subject: [PATCH 3/4] chore: disable progress on npm pack test --- test/lib/commands/pack.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/lib/commands/pack.js b/test/lib/commands/pack.js index 3ea67c78d996a..fd525114cde9f 100644 --- a/test/lib/commands/pack.js +++ b/test/lib/commands/pack.js @@ -82,7 +82,10 @@ t.test('should log scoped package output as valid json', async t => { }, }), }, - config: { json: true }, + config: { + json: true, + progress: false, + }, }) await npm.exec('pack', []) const filename = 'myscope-test-package-1.0.0.tgz' From 64ab520833f707eea8ffc20180c305568dc3c8f2 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 8 May 2024 09:57:59 -0700 Subject: [PATCH 4/4] fix: avoid caching manifests as promises Originally this was in https://github.com/npm/cli/pull/7468: We backed off of it while we were rebuilding pacote's packument cache. Now that that's done we can assess this in isolation. I think it makes sense. The packument is cached here, all this is awaiting is normalization and ssri calculation. The only place this potentially does anything is in the premature loading of manifests in `#buildDepStep` when looking at problem edges. We can just wait till we need them and not throw a ton of requests in parallel before we actually need them. Removing the premature loading in problem edges will have to be a separate effort, as it is somehow load bearing --- .../arborist/lib/arborist/build-ideal-tree.js | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 3a35810a76e83..d0d159976c5db 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1022,6 +1022,10 @@ This is a one-time fix-up, please be patient... // may well be an optional dep that has gone missing. it'll // fail later anyway. for (const e of this.#problemEdges(placed)) { + // XXX This is somehow load bearing. This makes tests that print + // the ideal tree of a tree with tarball dependencies fail. This + // can't be changed or removed till we figure out why + // The test is named "tarball deps with transitive tarball deps" promises.push(() => this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e))) .catch(() => null) @@ -1204,6 +1208,7 @@ This is a one-time fix-up, please be patient... const options = { ...this.options, avoid: this.#avoidRange(spec.name), + fullMetadata: true, } // get the intended spec and stored metadata from yarn.lock file, // if available and valid. @@ -1212,19 +1217,10 @@ This is a one-time fix-up, please be patient... if (this.#manifests.has(spec.raw)) { return this.#manifests.get(spec.raw) } else { - const cleanRawSpec = redact(spec.rawSpec) - log.silly('fetch manifest', spec.raw.replace(spec.rawSpec, cleanRawSpec)) - const o = { - ...options, - fullMetadata: true, - } - const p = pacote.manifest(spec, o) - .then((mani) => { - this.#manifests.set(spec.raw, mani) - return mani - }) - this.#manifests.set(spec.raw, p) - return p + log.silly('fetch manifest', spec.raw.replace(spec.rawSpec, redact(spec.rawSpec))) + const mani = await pacote.manifest(spec, options) + this.#manifests.set(spec.raw, mani) + return mani } }