From b8d608929f395d2d2fca4cb05905c5ef712c0faa Mon Sep 17 00:00:00 2001 From: Gar Date: Mon, 8 Nov 2021 10:38:58 -0800 Subject: [PATCH] chore: refactor pack tests Uses the real npm object and doesn't mock anything. PR-URL: https://github.com/npm/cli/pull/4018 Credit: @wraithgar Close: #4018 Reviewed-by: @lukekarrys --- .../test/lib/commands/pack.js.test.cjs | 78 +++ test/fixtures/mock-npm.js | 1 + test/lib/commands/pack.js | 465 +++++------------- test/lib/commands/shrinkwrap.js | 6 +- 4 files changed, 197 insertions(+), 353 deletions(-) create mode 100644 tap-snapshots/test/lib/commands/pack.js.test.cjs diff --git a/tap-snapshots/test/lib/commands/pack.js.test.cjs b/tap-snapshots/test/lib/commands/pack.js.test.cjs new file mode 100644 index 0000000000000..e3219b45ed54f --- /dev/null +++ b/tap-snapshots/test/lib/commands/pack.js.test.cjs @@ -0,0 +1,78 @@ +/* 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/pack.js TAP dry run > logs pack contents 1`] = ` +Array [ + undefined, + "package: test-package@1.0.0", + undefined, + "41B package.json", + undefined, + String( + name: test-package + version: 1.0.0 + filename: test-package-1.0.0.tgz + package size: 136 B + unpacked size: 41 B + shasum: a92a0679a70a450f14f98a468756948a679e4107 + integrity: sha512-Gka9ZV/Bryxky[...]LgMJ+0F+FhXMA== + total files: 1 + ), + "", +] +` + +exports[`test/lib/commands/pack.js TAP should log output as valid json > logs pack contents 1`] = ` +Array [] +` + +exports[`test/lib/commands/pack.js TAP should log output as valid json > outputs as json 1`] = ` +Array [ + Array [ + Object { + "bundled": Array [], + "entryCount": 1, + "filename": "test-package-1.0.0.tgz", + "files": Array [ + Object { + "mode": 420, + "path": "package.json", + "size": 41, + }, + ], + "id": "test-package@1.0.0", + "integrity": "sha512-Gka9ZV/BryxkypfvMpTvLfaJE1AUi7PK1EAbYqnVzqtucf6QvUK4CFsLVzagY1GwZVx2T1jwWLgMJ+0F+FhXMA==", + "name": "test-package", + "shasum": "a92a0679a70a450f14f98a468756948a679e4107", + "size": 136, + "unpackedSize": 41, + "version": "1.0.0", + }, + ], +] +` + +exports[`test/lib/commands/pack.js TAP should pack current directory with no arguments > logs pack contents 1`] = ` +Array [ + undefined, + "package: test-package@1.0.0", + undefined, + "41B package.json", + undefined, + String( + name: test-package + version: 1.0.0 + filename: test-package-1.0.0.tgz + package size: 136 B + unpacked size: 41 B + shasum: a92a0679a70a450f14f98a468756948a679e4107 + integrity: sha512-Gka9ZV/Bryxky[...]LgMJ+0F+FhXMA== + total files: 1 + ), + "", +] +` diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index abb45613c85a9..a51ec3e5bb879 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -25,6 +25,7 @@ const RealMockNpm = (t, otherMocks = {}) => { mock.joinedOutput = () => { return mock.outputs.map(o => o.join(' ')).join('\n') } + mock.filteredLogs = title => mock.logs.filter(([t]) => t === title).map(([, , msg]) => msg) const Npm = t.mock('../../lib/npm.js', otherMocks) class MockNpm extends Npm { constructor () { diff --git a/test/lib/commands/pack.js b/test/lib/commands/pack.js index f0349823a677b..bc88772086322 100644 --- a/test/lib/commands/pack.js +++ b/test/lib/commands/pack.js @@ -1,337 +1,125 @@ const t = require('tap') -const { fake: mockNpm } = require('../../fixtures/mock-npm') -const pacote = require('pacote') +const { real: mockNpm } = require('../../fixtures/mock-npm') const path = require('path') +const fs = require('fs') -const OUTPUT = [] -const output = (...msg) => OUTPUT.push(msg) - -const libnpmpack = async (spec, opts) => { - if (!opts) { - throw new Error('expected options object') - } - - return '' -} -const mockPacote = { - manifest: spec => { - if (spec.type === 'directory') { - return pacote.manifest(spec) - } - const m = { - name: spec.name || 'test-package', - version: spec.version || '1.0.0-test', - } - m._id = `${m.name}@${m.version}` - return m - }, -} - -t.afterEach(() => (OUTPUT.length = 0)) +const cwd = process.cwd() +t.afterEach(t => { + process.chdir(cwd) +}) t.test('should pack current directory with no arguments', async t => { - let tarballFileName - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => { - tarballFileName = file - cb() - }, - }, - }) - const npm = mockNpm({ - output, - }) - const pack = new Pack(npm) - - await pack.exec([]) - const filename = `npm-${require('../../../package.json').version}.tgz` - t.strictSame(OUTPUT, [[filename]]) - t.strictSame(tarballFileName, path.resolve(filename)) + const { Npm, outputs, filteredLogs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + }), + }) + process.chdir(npm.prefix) + await npm.exec('pack', []) + const filename = 'test-package-1.0.0.tgz' + t.strictSame(outputs, [[filename]]) + t.matchSnapshot(filteredLogs('notice'), 'logs pack contents') + t.ok(fs.statSync(path.resolve(npm.prefix, filename))) }) t.test('follows pack-destination config', async t => { - let tarballFileName - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => { - tarballFileName = file - cb() - }, - }, - }) - const npm = mockNpm({ - config: { - 'pack-destination': '/tmp/test', - }, - output, - }) - const pack = new Pack(npm) - - await pack.exec([]) - - const filename = `npm-${require('../../../package.json').version}.tgz` - t.strictSame(OUTPUT, [[filename]]) - t.strictSame(tarballFileName, path.resolve('/tmp/test', filename)) -}) - -t.test('should pack given directory', async t => { - const testDir = t.testdir({ - 'package.json': JSON.stringify( - { - name: 'my-cool-pkg', - version: '1.0.0', - }, - null, - 2 - ), - }) - - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - config: { - unicode: true, - json: false, - 'dry-run': true, - }, - output, - }) - const pack = new Pack(npm) - - await pack.exec([testDir]) - - const filename = 'my-cool-pkg-1.0.0.tgz' - t.strictSame(OUTPUT, [[filename]]) + const { Npm, outputs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + }), + 'tar-destination': {}, + }) + process.chdir(npm.prefix) + npm.config.set('pack-destination', path.join(npm.prefix, 'tar-destination')) + await npm.exec('pack', []) + const filename = 'test-package-1.0.0.tgz' + t.strictSame(outputs, [[filename]]) + t.ok(fs.statSync(path.resolve(npm.prefix, 'tar-destination', filename))) }) t.test('should pack given directory for scoped package', async t => { - const testDir = t.testdir({ - 'package.json': JSON.stringify( - { - name: '@cool/my-pkg', - version: '1.0.0', - }, - null, - 2 - ), - }) - - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - config: { - unicode: true, - json: false, - 'dry-run': true, - }, - output, - }) - const pack = new Pack(npm) - - await pack.exec([testDir]) - - const filename = 'cool-my-pkg-1.0.0.tgz' - t.strictSame(OUTPUT, [[filename]]) -}) - -t.test('should log pack contents', async t => { - const Pack = t.mock('../../../lib/commands/pack.js', { - '../../../lib/utils/tar.js': { - ...require('../../../lib/utils/tar.js'), - logTar: () => { - t.ok(true, 'logTar is called') - }, - }, - libnpmpack, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - config: { - unicode: false, - json: false, - 'dry-run': false, - }, - output, - }) - const pack = new Pack(npm) - - await pack.exec([]) - - const filename = `npm-${require('../../../package.json').version}.tgz` - t.strictSame(OUTPUT, [[filename]]) + const { Npm, outputs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: '@npm/test-package', + version: '1.0.0', + }), + }) + process.chdir(npm.prefix) + await npm.exec('pack', []) + const filename = 'npm-test-package-1.0.0.tgz' + t.strictSame(outputs, [[filename]]) + t.ok(fs.statSync(path.resolve(npm.prefix, filename))) }) t.test('should log output as valid json', async t => { - const testDir = t.testdir({ - 'package.json': JSON.stringify( - { - name: 'my-cool-pkg', - version: '1.0.0', - main: './index.js', - }, - null, - 2 - ), - 'README.md': 'text', - 'index.js': 'void', - }) - - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - '../../../lib/utils/tar.js': { - getContents: async () => ({ - id: '@ruyadorno/redact@1.0.0', - name: '@ruyadorno/redact', - version: '1.0.0', - size: 2450, - unpackedSize: 4911, - shasum: '044c7574639b923076069d6e801e2d1866430f17', - // mocks exactly how ssri Integrity works: - integrity: { - sha512: [ - { - /* eslint-disable-next-line max-len */ - source: 'sha512-JSdyskeR2qonBUaQ4vdlU/vQGSfgCxSq5O+vH+d2yVWRqzso4O3gUzd6QX/V7OWV//zU7kA5o63Zf433jUnOtQ==', - /* eslint-disable-next-line max-len */ - digest: 'JSdyskeR2qonBUaQ4vdlU/vQGSfgCxSq5O+vH+d2yVWRqzso4O3gUzd6QX/V7OWV//zU7kA5o63Zf433jUnOtQ==', - algorithm: 'sha512', - options: [], - }, - ], - toJSON () { - /* eslint-disable-next-line max-len */ - return 'sha512-JSdyskeR2qonBUaQ4vdlU/vQGSfgCxSq5O+vH+d2yVWRqzso4O3gUzd6QX/V7OWV//zU7kA5o63Zf433jUnOtQ==' - }, - }, - filename: '@ruyadorno/redact-1.0.0.tgz', - files: [ - { path: 'LICENSE', size: 1113, mode: 420 }, - { path: 'README.md', size: 2639, mode: 420 }, - { path: 'index.js', size: 719, mode: 493 }, - { path: 'package.json', size: 440, mode: 420 }, - ], - entryCount: 4, - bundled: [], - }), - }, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - config: { - unicode: true, - json: true, - 'dry-run': true, - }, - output, - }) - const pack = new Pack(npm) - - await pack.exec([testDir]) + const { Npm, outputs, filteredLogs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + }), + }) + process.chdir(npm.prefix) + npm.config.set('json', true) + await npm.exec('pack', []) + const filename = 'test-package-1.0.0.tgz' + t.matchSnapshot(outputs.map(JSON.parse), 'outputs as json') + t.matchSnapshot(filteredLogs('notice'), 'logs pack contents') + t.ok(fs.statSync(path.resolve(npm.prefix, filename))) +}) - t.match( - JSON.parse(OUTPUT), - [ - { - id: '@ruyadorno/redact@1.0.0', - name: '@ruyadorno/redact', - version: '1.0.0', - size: 2450, - unpackedSize: 4911, - shasum: '044c7574639b923076069d6e801e2d1866430f17', - /* eslint-disable-next-line max-len */ - integrity: 'sha512-JSdyskeR2qonBUaQ4vdlU/vQGSfgCxSq5O+vH+d2yVWRqzso4O3gUzd6QX/V7OWV//zU7kA5o63Zf433jUnOtQ==', - filename: '@ruyadorno/redact-1.0.0.tgz', - files: [ - { path: 'LICENSE' }, - { path: 'README.md' }, - { path: 'index.js' }, - { path: 'package.json' }, - ], - entryCount: 4, - }, - ], - 'pack details output as valid json' - ) +t.test('dry run', async t => { + const { Npm, outputs, filteredLogs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + }), + }) + npm.config.set('dry-run', true) + process.chdir(npm.prefix) + await npm.exec('pack', []) + const filename = 'test-package-1.0.0.tgz' + t.strictSame(outputs, [[filename]]) + t.matchSnapshot(filteredLogs('notice'), 'logs pack contents') + t.throws(() => fs.statSync(path.resolve(npm.prefix, filename))) }) t.test('invalid packument', async t => { - const mockPacote = { - manifest: () => { - return {} - }, - } - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - pacote: mockPacote, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - config: { - unicode: true, - json: false, - 'dry-run': true, - }, - output, - }) - const pack = new Pack(npm) - await t.rejects(pack.exec([]), 'Invalid package, must have name and version') - t.strictSame(OUTPUT, []) + const { Npm, outputs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ + 'package.json': '{}', + }) + process.chdir(npm.prefix) + await t.rejects( + npm.exec('pack', []), + /Invalid package, must have name and version/ + ) + t.strictSame(outputs, []) }) -t.test('workspaces', t => { - const testDir = t.testdir({ +t.test('workspaces', async t => { + const { Npm, outputs } = mockNpm(t) + const npm = new Npm() + await npm.load() + npm.prefix = t.testdir({ 'package.json': JSON.stringify( { name: 'workspaces-test', @@ -354,51 +142,28 @@ t.test('workspaces', t => { }), }, }) - const Pack = t.mock('../../../lib/commands/pack.js', { - libnpmpack, - pacote: mockPacote, - npmlog: { - notice: () => {}, - showProgress: () => {}, - clearProgress: () => {}, - }, - fs: { - writeFile: (file, data, cb) => cb(), - }, - }) - const npm = mockNpm({ - localPrefix: testDir, - config: { - unicode: false, - json: false, - 'dry-run': false, - }, - output, - }) - const pack = new Pack(npm) - + npm.config.set('workspaces', true) t.test('all workspaces', async t => { - await pack.execWorkspaces([], []) - - t.strictSame(OUTPUT, [['workspace-a-1.0.0.tgz'], ['workspace-b-1.0.0.tgz']]) + process.chdir(npm.prefix) + await npm.exec('pack', []) + t.strictSame(outputs, [['workspace-a-1.0.0.tgz'], ['workspace-b-1.0.0.tgz']]) }) t.test('all workspaces, `.` first arg', async t => { - await pack.execWorkspaces(['.'], []) - - t.strictSame(OUTPUT, [['workspace-a-1.0.0.tgz'], ['workspace-b-1.0.0.tgz']]) + process.chdir(npm.prefix) + await npm.exec('pack', ['.']) + t.strictSame(outputs, [['workspace-a-1.0.0.tgz'], ['workspace-b-1.0.0.tgz']]) }) t.test('one workspace', async t => { - await pack.execWorkspaces([], ['workspace-a']) - - t.strictSame(OUTPUT, [['workspace-a-1.0.0.tgz']]) + process.chdir(npm.prefix) + await npm.exec('pack', ['workspace-a']) + t.strictSame(outputs, [['workspace-a-1.0.0.tgz']]) }) t.test('specific package', async t => { - await pack.execWorkspaces(['abbrev'], []) - - t.strictSame(OUTPUT, [['abbrev-1.0.0-test.tgz']]) + process.chdir(npm.prefix) + await npm.exec('pack', [npm.prefix]) + t.strictSame(outputs, [['workspaces-test-1.0.0.tgz']]) }) - t.end() }) diff --git a/test/lib/commands/shrinkwrap.js b/test/lib/commands/shrinkwrap.js index efe7e538051f7..db4021abd6560 100644 --- a/test/lib/commands/shrinkwrap.js +++ b/test/lib/commands/shrinkwrap.js @@ -23,7 +23,7 @@ t.formatSnapshot = obj => // and make some assertions that should always be true. Sets // the results on t.context for use in child tests const shrinkwrap = async (t, testdir = {}, config = {}, mocks = {}) => { - const { Npm, logs } = mockNpm(t, mocks) + const { Npm, filteredLogs } = mockNpm(t, mocks) const npm = new Npm() await npm.load() @@ -39,8 +39,8 @@ const shrinkwrap = async (t, testdir = {}, config = {}, mocks = {}) => { const newFile = resolve(npm.localPrefix, 'npm-shrinkwrap.json') const oldFile = resolve(npm.localPrefix, 'package-lock.json') - const notices = logs.filter(([title]) => title === 'notice').map(([, , msg]) => msg) - const warnings = logs.filter(([title]) => title === 'warn').map(([, , msg]) => msg) + const notices = filteredLogs('notice') + const warnings = filteredLogs('warn') t.notOk(fs.existsSync(oldFile), 'package-lock is always deleted') t.same(warnings, [], 'no warnings')