From c387216117b47d402378ceb1e5db70a2667e2795 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 30 Mar 2022 07:46:50 -0700 Subject: [PATCH 1/6] chore: remove unneeded otplease mocks --- test/lib/auth/sso.js | 11 ----------- test/lib/commands/deprecate.js | 1 - test/lib/commands/hook.js | 1 - test/lib/commands/org.js | 1 - test/lib/commands/owner.js | 3 --- test/lib/commands/profile.js | 4 ---- test/lib/commands/team.js | 1 - test/lib/commands/token.js | 3 --- 8 files changed, 25 deletions(-) diff --git a/test/lib/auth/sso.js b/test/lib/auth/sso.js index 473c8cc241467..eab51f06c0ab2 100644 --- a/test/lib/auth/sso.js +++ b/test/lib/auth/sso.js @@ -29,16 +29,6 @@ const sso = t.mock('../../../lib/auth/sso.js', { ) } }, - '../../../lib/utils/otplease.js': (opts, fn) => { - if (opts) { - return fn({ ...opts, otp: '1234' }) - } else { - throw Object.assign( - new Error('failed retrieving otp'), - { code: 'ERROR' } - ) - } - }, }) const npm = { @@ -75,7 +65,6 @@ t.test('simple login', async (t) => { opts, { creds: {}, - otp: '1234', registry: 'https://registry.npmjs.org/', scope: '', ssoType: 'oauth', diff --git a/test/lib/commands/deprecate.js b/test/lib/commands/deprecate.js index 180629a7488f9..37a407c3b6a1a 100644 --- a/test/lib/commands/deprecate.js +++ b/test/lib/commands/deprecate.js @@ -19,7 +19,6 @@ npmFetch.json = async (uri, opts) => { const Deprecate = t.mock('../../../lib/commands/deprecate.js', { '../../../lib/utils/get-identity.js': async () => getIdentityImpl(), - '../../../lib/utils/otplease.js': async (opts, fn) => fn(opts), libnpmaccess: { lsPackages: async () => ({ foo: 'write', bar: 'write', baz: 'write', buzz: 'read' }), }, diff --git a/test/lib/commands/hook.js b/test/lib/commands/hook.js index 60a59a3fe7a3b..95bac83d7b6eb 100644 --- a/test/lib/commands/hook.js +++ b/test/lib/commands/hook.js @@ -63,7 +63,6 @@ const libnpmhook = { } const Hook = t.mock('../../../lib/commands/hook.js', { - '../../../lib/utils/otplease.js': async (opts, fn) => fn(opts), libnpmhook, }) const hook = new Hook(npm) diff --git a/test/lib/commands/org.js b/test/lib/commands/org.js index 3ae951dd5c453..cd25fc23aa334 100644 --- a/test/lib/commands/org.js +++ b/test/lib/commands/org.js @@ -43,7 +43,6 @@ const libnpmorg = { } const Org = t.mock('../../../lib/commands/org.js', { - '../../../lib/utils/otplease.js': async (opts, fn) => fn(opts), libnpmorg, }) const org = new Org(npm) diff --git a/test/lib/commands/owner.js b/test/lib/commands/owner.js index 83f4d880f74ad..eadfa2bf08b56 100644 --- a/test/lib/commands/owner.js +++ b/test/lib/commands/owner.js @@ -21,7 +21,6 @@ const mocks = { 'proc-log': log, 'npm-registry-fetch': npmFetch, pacote, - '../../../lib/utils/otplease.js': async (opts, fn) => fn({ otp: '123456', opts }), '../../../lib/utils/read-package-name.js': async (prefix) => { readPackageNamePrefix = prefix return readPackageNameResponse @@ -193,7 +192,6 @@ t.test('owner add ', async t => { _rev: '1-foobaaa1', maintainers: npmcliMaintainers, }, - otp: '123456', spec: { name: '@npmcli/map-workspaces', }, @@ -507,7 +505,6 @@ t.test('owner rm ', async t => { body: { _rev: '1-foobaaa1', }, - otp: '123456', spec: { name: '@npmcli/map-workspaces', }, diff --git a/test/lib/commands/profile.js b/test/lib/commands/profile.js index 82ac495420928..da32d4300840a 100644 --- a/test/lib/commands/profile.js +++ b/test/lib/commands/profile.js @@ -41,10 +41,6 @@ const mocks = { .join('\n') } }, - '../../../lib/utils/pulse-till-done.js': { - withPromise: async a => a, - }, - '../../../lib/utils/otplease.js': async (opts, fn) => fn(opts), '../../../lib/utils/read-user-info.js': { async password () {}, async otp () {}, diff --git a/test/lib/commands/team.js b/test/lib/commands/team.js index 592dbc3a0ec2e..792418788bcd1 100644 --- a/test/lib/commands/team.js +++ b/test/lib/commands/team.js @@ -22,7 +22,6 @@ const npm = mockNpm({ const mocks = { libnpmteam, 'cli-columns': a => a.join(' '), - '../../../lib/utils/otplease.js': async (opts, fn) => fn(opts), } t.afterEach(() => { diff --git a/test/lib/commands/token.js b/test/lib/commands/token.js index 65a094a0bca24..c32c0b74a96f4 100644 --- a/test/lib/commands/token.js +++ b/test/lib/commands/token.js @@ -10,9 +10,6 @@ const npm = { } const mockToken = (otherMocks) => t.mock('../../../lib/commands/token.js', { - '../../../lib/utils/otplease.js': (opts, fn) => { - return Promise.resolve().then(() => fn(opts)) - }, '../../../lib/utils/read-user-info.js': mocks.readUserInfo, 'npm-profile': mocks.profile, ...otherMocks, From 16a1ac9357b4ade9d79a87ea3f48b6f08fa3a307 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 30 Mar 2022 15:37:01 -0700 Subject: [PATCH 2/6] fix: remove dedupe --save * Removed dedupe --save documentation and attempted implementation. * Remove some unneeded otplease mocks from test `npm dedupe --save` didn't work in a easy to understand way. It would only update a top level dependency that was duplicated in the tree. Found this out rewriting the dedupe tests to be real. This is not very intuitive and it's best if folks use update or install for saving to package.json. --- docs/content/commands/npm-dedupe.md | 24 +- lib/commands/dedupe.js | 13 +- lib/npm.js | 3 + .../test/lib/load-all-commands.js.test.cjs | 1 - .../test/lib/utils/npm-usage.js.test.cjs | 1 - test/fixtures/mock-npm.js | 4 + test/fixtures/mock-registry.js | 113 +++++++ test/lib/commands/dedupe.js | 103 +++++-- test/lib/commands/unpublish.js | 281 ++++++++++-------- test/lib/npm.js | 11 +- 10 files changed, 357 insertions(+), 197 deletions(-) create mode 100644 test/fixtures/mock-registry.js diff --git a/docs/content/commands/npm-dedupe.md b/docs/content/commands/npm-dedupe.md index b5a64831c0bba..91f0105e35f21 100644 --- a/docs/content/commands/npm-dedupe.md +++ b/docs/content/commands/npm-dedupe.md @@ -80,11 +80,9 @@ result in new modules being installed. Using `npm find-dupes` will run the command in `--dry-run` mode. -Note that by default `npm dedupe` will not update the semver values of direct -dependencies in your project `package.json`, if you want to also update -values in `package.json` you can run: `npm dedupe --save` (or add the -`save=true` option to a [configuration file](/configuring-npm/npmrc) -to make that the default behavior). +Note: `npm dedupe` will never update the semver values of direct +dependencies in your project `package.json`, if you want to update +values in `package.json` you can run: `npm update --save` instead. ### Configuration @@ -158,22 +156,6 @@ This configuration does not affect `npm ci`. -#### `save` - -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` -* Type: Boolean - -Save installed packages to a `package.json` file as dependencies. - -When used with the `npm rm` command, removes the dependency from -`package.json`. - -Will also prevent writing to `package-lock.json` if set to `false`. - - - - #### `omit` * Default: 'dev' if the `NODE_ENV` environment variable is set to diff --git a/lib/commands/dedupe.js b/lib/commands/dedupe.js index 4662ce3241b24..96d1ac2ae9a74 100644 --- a/lib/commands/dedupe.js +++ b/lib/commands/dedupe.js @@ -12,7 +12,6 @@ class Dedupe extends ArboristWorkspaceCmd { 'legacy-bundling', 'strict-peer-deps', 'package-lock', - 'save', 'omit', 'ignore-scripts', 'audit', @@ -29,19 +28,17 @@ class Dedupe extends ArboristWorkspaceCmd { throw er } - // In the context of `npm dedupe` the save - // config value should default to `false` - const save = this.npm.config.isDefault('save') - ? false - : this.npm.config.get('save') - const dryRun = this.npm.config.get('dry-run') const where = this.npm.prefix const opts = { ...this.npm.flatOptions, path: where, dryRun, - save, + // Saving during dedupe would only update if one of your direct + // dependencies was also duplicated somewhere in your tree. It would be + // confusing if running this were to also update your package.json. In + // order to reduce potential confusion we set this to false. + save: false, workspaces: this.workspaceNames, } const arb = new Arborist(opts) diff --git a/lib/npm.js b/lib/npm.js index 74825c97c2355..c819a0807b507 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -131,6 +131,7 @@ class Npm extends EventEmitter { const isGlobal = this.config.get('global') const workspacesEnabled = this.config.get('workspaces') + // if cwd is a workspace, the default is set to [that workspace] const implicitWorkspace = this.config.get('workspace', 'default').length > 0 const workspacesFilters = this.config.get('workspace') const includeWorkspaceRoot = this.config.get('include-workspace-root') @@ -138,6 +139,8 @@ class Npm extends EventEmitter { // or when it is implicit and not in our ignore list const hasWorkspaceFilters = workspacesFilters.length > 0 const invalidWorkspaceConfig = workspacesEnabled === false && hasWorkspaceFilters + + // (-ws || -w foo) && (cwd is not a workspace || command is not ignoring implicit workspaces) const filterByWorkspaces = (workspacesEnabled || hasWorkspaceFilters) && (!implicitWorkspace || !command.ignoreImplicitWorkspace) // normally this would go in the constructor, but our tests don't diff --git a/tap-snapshots/test/lib/load-all-commands.js.test.cjs b/tap-snapshots/test/lib/load-all-commands.js.test.cjs index 0ad87b945fce7..cd8b0592c36e8 100644 --- a/tap-snapshots/test/lib/load-all-commands.js.test.cjs +++ b/tap-snapshots/test/lib/load-all-commands.js.test.cjs @@ -161,7 +161,6 @@ npm dedupe Options: [--global-style] [--legacy-bundling] [--strict-peer-deps] [--no-package-lock] -[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer|--save-bundle] [--omit [--omit ...]] [--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] [-w|--workspace [-w|--workspace ...]] diff --git a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs index e6afc973ab10e..fef4cc65edc65 100644 --- a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs +++ b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs @@ -293,7 +293,6 @@ All commands: Options: [--global-style] [--legacy-bundling] [--strict-peer-deps] [--no-package-lock] - [-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer|--save-bundle] [--omit [--omit ...]] [--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] [-w|--workspace [-w|--workspace ...]] diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 13f8a0ea01c7e..07e4cb8283b4c 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -83,6 +83,10 @@ const LoadMockNpm = async (t, { const { Npm, ...rest } = RealMockNpm(t, mocks) + // We want to fail fast when writing tests. Default this to 0 unless it was + // explicitly set in a test. + config = { 'fetch-retries': 0, ...config } + if (!init && load) { throw new Error('cant `load` without `init`') } diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js new file mode 100644 index 0000000000000..ec4ebc2a4c24f --- /dev/null +++ b/test/fixtures/mock-registry.js @@ -0,0 +1,113 @@ +/* + * Mock registry class + * + * This should end up as the centralized place where we generate test fixtures + * for tests against any registry data. + */ +const pacote = require('pacote') +class MockRegistry { + #tap + #nock + #registry + #authorization + + constructor (opts) { + if (!opts.registry) { + throw new Error('mock registry requires a registry value') + } + this.#registry = (new URL(opts.registry)).origin + this.#authorization = opts.authorization + // Required for this.package + this.#tap = opts.tap + } + + get nock () { + if (!this.#nock) { + const tnock = require('./tnock.js') + const reqheaders = {} + if (this.#authorization) { + reqheaders.authorization = `Bearer ${this.#authorization}` + } + this.#nock = tnock(this.#tap, this.#registry, { reqheaders }) + } + return this.#nock + } + + set nock (nock) { + this.#nock = nock + } + + async package ({ manifest, times = 1, query, tarballs }) { + let nock = this.nock + if (!nock) { + throw new Error('cannot mock packages without a tap fixture') + } + nock = nock.get(`/${manifest.name}`).times(times) + if (query) { + nock = nock.query(query) + } + nock = nock.reply(200, manifest) + if (tarballs) { + for (const version in manifest.versions) { + const packument = manifest.versions[version] + const dist = new URL(packument.dist.tarball) + const tarball = await pacote.tarball(tarballs[version]) + nock.get(dist.pathname).reply(200, tarball) + } + } + this.nock = nock + } + + // the last packument in the packuments array will be tagged as latest + manifest ({ name = 'test-package', packuments } = {}) { + packuments = this.packuments(packuments, name) + const latest = packuments.slice(-1)[0] + const manifest = { + _id: `${name}@${latest.version}`, + _rev: '00-testdeadbeef', + name, + description: 'test package mock manifest', + dependencies: {}, + versions: {}, + time: {}, + 'dist-tags': { latest: latest.version }, + ...latest, + } + + for (const packument of packuments) { + manifest.versions[packument.version] = { + _id: `${name}@${packument.version}`, + name, + description: 'test package mock manifest', + dependencies: {}, + dist: { + tarball: `${this.#registry}/${name}/-/${name}-${packument.version}.tgz`, + }, + ...packument, + } + manifest.time[packument.version] = new Date() + } + + return manifest + } + + packuments (packuments = ['1.0.0'], name) { + return packuments.map(p => this.packument(p, name)) + } + + // Generate packument from shorthand + packument (packument, name = 'test-package') { + if (!packument.version) { + packument = { version: packument } + } + return { + name, + version: '1.0.0', + description: 'mocked test package', + dependencies: {}, + ...packument, + } + } +} + +module.exports = MockRegistry diff --git a/test/lib/commands/dedupe.js b/test/lib/commands/dedupe.js index bf6964081ca79..a985f4a710753 100644 --- a/test/lib/commands/dedupe.js +++ b/test/lib/commands/dedupe.js @@ -1,5 +1,9 @@ const t = require('tap') const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const path = require('path') +const fs = require('fs') + +const MockRegistry = require('../../fixtures/mock-registry.js') t.test('should throw in global mode', async (t) => { const { npm } = await loadMockNpm(t, { @@ -14,45 +18,78 @@ t.test('should throw in global mode', async (t) => { ) }) -t.test('should remove dupes using Arborist', async (t) => { - t.plan(5) - const { npm } = await loadMockNpm(t, { - mocks: { - '@npmcli/arborist': function (args) { - t.ok(args, 'gets options object') - t.ok(args.path, 'gets path option') - t.ok(args.dryRun, 'gets dryRun from user') - this.dedupe = () => { - t.ok(true, 'dedupe is called') - } - }, - '../../lib/utils/reify-finish.js': (npm, arb) => { - t.ok(arb, 'gets arborist tree') +const testTop = { + name: 'test-top', + version: '1.0.0', + dependencies: { + 'test-dep-a': '*', + 'test-dep-b': '*', + }, +} +const testDepA = { + name: 'test-dep-a', + version: '1.0.1', + dependencies: { 'test-sub': '*' }, +} +const testDepB = { + name: 'test-dep-b', + version: '1.0.0', + dependencies: { 'test-sub': '*' }, +} +const testSub = { + name: 'test-sub', + version: '1.0.0', +} + +const treeWithDupes = { + 'package.json': JSON.stringify(testTop), + node_modules: { + 'test-dep-a': { + 'package.json': JSON.stringify(testDepA), + node_modules: { + 'test-sub': { + 'package.json': JSON.stringify(testSub), + }, }, }, - config: { - 'dry-run': 'true', + 'test-dep-b': { + 'package.json': JSON.stringify(testDepB), + node_modules: { + 'test-sub': { + 'package.json': JSON.stringify(testSub), + }, + }, }, + }, +} + +t.test('dedupe', async (t) => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: treeWithDupes, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifestSub = registry.manifest({ + name: 'test-sub', + packuments: [{ version: '1.0.0' }], }) - await npm.exec('dedupe', []) -}) -t.test('should remove dupes using Arborist - no arguments', async (t) => { - t.plan(2) - const { npm } = await loadMockNpm(t, { - mocks: { - '@npmcli/arborist': function (args) { - t.ok(args.dryRun, 'gets dryRun from config') - t.ok(args.save, 'gets user-set save value from config') - this.dedupe = () => {} - }, - '../../lib/utils/reify-output.js': () => {}, - '../../lib/utils/reify-finish.js': () => {}, - }, - config: { - 'dry-run': true, - save: true, + await registry.package({ + manifest: manifestSub, + tarballs: { + '1.0.0': path.join(npm.prefix, 'node_modules', 'test-dep-a', 'node_modules', 'test-sub'), }, }) await npm.exec('dedupe', []) + t.match(joinedOutput(), /added 1 package, and removed 2 packages/) + t.ok(fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-sub')), 'test-sub was hoisted') + t.notOk( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-a', 'node_modules', 'test-sub')), + 'test-dep-a/test-sub was removed' + ) + t.notOk( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-b', 'node_modules', 'test-sub')), + 'test-dep-b/test-sub was removed') }) diff --git a/test/lib/commands/unpublish.js b/test/lib/commands/unpublish.js index 71be4a5d6908d..829d41c5bb875 100644 --- a/test/lib/commands/unpublish.js +++ b/test/lib/commands/unpublish.js @@ -1,36 +1,7 @@ const t = require('tap') const { load: loadMockNpm } = require('../../fixtures/mock-npm') -const tnock = require('../../fixtures/tnock.js') - -const pkgManifest = (name, version = '1.0.0') => { - return { - _id: `${name}@${version}`, - _rev: '00-testdeadbeef', - name, - versions: { - '1.0.0': { - name, - version: '1.0.0', - dist: { - tarball: `https://registry.npmjs.org/${name}/-/${name}-1.0.0.tgz`, - }, - }, - [version]: { - name, - version: version, - dist: { - tarball: `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz`, - }, - }, - }, - time: { - '1.0.0': new Date(), - [version]: new Date(), - }, - 'dist-tags': { latest: version }, - } -} +const MockRegistry = require('../../fixtures/mock-registry.js') const user = 'test-user' const pkg = 'test-package' const auth = { '//registry.npmjs.org/:_authToken': 'test-auth-token' } @@ -49,12 +20,14 @@ t.test('no args --force success', async t => { }, }) - const manifest = pkgManifest(pkg) - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) - .delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) - + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true } }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', []) t.equal(joinedOutput(), '- test-package@1.0.0') }) @@ -116,14 +89,20 @@ t.test('unpublish @version not the last version', async t => { ...auth, }, }) - const manifest = pkgManifest(pkg, '1.0.1') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).times(3).reply(200, manifest) - .put(`/${pkg}/-rev/${manifest._rev}`, body => { - // sets latest and deletes version 1.0.1 - return body['dist-tags'].latest === '1.0.0' && body.versions['1.0.1'] === undefined - }).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ + name: pkg, + packuments: ['1.0.0', '1.0.1'], + }) + await registry.package({ manifest, query: { write: true }, times: 3 }) + registry.nock.put(`/${pkg}/-rev/${manifest._rev}`, body => { + // sets latest and deletes version 1.0.1 + return body['dist-tags'].latest === '1.0.0' && body.versions['1.0.1'] === undefined + }).reply(201) .intercept(`/${pkg}/-/${pkg}-1.0.1.tgz/-rev/${manifest._rev}`, 'DELETE').reply(201) await npm.exec('unpublish', ['test-package@1.0.1']) @@ -136,10 +115,13 @@ t.test('unpublish @version last version', async t => { ...auth, }, }) - const manifest = pkgManifest(pkg) - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true } }) await t.rejects( npm.exec('unpublish', ['test-package@1.0.0']), @@ -159,13 +141,14 @@ t.test('no version found in package.json', async t => { }, null, 2), }, }) - - const manifest = pkgManifest(pkg) - - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) - .delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true } }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', []) t.equal(joinedOutput(), '- test-package') @@ -178,11 +161,14 @@ t.test('unpublish --force no version set', async t => { ...auth, }, }) - const manifest = pkgManifest(pkg) - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).times(2).reply(200, manifest) - .delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true }, times: 2 }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', ['test-package']) t.equal(joinedOutput(), '- test-package') @@ -196,14 +182,20 @@ t.test('silent', async t => { ...auth, }, }) - const manifest = pkgManifest(pkg, '1.0.1') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).times(3).reply(200, manifest) - .put(`/${pkg}/-rev/${manifest._rev}`, body => { - // sets latest and deletes version 1.0.1 - return body['dist-tags'].latest === '1.0.0' && body.versions['1.0.1'] === undefined - }).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ + name: pkg, + packuments: ['1.0.0', '1.0.1'], + }) + await registry.package({ manifest, query: { write: true }, times: 3 }) + registry.nock.put(`/${pkg}/-rev/${manifest._rev}`, body => { + // sets latest and deletes version 1.0.1 + return body['dist-tags'].latest === '1.0.0' && body.versions['1.0.1'] === undefined + }).reply(201) .delete(`/${pkg}/-/${pkg}-1.0.1.tgz/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', ['test-package@1.0.1']) @@ -261,16 +253,19 @@ t.test('workspaces', async t => { }, prefixDir, }) - const manifestA = pkgManifest('workspace-a') - const manifestB = pkgManifest('workspace-b') - const manifestN = pkgManifest('workspace-n') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/workspace-a?write=true').times(2).reply(200, manifestA) - .delete(`/workspace-a/-rev/${manifestA._rev}`).reply(201) - .get('/workspace-b?write=true').times(2).reply(200, manifestB) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifestA = registry.manifest({ name: 'workspace-a' }) + const manifestB = registry.manifest({ name: 'workspace-b' }) + const manifestN = registry.manifest({ name: 'workspace-n' }) + await registry.package({ manifest: manifestA, query: { write: true }, times: 2 }) + await registry.package({ manifest: manifestB, query: { write: true }, times: 2 }) + await registry.package({ manifest: manifestN, query: { write: true }, times: 2 }) + registry.nock.delete(`/workspace-a/-rev/${manifestA._rev}`).reply(201) .delete(`/workspace-b/-rev/${manifestB._rev}`).reply(201) - .get('/workspace-n?write=true').times(2).reply(200, manifestN) .delete(`/workspace-n/-rev/${manifestN._rev}`).reply(201) await npm.exec('unpublish', []) @@ -286,11 +281,14 @@ t.test('workspaces', async t => { }, prefixDir, }) - const manifestA = pkgManifest('workspace-a') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/workspace-a?write=true').times(2).reply(200, manifestA) - .delete(`/workspace-a/-rev/${manifestA._rev}`).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifestA = registry.manifest({ name: 'workspace-a' }) + await registry.package({ manifest: manifestA, query: { write: true }, times: 2 }) + registry.nock.delete(`/workspace-a/-rev/${manifestA._rev}`).reply(201) await npm.exec('unpublish', []) t.equal(joinedOutput(), '- workspace-a') @@ -304,11 +302,16 @@ t.test('dryRun with spec', async t => { ...auth, }, }) - - const manifest = pkgManifest(pkg, '1.0.1') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ + name: pkg, + packuments: ['1.0.0', '1.0.1'], + }) + await registry.package({ manifest, query: { write: true } }) await npm.exec('unpublish', ['test-package@1.0.1']) t.equal(joinedOutput(), '- test-package@1.0.1') @@ -338,7 +341,6 @@ t.test('publishConfig no spec', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { force: true, - 'fetch-retries': 0, '//other.registry.npmjs.org/:_authToken': 'test-other-token', }, prefixDir: { @@ -352,11 +354,14 @@ t.test('publishConfig no spec', async t => { }, }) - const manifest = pkgManifest(pkg) - tnock(t, alternateRegistry, - { reqheaders: { authorization: 'Bearer test-other-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) - .delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true } }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', []) t.equal(joinedOutput(), '- test-package@1.0.0') }) @@ -366,7 +371,6 @@ t.test('publishConfig with spec', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { force: true, - 'fetch-retries': 0, '//other.registry.npmjs.org/:_authToken': 'test-other-token', }, prefixDir: { @@ -380,11 +384,14 @@ t.test('publishConfig with spec', async t => { }, }) - const manifest = pkgManifest(pkg) - tnock(t, alternateRegistry, - { reqheaders: { authorization: 'Bearer test-other-token' } }) - .get(`/${pkg}?write=true`).reply(200, manifest) - .delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true }, times: 2 }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(201) await npm.exec('unpublish', ['test-package']) t.equal(joinedOutput(), '- test-package') }) @@ -406,12 +413,18 @@ t.test('completion', async t => { } t.test('completing with multiple versions from the registry', async t => { - const manifest = pkgManifest(pkg, '1.0.1') - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ + name: pkg, + packuments: ['1.0.0', '1.0.1'], + }) + await registry.package({ manifest, query: { write: true } }) + registry.nock.get('/-/whoami').reply(200, { username: user }) .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) - .get(`/${pkg}?write=true`).reply(200, manifest) await testComp(t, { argv: ['npm', 'unpublish'], @@ -424,13 +437,16 @@ t.test('completion', async t => { }) t.test('no versions retrieved', async t => { - const manifest = pkgManifest(pkg) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) manifest.versions = {} - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) + await registry.package({ manifest, query: { write: true } }) + registry.nock.get('/-/whoami').reply(200, { username: user }) .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) - .get(`/${pkg}?write=true`).reply(200, manifest) await testComp(t, { argv: ['npm', 'unpublish'], @@ -443,9 +459,12 @@ t.test('completion', async t => { }) t.test('packages starting with same letters', async t => { - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.nock.get('/-/whoami').reply(200, { username: user }) .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write', [`${pkg}a`]: 'write', @@ -464,10 +483,13 @@ t.test('completion', async t => { }) t.test('no packages retrieved', async t => { - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, {}) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.nock.get('/-/whoami').reply(200, { username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {}) await testComp(t, { argv: ['npm', 'unpublish'], @@ -478,9 +500,12 @@ t.test('completion', async t => { }) t.test('no pkg name to complete', async t => { - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.nock.get('/-/whoami').reply(200, { username: user }) .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write', [`${pkg}a`]: 'write', @@ -495,9 +520,12 @@ t.test('completion', async t => { }) t.test('no pkg names retrieved from user account', async t => { - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(200, { username: user }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.nock.get('/-/whoami').reply(200, { username: user }) .get('/-/org/test-user/package?format=cli').reply(200, null) await testComp(t, { @@ -509,9 +537,12 @@ t.test('completion', async t => { }) t.test('logged out user', async t => { - tnock(t, npm.config.get('registry'), - { reqheaders: { authorization: 'Bearer test-auth-token' } }) - .get('/-/whoami').reply(404) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.nock.get('/-/whoami').reply(404) await testComp(t, { argv: ['npm', 'unpublish'], diff --git a/test/lib/npm.js b/test/lib/npm.js index 998e96314d2b4..1966ca9600088 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -609,16 +609,14 @@ t.test('implicit workspace rejection', async t => { workspaces: ['./packages/a'], }), }, - globals: { + globals: ({ prefix }) => ({ + 'process.cwd': () => join(prefix, 'packages', 'a'), 'process.argv': [ process.execPath, process.argv[1], '--color', 'false', '--workspace', './packages/a', ], - }, - config: ({ prefix }) => ({ - workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' }, }), }) await t.rejects( @@ -646,16 +644,13 @@ t.test('implicit workspace accept', async t => { }), }, globals: ({ prefix }) => ({ - 'process.cwd': () => prefix, + 'process.cwd': () => join(prefix, 'packages', 'a'), 'process.argv': [ process.execPath, process.argv[1], '--color', 'false', ], }), - config: ({ prefix }) => ({ - workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' }, - }), }) await t.rejects(mock.npm.exec('org', []), /.*Usage/) }) From d78cccc078d046b5d2280927e76ff5755454cdf5 Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 31 Mar 2022 09:42:24 -0700 Subject: [PATCH 3/6] fix: do not export npm_config_include_workspace_root --- docs/content/commands/npm-audit.md | 2 ++ docs/content/commands/npm-dedupe.md | 2 ++ docs/content/commands/npm-diff.md | 2 ++ docs/content/commands/npm-dist-tag.md | 2 ++ docs/content/commands/npm-docs.md | 2 ++ docs/content/commands/npm-exec.md | 2 ++ docs/content/commands/npm-find-dupes.md | 2 ++ docs/content/commands/npm-init.md | 2 ++ docs/content/commands/npm-install-test.md | 5 +++-- docs/content/commands/npm-install.md | 5 +++-- docs/content/commands/npm-link.md | 5 +++-- docs/content/commands/npm-ls.md | 2 ++ docs/content/commands/npm-pack.md | 2 ++ docs/content/commands/npm-prune.md | 2 ++ docs/content/commands/npm-publish.md | 2 ++ docs/content/commands/npm-rebuild.md | 2 ++ docs/content/commands/npm-repo.md | 2 ++ docs/content/commands/npm-run-script.md | 2 ++ docs/content/commands/npm-set-script.md | 2 ++ docs/content/commands/npm-uninstall.md | 5 +++-- docs/content/commands/npm-update.md | 5 +++-- docs/content/commands/npm-version.md | 2 ++ docs/content/commands/npm-view.md | 2 ++ docs/content/using-npm/config.md | 5 +++-- lib/utils/config/definitions.js | 5 +++-- tap-snapshots/test/lib/utils/config/definitions.js.test.cjs | 5 +++-- tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs | 5 +++-- 27 files changed, 63 insertions(+), 18 deletions(-) diff --git a/docs/content/commands/npm-audit.md b/docs/content/commands/npm-audit.md index 24b700ff5abd8..0f164ac9d3ec5 100644 --- a/docs/content/commands/npm-audit.md +++ b/docs/content/commands/npm-audit.md @@ -394,6 +394,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-dedupe.md b/docs/content/commands/npm-dedupe.md index 91f0105e35f21..b9768c25db88d 100644 --- a/docs/content/commands/npm-dedupe.md +++ b/docs/content/commands/npm-dedupe.md @@ -305,6 +305,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-diff.md b/docs/content/commands/npm-diff.md index c4c9eafdb3524..7dcc8af7c3b4c 100644 --- a/docs/content/commands/npm-diff.md +++ b/docs/content/commands/npm-diff.md @@ -335,6 +335,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-dist-tag.md b/docs/content/commands/npm-dist-tag.md index a0f306cd4970d..b9caf1fbe7fa9 100644 --- a/docs/content/commands/npm-dist-tag.md +++ b/docs/content/commands/npm-dist-tag.md @@ -159,6 +159,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-docs.md b/docs/content/commands/npm-docs.md index 8d5a278286a88..790d563bdb1fb 100644 --- a/docs/content/commands/npm-docs.md +++ b/docs/content/commands/npm-docs.md @@ -116,6 +116,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-exec.md b/docs/content/commands/npm-exec.md index 3645e336b9da9..8ccfa75c73386 100644 --- a/docs/content/commands/npm-exec.md +++ b/docs/content/commands/npm-exec.md @@ -205,6 +205,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-find-dupes.md b/docs/content/commands/npm-find-dupes.md index f7366fa6375d1..3549be47daae9 100644 --- a/docs/content/commands/npm-find-dupes.md +++ b/docs/content/commands/npm-find-dupes.md @@ -229,6 +229,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-init.md b/docs/content/commands/npm-init.md index 71109cd360511..5a7dd4c97ac60 100644 --- a/docs/content/commands/npm-init.md +++ b/docs/content/commands/npm-init.md @@ -264,6 +264,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-install-test.md b/docs/content/commands/npm-install-test.md index 5ac31cbf08e25..8975fc4ce61de 100644 --- a/docs/content/commands/npm-install-test.md +++ b/docs/content/commands/npm-install-test.md @@ -42,8 +42,7 @@ takes exactly the same arguments as `npm install`. #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. @@ -315,6 +314,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-install.md b/docs/content/commands/npm-install.md index 328d6fc14d9db..7e370319b82ff 100644 --- a/docs/content/commands/npm-install.md +++ b/docs/content/commands/npm-install.md @@ -432,8 +432,7 @@ These are some of the most common options related to installation. #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. @@ -705,6 +704,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-link.md b/docs/content/commands/npm-link.md index 892b55496c9b6..fb7e46de04a09 100644 --- a/docs/content/commands/npm-link.md +++ b/docs/content/commands/npm-link.md @@ -125,8 +125,7 @@ workspace(s). #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. @@ -383,6 +382,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-ls.md b/docs/content/commands/npm-ls.md index 0f06e131f414b..8d4799777e20f 100644 --- a/docs/content/commands/npm-ls.md +++ b/docs/content/commands/npm-ls.md @@ -280,6 +280,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-pack.md b/docs/content/commands/npm-pack.md index c834f643ac0bb..fa6c005e9d228 100644 --- a/docs/content/commands/npm-pack.md +++ b/docs/content/commands/npm-pack.md @@ -122,6 +122,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-prune.md b/docs/content/commands/npm-prune.md index a10a353801b7c..81dccf889ce4d 100644 --- a/docs/content/commands/npm-prune.md +++ b/docs/content/commands/npm-prune.md @@ -186,6 +186,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-publish.md b/docs/content/commands/npm-publish.md index ce6e1c1012c8e..2995f6bc81551 100644 --- a/docs/content/commands/npm-publish.md +++ b/docs/content/commands/npm-publish.md @@ -238,6 +238,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-rebuild.md b/docs/content/commands/npm-rebuild.md index d63e00b79d386..bddd18c2bcaf4 100644 --- a/docs/content/commands/npm-rebuild.md +++ b/docs/content/commands/npm-rebuild.md @@ -157,6 +157,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-repo.md b/docs/content/commands/npm-repo.md index e14f07012a248..404c724ff0290 100644 --- a/docs/content/commands/npm-repo.md +++ b/docs/content/commands/npm-repo.md @@ -103,6 +103,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-run-script.md b/docs/content/commands/npm-run-script.md index 79b7c9a25780e..aab8d769dc9e2 100644 --- a/docs/content/commands/npm-run-script.md +++ b/docs/content/commands/npm-run-script.md @@ -203,6 +203,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-set-script.md b/docs/content/commands/npm-set-script.md index 2d8e87db85219..0fc267f760c81 100644 --- a/docs/content/commands/npm-set-script.md +++ b/docs/content/commands/npm-set-script.md @@ -97,6 +97,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-uninstall.md b/docs/content/commands/npm-uninstall.md index 9316c686d4095..2f6618bde7995 100644 --- a/docs/content/commands/npm-uninstall.md +++ b/docs/content/commands/npm-uninstall.md @@ -70,8 +70,7 @@ npm uninstall lodash --no-save #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. @@ -141,6 +140,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-update.md b/docs/content/commands/npm-update.md index aff46b1e5b557..be0b0cb937eae 100644 --- a/docs/content/commands/npm-update.md +++ b/docs/content/commands/npm-update.md @@ -171,8 +171,7 @@ be _downgraded_. #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. @@ -433,6 +432,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-version.md b/docs/content/commands/npm-version.md index 713e5ae410dfc..8e3334d788984 100644 --- a/docs/content/commands/npm-version.md +++ b/docs/content/commands/npm-version.md @@ -166,6 +166,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/commands/npm-view.md b/docs/content/commands/npm-view.md index b50b4bfb56c5d..0ef17d8adfb39 100644 --- a/docs/content/commands/npm-view.md +++ b/docs/content/commands/npm-view.md @@ -180,6 +180,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index 39870922c0253..858fe4d638bed 100644 --- a/docs/content/using-npm/config.md +++ b/docs/content/using-npm/config.md @@ -819,6 +819,8 @@ When false, specifying individual workspaces via the `workspace` config, or all workspaces via the `workspaces` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + @@ -1343,8 +1345,7 @@ The base URL of the npm registry. #### `save` -* Default: `true` unless when using `npm update` or `npm dedupe` where it - defaults to `false` +* Default: `true` unless when using `npm update` where it defaults to `false` * Type: Boolean Save installed packages to a `package.json` file as dependencies. diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index efc1f72a02059..316e737091eb4 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -938,6 +938,7 @@ define('include-staged', { define('include-workspace-root', { default: false, type: Boolean, + envExport: false, description: ` Include the workspace root when workspaces are enabled for a command. @@ -1605,8 +1606,8 @@ define('registry', { define('save', { default: true, - defaultDescription: `\`true\` unless when using \`npm update\` or - \`npm dedupe\` where it defaults to \`false\``, + defaultDescription: `\`true\` unless when using \`npm update\` where it + defaults to \`false\``, usage: '-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer|--save-bundle', type: Boolean, short: 'S', diff --git a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs index d7c43080298eb..b18b8e7a829e6 100644 --- a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs @@ -848,6 +848,8 @@ Include the workspace root when workspaces are enabled for a command. When false, specifying individual workspaces via the \`workspace\` config, or all workspaces via the \`workspaces\` flag, will cause npm to operate only on the specified workspaces, and not on the root project. + +This value is not exported to the environment for child processes. ` exports[`test/lib/utils/config/definitions.js TAP > config description for init-author-email 1`] = ` @@ -1424,8 +1426,7 @@ The base URL of the npm registry. exports[`test/lib/utils/config/definitions.js TAP > config description for save 1`] = ` #### \`save\` -* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it - defaults to \`false\` +* Default: \`true\` unless when using \`npm update\` where it defaults to \`false\` * Type: Boolean Save installed packages to a \`package.json\` file as dependencies. diff --git a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs index 2647bc31bcdb8..564ade46e731d 100644 --- a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs @@ -693,6 +693,8 @@ When false, specifying individual workspaces via the \`workspace\` config, or all workspaces via the \`workspaces\` flag, will cause npm to operate only on the specified workspaces, and not on the root project. +This value is not exported to the environment for child processes. + @@ -1217,8 +1219,7 @@ The base URL of the npm registry. #### \`save\` -* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it - defaults to \`false\` +* Default: \`true\` unless when using \`npm update\` where it defaults to \`false\` * Type: Boolean Save installed packages to a \`package.json\` file as dependencies. From 9f03e877252495079b3fc7aada5b7fd19048b955 Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 1 Apr 2022 09:17:01 -0700 Subject: [PATCH 4/6] chore: remove get-idendity mock from whoami tests --- test/fixtures/mock-registry.js | 10 ++-- test/lib/commands/dedupe.js | 61 ++++++++++++------------ test/lib/commands/find-dupes.js | 82 +++++++++++++++++++++++++++------ test/lib/commands/whoami.js | 29 ++++++++---- 4 files changed, 127 insertions(+), 55 deletions(-) diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js index ec4ebc2a4c24f..34969b65fc6c0 100644 --- a/test/fixtures/mock-registry.js +++ b/test/fixtures/mock-registry.js @@ -23,6 +23,9 @@ class MockRegistry { get nock () { if (!this.#nock) { + if (!this.#tap) { + throw new Error('cannot mock packages without a tap fixture') + } const tnock = require('./tnock.js') const reqheaders = {} if (this.#authorization) { @@ -37,11 +40,12 @@ class MockRegistry { this.#nock = nock } + async whoami ({ username }) { + this.nock.get('/-/whoami').reply(200, { username }) + } + async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock - if (!nock) { - throw new Error('cannot mock packages without a tap fixture') - } nock = nock.get(`/${manifest.name}`).times(times) if (query) { nock = nock.query(query) diff --git a/test/lib/commands/dedupe.js b/test/lib/commands/dedupe.js index a985f4a710753..0ca51245cc818 100644 --- a/test/lib/commands/dedupe.js +++ b/test/lib/commands/dedupe.js @@ -1,8 +1,8 @@ const t = require('tap') -const { load: loadMockNpm } = require('../../fixtures/mock-npm') const path = require('path') const fs = require('fs') +const { load: loadMockNpm } = require('../../fixtures/mock-npm') const MockRegistry = require('../../fixtures/mock-registry.js') t.test('should throw in global mode', async (t) => { @@ -18,45 +18,43 @@ t.test('should throw in global mode', async (t) => { ) }) -const testTop = { - name: 'test-top', - version: '1.0.0', - dependencies: { - 'test-dep-a': '*', - 'test-dep-b': '*', - }, -} -const testDepA = { - name: 'test-dep-a', - version: '1.0.1', - dependencies: { 'test-sub': '*' }, -} -const testDepB = { - name: 'test-dep-b', - version: '1.0.0', - dependencies: { 'test-sub': '*' }, -} -const testSub = { - name: 'test-sub', - version: '1.0.0', -} - const treeWithDupes = { - 'package.json': JSON.stringify(testTop), + 'package.json': JSON.stringify({ + name: 'test-top', + version: '1.0.0', + dependencies: { + 'test-dep-a': '*', + 'test-dep-b': '*', + }, + }), node_modules: { 'test-dep-a': { - 'package.json': JSON.stringify(testDepA), + 'package.json': JSON.stringify({ + name: 'test-dep-a', + version: '1.0.1', + dependencies: { 'test-sub': '*' }, + }), node_modules: { 'test-sub': { - 'package.json': JSON.stringify(testSub), + 'package.json': JSON.stringify({ + name: 'test-sub', + version: '1.0.0', + }), }, }, }, 'test-dep-b': { - 'package.json': JSON.stringify(testDepB), + 'package.json': JSON.stringify({ + name: 'test-dep-b', + version: '1.0.0', + dependencies: { 'test-sub': '*' }, + }), node_modules: { 'test-sub': { - 'package.json': JSON.stringify(testSub), + 'package.json': JSON.stringify({ + name: 'test-sub', + version: '1.0.0', + }), }, }, }, @@ -84,7 +82,10 @@ t.test('dedupe', async (t) => { }) await npm.exec('dedupe', []) t.match(joinedOutput(), /added 1 package, and removed 2 packages/) - t.ok(fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-sub')), 'test-sub was hoisted') + t.ok( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-sub')), + 'test-sub was hoisted' + ) t.notOk( fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-a', 'node_modules', 'test-sub')), 'test-dep-a/test-sub was removed' diff --git a/test/lib/commands/find-dupes.js b/test/lib/commands/find-dupes.js index 06bd097b6ca59..228ac662b4454 100644 --- a/test/lib/commands/find-dupes.js +++ b/test/lib/commands/find-dupes.js @@ -1,28 +1,84 @@ const t = require('tap') +const path = require('path') +const fs = require('fs') const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const MockRegistry = require('../../fixtures/mock-registry.js') -t.test('should run dedupe in dryRun mode', async (t) => { - t.plan(5) - const { npm } = await loadMockNpm(t, { - mocks: { - '@npmcli/arborist': function (args) { - t.ok(args, 'gets options object') - t.ok(args.path, 'gets path option') - t.ok(args.dryRun, 'is called in dryRun mode') - this.dedupe = () => { - t.ok(true, 'dedupe is called') - } +const treeWithDupes = { + 'package.json': JSON.stringify({ + name: 'test-top', + version: '1.0.0', + dependencies: { + 'test-dep-a': '*', + 'test-dep-b': '*', + }, + }), + node_modules: { + 'test-dep-a': { + 'package.json': JSON.stringify({ + name: 'test-dep-a', + version: '1.0.1', + dependencies: { 'test-sub': '*' }, + }), + node_modules: { + 'test-sub': { + 'package.json': JSON.stringify({ + name: 'test-sub', + version: '1.0.0', + }), + }, }, - '../../lib/utils/reify-finish.js': (npm, arb) => { - t.ok(arb, 'gets arborist tree') + }, + 'test-dep-b': { + 'package.json': JSON.stringify({ + name: 'test-dep-b', + version: '1.0.0', + dependencies: { 'test-sub': '*' }, + }), + node_modules: { + 'test-sub': { + 'package.json': JSON.stringify({ + name: 'test-sub', + version: '1.0.0', + }), + }, }, }, + }, +} + +t.test('should run dedupe in dryRun mode', async (t) => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: treeWithDupes, config: { // explicitly set to false so we can be 100% sure it's always true when it // hits arborist 'dry-run': false, }, }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + + const manifestSub = registry.manifest({ + name: 'test-sub', + manifests: [{ name: 'test-sub', version: '1.0.0' }], + }) + + await registry.package({ manifest: manifestSub }) await npm.exec('find-dupes', []) + t.match(joinedOutput(), /added 1 package, and removed 2 packages/) + t.notOk( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-sub')), + 'test-sub was not hoisted' + ) + t.ok( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-a', 'node_modules', 'test-sub')), + 'test-dep-a/test-sub was not removed' + ) + t.ok( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-b', 'node_modules', 'test-sub')), + 'test-dep-b/test-sub was not removed') }) diff --git a/test/lib/commands/whoami.js b/test/lib/commands/whoami.js index 66c3f0c6b30bf..f483bd46d5c4b 100644 --- a/test/lib/commands/whoami.js +++ b/test/lib/commands/whoami.js @@ -1,24 +1,35 @@ const t = require('tap') -const { load: _loadMockNpm } = require('../../fixtures/mock-npm') +const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const MockRegistry = require('../../fixtures/mock-registry.js') const username = 'foo' -const loadMockNpm = (t, options) => _loadMockNpm(t, { - mocks: { - '../../lib/utils/get-identity.js': () => Promise.resolve(username), - }, - ...options, -}) +const auth = { '//registry.npmjs.org/:_authToken': 'test-auth-token' } t.test('npm whoami', async (t) => { - const { npm, joinedOutput } = await loadMockNpm(t) + const { npm, joinedOutput } = await loadMockNpm(t, { config: auth }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + registry.whoami({ username }) await npm.exec('whoami', []) t.equal(joinedOutput(), username, 'should print username') }) t.test('npm whoami --json', async (t) => { const { npm, joinedOutput } = await loadMockNpm(t, { - config: { json: true }, + config: { + json: true, + ...auth, + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', }) + registry.whoami({ username }) await npm.exec('whoami', []) t.equal(JSON.parse(joinedOutput()), username, 'should print username') }) From 98205912a384b97d79570479353322e328f08ade Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 1 Apr 2022 16:45:01 -0700 Subject: [PATCH 5/6] chore: remove mocks from npm audit tests --- .../test/lib/commands/audit.js.test.cjs | 76 ++++++ test/fixtures/mock-registry.js | 21 +- test/lib/commands/audit.js | 229 +++++++++--------- 3 files changed, 215 insertions(+), 111 deletions(-) create mode 100644 tap-snapshots/test/lib/commands/audit.js.test.cjs diff --git a/tap-snapshots/test/lib/commands/audit.js.test.cjs b/tap-snapshots/test/lib/commands/audit.js.test.cjs new file mode 100644 index 0000000000000..1f8e2e6c7a361 --- /dev/null +++ b/tap-snapshots/test/lib/commands/audit.js.test.cjs @@ -0,0 +1,76 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'use strict' +exports[`test/lib/commands/audit.js TAP audit fix > must match snapshot 1`] = ` + +added 1 package, and audited 2 packages in 120ms + +found 0 vulnerabilities +` + +exports[`test/lib/commands/audit.js TAP json audit > must match snapshot 1`] = ` +{ + "auditReportVersion": 2, + "vulnerabilities": { + "test-dep-a": { + "name": "test-dep-a", + "severity": "high", + "isDirect": true, + "via": [ + { + "source": 100, + "name": "test-dep-a", + "dependency": "test-dep-a", + "title": "Test advisory 100", + "url": "https://github.com/advisories/GHSA-100", + "severity": "high", + "range": "*" + } + ], + "effects": [], + "range": "*", + "nodes": [ + "node_modules/test-dep-a" + ], + "fixAvailable": false + } + }, + "metadata": { + "vulnerabilities": { + "info": 0, + "low": 0, + "moderate": 0, + "high": 1, + "critical": 0, + "total": 1 + }, + "dependencies": { + "prod": 2, + "dev": 0, + "optional": 0, + "peer": 0, + "peerOptional": 0, + "total": 1 + } + } +} +` + +exports[`test/lib/commands/audit.js TAP normal audit > must match snapshot 1`] = ` +# npm audit report + +test-dep-a * +Severity: high +Test advisory 100 - https://github.com/advisories/GHSA-100 +No fix available +node_modules/test-dep-a + +1 high severity vulnerability + +Some issues need review, and may require choosing +a different dependency. +` diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js index 34969b65fc6c0..1cbe7b52dbe4e 100644 --- a/test/fixtures/mock-registry.js +++ b/test/fixtures/mock-registry.js @@ -44,6 +44,24 @@ class MockRegistry { this.nock.get('/-/whoami').reply(200, { username }) } + advisory (advisory = {}) { + const id = advisory.id || parseInt(Math.random() * 1000000) + return { + id, + url: `https://github.com/advisories/GHSA-${id}`, + title: `Test advisory ${id}`, + severity: 'high', + vulnerable_versions: '*', + cwe: [ + 'cwe-0', + ], + cvss: { + score: 0, + }, + ...advisory, + } + } + async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock nock = nock.get(`/${manifest.name}`).times(times) @@ -52,7 +70,8 @@ class MockRegistry { } nock = nock.reply(200, manifest) if (tarballs) { - for (const version in manifest.versions) { + for (const version in tarballs) { + // for (const version in manifest.versions) { const packument = manifest.versions[version] const dist = new URL(packument.dist.tarball) const tarball = await pacote.tarball(tarballs[version]) diff --git a/test/lib/commands/audit.js b/test/lib/commands/audit.js index 05f268d6bcd0e..8b79b02ba6900 100644 --- a/test/lib/commands/audit.js +++ b/test/lib/commands/audit.js @@ -1,139 +1,148 @@ const t = require('tap') -const { load: _loadMockNpm } = require('../../fixtures/mock-npm') -t.test('should audit using Arborist', async t => { - let ARB_ARGS = null - let AUDIT_CALLED = false - let REIFY_FINISH_CALLED = false - let AUDIT_REPORT_CALLED = false - let ARB_OBJ = null +const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const MockRegistry = require('../../fixtures/mock-registry.js') +const util = require('util') +const zlib = require('zlib') +const gzip = util.promisify(zlib.gzip) +const path = require('path') - const loadMockNpm = (t) => _loadMockNpm(t, { - mocks: { - 'npm-audit-report': () => { - AUDIT_REPORT_CALLED = true - return { - report: 'there are vulnerabilities', - exitCode: 0, - } +const tree = { + 'package.json': JSON.stringify({ + name: 'test-dep', + version: '1.0.0', + dependencies: { + 'test-dep-a': '*', + }, + }), + 'package-lock.json': JSON.stringify({ + name: 'test-dep', + version: '1.0.0', + lockfileVersion: 2, + requires: true, + packages: { + '': { + xname: 'scratch', + version: '1.0.0', + dependencies: { + 'test-dep-a': '*', + }, + devDependencies: {}, }, - '@npmcli/arborist': function (args) { - ARB_ARGS = args - ARB_OBJ = this - this.audit = () => { - AUDIT_CALLED = true - this.auditReport = {} - } + 'node_modules/test-dep-a': { + name: 'test-dep-a', + version: '1.0.0', }, - '../../lib/utils/reify-finish.js': (npm, arb) => { - if (arb !== ARB_OBJ) { - throw new Error('got wrong object passed to reify-output') - } - - REIFY_FINISH_CALLED = true + }, + dependencies: { + 'test-dep-a': { + version: '1.0.0', }, }, - }) + }), + 'test-dep-a': { + 'package.json': JSON.stringify({ + name: 'test-dep-a', + version: '1.0.1', + }), + 'fixed.txt': 'fixed test-dep-a', + }, +} - t.test('audit', async t => { - const { npm, outputs } = await loadMockNpm(t) - await npm.exec('audit', []) - t.match(ARB_ARGS, { audit: true, path: npm.prefix }) - t.equal(AUDIT_CALLED, true, 'called audit') - t.equal(AUDIT_REPORT_CALLED, true, 'called audit report') - t.match(outputs, [['there are vulnerabilities']]) +t.test('normal audit', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: tree, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) - t.test('audit fix', async t => { - const { npm } = await loadMockNpm(t) - await npm.exec('audit', ['fix']) - t.equal(REIFY_FINISH_CALLED, true, 'called reify output') + const manifest = registry.manifest({ + name: 'test-dep-a', + packuments: [{ version: '1.0.0' }, { version: '1.0.1' }], }) + await registry.package({ manifest }) + const advisory = registry.advisory({ id: 100 }) + const bulkBody = await gzip(JSON.stringify({ 'test-dep-a': ['1.0.0'] })) + registry.nock.post('/-/npm/v1/security/advisories/bulk', bulkBody) + .reply(200, { + 'test-dep-a': [advisory], + }) + + await npm.exec('audit', []) + t.ok(process.exitCode, 'would have exited uncleanly') + process.exitCode = 0 + t.matchSnapshot(joinedOutput()) }) -t.test('should audit - json', async t => { - t.plan(1) - const { npm } = await _loadMockNpm(t, { - mocks: { - 'npm-audit-report': (_, opts) => { - t.match(opts.reporter, 'json') - return { - report: 'there are vulnerabilities', - exitCode: 0, - } - }, - '@npmcli/arborist': function () { - this.audit = () => { - this.auditReport = {} - } - }, - '../../lib/utils/reify-output.js': () => {}, - }, +t.test('json audit', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: tree, config: { json: true, }, }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + + const manifest = registry.manifest({ + name: 'test-dep-a', + packuments: [{ version: '1.0.0' }, { version: '1.0.1' }], + }) + await registry.package({ manifest }) + const advisory = registry.advisory({ id: 100 }) + const bulkBody = await gzip(JSON.stringify({ 'test-dep-a': ['1.0.0'] })) + registry.nock.post('/-/npm/v1/security/advisories/bulk', bulkBody) + .reply(200, { + 'test-dep-a': [advisory], + }) + await npm.exec('audit', []) + t.ok(process.exitCode, 'would have exited uncleanly') + process.exitCode = 0 + t.matchSnapshot(joinedOutput()) }) -t.test('report endpoint error', async t => { - const loadMockNpm = (t, options) => _loadMockNpm(t, { - mocks: { - 'npm-audit-report': () => { - throw new Error('should not call audit report when there are errors') - }, - '@npmcli/arborist': function () { - this.audit = () => { - this.auditReport = { - error: { - message: 'hello, this didnt work', - method: 'POST', - uri: 'https://example.com/', - headers: { - head: ['ers'], - }, - statusCode: 420, - body: 'this is a string', - }, - } - } - }, - '../../lib/utils/reify-output.js': () => {}, - }, - ...options, +t.test('audit fix', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: tree, }) - - t.test('json=false', async t => { - const { npm, outputs, logs } = await loadMockNpm(t, { config: { json: false } }) - await t.rejects(npm.exec('audit', []), 'audit endpoint returned an error') - t.match(logs.warn, [['audit', 'hello, this didnt work']]) - t.strictSame(outputs, [['this is a string']]) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) - - t.test('json=true', async t => { - const { npm, outputs, logs } = await loadMockNpm(t, { config: { json: true } }) - await t.rejects(npm.exec('audit', []), 'audit endpoint returned an error') - t.match(logs.warn, [['audit', 'hello, this didnt work']]) - t.strictSame(outputs, [[ - '{\n' + - ' "message": "hello, this didnt work",\n' + - ' "method": "POST",\n' + - ' "uri": "https://example.com/",\n' + - ' "headers": {\n' + - ' "head": [\n' + - ' "ers"\n' + - ' ]\n' + - ' },\n' + - ' "statusCode": 420,\n' + - ' "body": "this is a string"\n' + - '}', - ], - ]) + // with fix + const manifest = registry.manifest({ + name: 'test-dep-a', + packuments: [{ version: '1.0.0' }, { version: '1.0.1' }], + }) + await registry.package({ + manifest, + tarballs: { + '1.0.1': path.join(npm.prefix, 'test-dep-a'), + }, }) + const advisory = registry.advisory({ id: 100, vulnerable_versions: '1.0.0' }) + // Can't validate this request body because it changes with each node + // version/npm version and nock's body validation is not async, while + // zlib.gunzip is + registry.nock.post('/-/npm/v1/security/advisories/bulk') + .reply(200, { // first audit + 'test-dep-a': [advisory], + }) + .post('/-/npm/v1/security/advisories/bulk') + .reply(200, { // after fix + 'test-dep-a': [advisory], + }) + await npm.exec('audit', ['fix']) + t.matchSnapshot(joinedOutput()) }) t.test('completion', async t => { - const { npm } = await _loadMockNpm(t) + const { npm } = await loadMockNpm(t) const audit = await npm.cmd('audit') t.test('fix', async t => { await t.resolveMatch( From 55db9dd79faaaece2d26248c8dc8a89b8034eaa8 Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 1 Apr 2022 17:48:22 -0700 Subject: [PATCH 6/6] chore: remove mocks from completion tests --- .../test/lib/commands/audit.js.test.cjs | 31 ++++++++++++++++++- test/lib/commands/audit.js | 10 +++++- test/lib/commands/completion.js | 19 ++++++------ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/tap-snapshots/test/lib/commands/audit.js.test.cjs b/tap-snapshots/test/lib/commands/audit.js.test.cjs index 1f8e2e6c7a361..f8cee61123a6e 100644 --- a/tap-snapshots/test/lib/commands/audit.js.test.cjs +++ b/tap-snapshots/test/lib/commands/audit.js.test.cjs @@ -5,9 +5,38 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' +exports[`test/lib/commands/audit.js TAP audit fix > lockfile has test-dep-a@1.0.1 1`] = ` +{ + "name": "test-dep", + "version": "1.0.0", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "name": "test-dep", + "version": "1.0.0", + "dependencies": { + "test-dep-a": "*" + } + }, + "node_modules/test-dep-a": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/test-dep-a/-/test-dep-a-1.0.1.tgz" + } + }, + "dependencies": { + "test-dep-a": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/test-dep-a/-/test-dep-a-1.0.1.tgz" + } + } +} + +` + exports[`test/lib/commands/audit.js TAP audit fix > must match snapshot 1`] = ` -added 1 package, and audited 2 packages in 120ms +added 1 package, and audited 2 packages in xxx found 0 vulnerabilities ` diff --git a/test/lib/commands/audit.js b/test/lib/commands/audit.js index 8b79b02ba6900..1afb8d333b7ce 100644 --- a/test/lib/commands/audit.js +++ b/test/lib/commands/audit.js @@ -6,6 +6,9 @@ const util = require('util') const zlib = require('zlib') const gzip = util.promisify(zlib.gzip) const path = require('path') +const fs = require('fs') + +t.cleanSnapshot = str => str.replace(/packages in [0-9]+[a-z]+/g, 'packages in xxx') const tree = { 'package.json': JSON.stringify({ @@ -114,7 +117,6 @@ t.test('audit fix', async t => { tap: t, registry: npm.config.get('registry'), }) - // with fix const manifest = registry.manifest({ name: 'test-dep-a', packuments: [{ version: '1.0.0' }, { version: '1.0.1' }], @@ -139,6 +141,12 @@ t.test('audit fix', async t => { }) await npm.exec('audit', ['fix']) t.matchSnapshot(joinedOutput()) + const pkg = fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8') + t.matchSnapshot(pkg, 'lockfile has test-dep-a@1.0.1') + t.ok( + fs.existsSync(path.join(npm.prefix, 'node_modules', 'test-dep-a', 'fixed.txt')), + 'has test-dep-a@1.0.1 on disk' + ) }) t.test('completion', async t => { diff --git a/test/lib/commands/completion.js b/test/lib/commands/completion.js index d4e6f1199c457..6cc1677552e8a 100644 --- a/test/lib/commands/completion.js +++ b/test/lib/commands/completion.js @@ -6,20 +6,19 @@ const completionScript = fs .readFileSync(path.resolve(__dirname, '../../../lib/utils/completion.sh'), { encoding: 'utf8' }) .replace(/^#!.*?\n/, '') -const { load: _loadMockNpm } = require('../../fixtures/mock-npm') +const { load: loadMockNpm } = require('../../fixtures/mock-npm') const mockGlobals = require('../../fixtures/mock-globals') const loadMockCompletion = async (t, o = {}) => { - const { globals, windows, ...options } = o + const { globals = {}, windows, ...options } = o let resetGlobals = {} - if (globals) { - resetGlobals = mockGlobals(t, globals).reset - } - const res = await _loadMockNpm(t, { - mocks: { - '../../lib/utils/is-windows.js': { isWindowsShell: !!windows }, - ...options.mocks, - }, + resetGlobals = mockGlobals(t, { + 'process.platform': windows ? 'win32' : 'posix', + 'process.env.term': 'notcygwin', + 'process.env.msystem': 'nogmingw', + ...globals, + }).reset + const res = await loadMockNpm(t, { ...options, }) const completion = await res.npm.cmd('completion')