From 4d224ee30e039853b4594dbe29e03b17b0d26166 Mon Sep 17 00:00:00 2001 From: cod1r Date: Sat, 9 Mar 2024 13:28:00 -0600 Subject: [PATCH 1/2] fix: added check for dry-run added a check for dry-run for `npm ci` so that node_modules won't be deleted closes #7239 --- lib/commands/ci.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 706b77ac361cf..b39126cff7a3b 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -75,14 +75,16 @@ class CI extends ArboristWorkspaceCmd { ) } - // Only remove node_modules after we've successfully loaded the virtual - // tree and validated the lockfile - await this.npm.time('npm-ci:rm', async () => { - const path = `${where}/node_modules` - // get the list of entries so we can skip the glob for performance - const entries = await fs.readdir(path, null).catch(er => []) - return Promise.all(entries.map(f => fs.rm(`${path}/${f}`, { force: true, recursive: true }))) - }) + if (!this.npm.flatOptions.dryRun) { + // Only remove node_modules after we've successfully loaded the virtual + // tree and validated the lockfile + await this.npm.time('npm-ci:rm', async () => { + const path = `${where}/node_modules` + // get the list of entries so we can skip the glob for performance + const entries = await fs.readdir(path, null).catch(er => []) + return Promise.all(entries.map(f => fs.rm(`${path}/${f}`, { force: true, recursive: true }))) + }) + } await arb.reify(opts) From 3a7a20a782662135fdb2d651980f290b6098f546 Mon Sep 17 00:00:00 2001 From: cod1r Date: Sun, 10 Mar 2024 07:13:24 -0500 Subject: [PATCH 2/2] chore: added test for ci --dry-run check --- lib/commands/ci.js | 6 ++++-- test/lib/commands/ci.js | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/commands/ci.js b/lib/commands/ci.js index b39126cff7a3b..428c43e6c30ed 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -75,14 +75,16 @@ class CI extends ArboristWorkspaceCmd { ) } - if (!this.npm.flatOptions.dryRun) { + const dryRun = this.npm.config.get('dry-run') + if (!dryRun) { // Only remove node_modules after we've successfully loaded the virtual // tree and validated the lockfile await this.npm.time('npm-ci:rm', async () => { const path = `${where}/node_modules` // get the list of entries so we can skip the glob for performance const entries = await fs.readdir(path, null).catch(er => []) - return Promise.all(entries.map(f => fs.rm(`${path}/${f}`, { force: true, recursive: true }))) + return Promise.all(entries.map(f => fs.rm(`${path}/${f}`, + { force: true, recursive: true }))) }) } diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 000ddc0eb8270..681ccad7d87a7 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -56,6 +56,26 @@ const abbrev = { test: 'test file', } +t.test('reifies, but doesn\'t remove node_modules because --dry-run', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { + 'dry-run': true, + }, + prefixDir: { + abbrev: abbrev, + 'package.json': JSON.stringify(packageJson), + 'package-lock.json': JSON.stringify(packageLock), + node_modules: { test: 'test file that will not be removed' }, + }, + }) + await npm.exec('ci', []) + t.match(joinedOutput(), 'added 1 package, and removed 1 package in') + const nmTest = path.join(npm.prefix, 'node_modules', 'test') + t.equal(fs.existsSync(nmTest), true, 'existing node_modules is not removed') + const nmAbbrev = path.join(npm.prefix, 'node_modules', 'abbrev') + t.equal(fs.existsSync(nmAbbrev), false, 'does not install abbrev') +}) + t.test('reifies, audits, removes node_modules', async t => { const { npm, joinedOutput, registry } = await loadMockNpm(t, { prefixDir: {