From 239083d768ebe5f95e8ef7245034570b4f351434 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 11 May 2022 14:40:12 -0700 Subject: [PATCH] chore: remove unit tests npm usage tests move to test/lib/npm.js --- .../npm-usage.js.test.cjs => npm.js.test.cjs} | 20 +-- test/fixtures/mock-registry.js | 4 + test/lib/commands/ping.js | 154 ++++++------------ test/lib/npm.js | 58 +++++++ test/lib/utils/is-windows.js | 39 ----- test/lib/utils/npm-usage.js | 60 ------- test/lib/utils/ping.js | 35 ---- 7 files changed, 126 insertions(+), 244 deletions(-) rename tap-snapshots/test/lib/{utils/npm-usage.js.test.cjs => npm.js.test.cjs} (98%) delete mode 100644 test/lib/utils/is-windows.js delete mode 100644 test/lib/utils/npm-usage.js delete mode 100644 test/lib/utils/ping.js diff --git a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs b/tap-snapshots/test/lib/npm.js.test.cjs similarity index 98% rename from tap-snapshots/test/lib/utils/npm-usage.js.test.cjs rename to tap-snapshots/test/lib/npm.js.test.cjs index 3487378a2119d..dd6107c57810f 100644 --- a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs +++ b/tap-snapshots/test/lib/npm.js.test.cjs @@ -5,7 +5,7 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/npm-usage.js TAP usage basic usage > must match snapshot 1`] = ` +exports[`test/lib/npm.js TAP usage basic usage > must match snapshot 1`] = ` npm Usage: @@ -38,10 +38,10 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@{VERSION} {BASEDIR} +npm@{VERSION} {BASEDIR}/cli ` -exports[`test/lib/utils/npm-usage.js TAP usage set process.stdout.columns columns=0 > must match snapshot 1`] = ` +exports[`test/lib/npm.js TAP usage set process.stdout.columns columns=0 > must match snapshot 1`] = ` npm Usage: @@ -74,10 +74,10 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@{VERSION} {BASEDIR} +npm@{VERSION} {BASEDIR}/cli ` -exports[`test/lib/utils/npm-usage.js TAP usage set process.stdout.columns columns=90 > must match snapshot 1`] = ` +exports[`test/lib/npm.js TAP usage set process.stdout.columns columns=90 > must match snapshot 1`] = ` npm Usage: @@ -110,10 +110,10 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@{VERSION} {BASEDIR} +npm@{VERSION} {BASEDIR}/cli ` -exports[`test/lib/utils/npm-usage.js TAP usage with browser > must match snapshot 1`] = ` +exports[`test/lib/npm.js TAP usage with browser > must match snapshot 1`] = ` npm Usage: @@ -146,10 +146,10 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@{VERSION} {BASEDIR} +npm@{VERSION} {BASEDIR}/cli ` -exports[`test/lib/utils/npm-usage.js TAP usage with long > must match snapshot 1`] = ` +exports[`test/lib/npm.js TAP usage with long > must match snapshot 1`] = ` npm Usage: @@ -1036,5 +1036,5 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@{VERSION} {BASEDIR} +npm@{VERSION} {BASEDIR}/cli ` diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js index a62890b72e2f6..37b1f17f3ebcb 100644 --- a/test/fixtures/mock-registry.js +++ b/test/fixtures/mock-registry.js @@ -192,6 +192,10 @@ class MockRegistry { }).reply(200, { ...manifest, users }) } + ping ({ body = {}, responseCode = 200 } = {}) { + this.nock = this.nock.get('/-/ping?write=true').reply(responseCode, body) + } + async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock const spec = npa(manifest.name) diff --git a/test/lib/commands/ping.js b/test/lib/commands/ping.js index f808e0ac3ba2a..dd2f83de08fc9 100644 --- a/test/lib/commands/ping.js +++ b/test/lib/commands/ping.js @@ -1,113 +1,67 @@ const t = require('tap') -const { fake: mockNpm } = require('../../fixtures/mock-npm') +const { load: loadMockNpm } = require('../../fixtures/mock-npm.js') +const MockRegistry = require('../../fixtures/mock-registry.js') -t.test('pings', async t => { - t.plan(6) - - const registry = 'https://registry.npmjs.org' - let noticeCalls = 0 - const Ping = t.mock('../../../lib/commands/ping.js', { - '../../../lib/utils/ping.js': function (spec) { - t.equal(spec.registry, registry, 'passes flatOptions') - return {} - }, - 'proc-log': { - notice: (type, spec) => { - ++noticeCalls - if (noticeCalls === 1) { - t.equal(type, 'PING', 'should log a PING') - t.equal(spec, registry, 'should log the registry url') - } else { - t.equal(type, 'PONG', 'should log a PONG') - t.match(spec, /\d+ms/, 'should log the elapsed milliseconds') - } - }, - }, - }) - const npm = mockNpm({ - config: { registry }, - flatOptions: { registry }, +t.test('no details', async t => { + const { npm, logs, joinedOutput } = await loadMockNpm(t) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) - const ping = new Ping(npm) - - await ping.exec([]) - t.equal(noticeCalls, 2, 'should have logged 2 lines') + registry.ping() + await npm.exec('ping', []) + t.match(logs.notice, [['PING', 'https://registry.npmjs.org/'], ['PONG', /[0-9]+ms/]]) + t.equal(joinedOutput(), '') }) -t.test('pings and logs details', async t => { - t.plan(8) +t.test('with details', async t => { + const { npm, logs, joinedOutput } = await loadMockNpm(t) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + registry.ping({ body: { test: true } }) + await npm.exec('ping', []) + t.match(logs.notice, [ + ['PING', 'https://registry.npmjs.org/'], + ['PONG', /[0-9]+ms/], + ['PONG', '{\n "test": true\n}'], + ]) + t.match(joinedOutput(), '') +}) - const registry = 'https://registry.npmjs.org' - const details = { extra: 'data' } - let noticeCalls = 0 - const Ping = t.mock('../../../lib/commands/ping.js', { - '../../../lib/utils/ping.js': function (spec) { - t.equal(spec.registry, registry, 'passes flatOptions') - return details - }, - 'proc-log': { - notice: (type, spec) => { - ++noticeCalls - if (noticeCalls === 1) { - t.equal(type, 'PING', 'should log a PING') - t.equal(spec, registry, 'should log the registry url') - } else if (noticeCalls === 2) { - t.equal(type, 'PONG', 'should log a PONG') - t.match(spec, /\d+ms/, 'should log the elapsed milliseconds') - } else { - t.equal(type, 'PONG', 'should log a PONG') - const parsed = JSON.parse(spec) - t.match(parsed, details, 'should log JSON stringified details') - } - }, - }, +t.test('valid json', async t => { + const { npm, logs, joinedOutput } = await loadMockNpm(t, { + config: { json: true }, }) - const npm = mockNpm({ - config: { registry }, - flatOptions: { registry }, + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + registry.ping() + await npm.exec('ping', []) + t.match(logs.notice, [['PING', 'https://registry.npmjs.org/'], ['PONG', /[0-9]+ms/]]) + t.match(JSON.parse(joinedOutput()), { + registry: npm.config.get('registry'), + time: /[0-9]+/, + details: {}, }) - const ping = new Ping(npm) - - await ping.exec([]) - t.equal(noticeCalls, 3, 'should have logged 3 lines') }) -t.test('pings and returns json', async t => { - t.plan(9) - - const registry = 'https://registry.npmjs.org' - const details = { extra: 'data' } - let noticeCalls = 0 - const Ping = t.mock('../../../lib/commands/ping.js', { - '../../../lib/utils/ping.js': function (spec) { - t.equal(spec.registry, registry, 'passes flatOptions') - return details - }, - 'proc-log': { - notice: (type, spec) => { - ++noticeCalls - if (noticeCalls === 1) { - t.equal(type, 'PING', 'should log a PING') - t.equal(spec, registry, 'should log the registry url') - } else { - t.equal(type, 'PONG', 'should log a PONG') - t.match(spec, /\d+ms/, 'should log the elapsed milliseconds') - } - }, - }, +t.test('invalid json', async t => { + const { npm, logs, joinedOutput } = await loadMockNpm(t, { + config: { json: true }, }) - const npm = mockNpm({ - config: { registry, json: true }, - flatOptions: { registry }, - output: function (spec) { - const parsed = JSON.parse(spec) - t.equal(parsed.registry, registry, 'returns the correct registry url') - t.match(parsed.details, details, 'prints returned details') - t.type(parsed.time, 'number', 'returns time as a number') - }, + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + registry.ping({ body: '{not: real"json]' }) + await npm.exec('ping', []) + t.match(logs.notice, [['PING', 'https://registry.npmjs.org/'], ['PONG', /[0-9]+ms/]]) + t.match(JSON.parse(joinedOutput()), { + registry: npm.config.get('registry'), + time: /[0-9]+/, + details: {}, }) - const ping = new Ping(npm) - - await ping.exec([]) - t.equal(noticeCalls, 2, 'should have logged 2 lines') }) diff --git a/test/lib/npm.js b/test/lib/npm.js index 566cbca20f6bf..b2ada9b9dcf81 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -654,3 +654,61 @@ t.test('implicit workspace accept', async t => { }) await t.rejects(mock.npm.exec('org', []), /.*Usage/) }) + +t.test('usage', async t => { + const { npm } = await loadMockNpm(t) + t.afterEach(() => { + npm.config.set('viewer', null) + npm.config.set('long', false) + npm.config.set('userconfig', '/some/config/file/.npmrc') + }) + const { dirname } = require('path') + const basedir = dirname(dirname(dirname(__dirname))) + t.cleanSnapshot = str => str.split(basedir).join('{BASEDIR}') + .split(require('../../package.json').version).join('{VERSION}') + + npm.config.set('viewer', null) + npm.config.set('long', false) + npm.config.set('userconfig', '/some/config/file/.npmrc') + + t.test('basic usage', async t => { + t.matchSnapshot(await npm.usage) + t.end() + }) + + t.test('with browser', async t => { + npm.config.set('viewer', 'browser') + t.matchSnapshot(await npm.usage) + t.end() + }) + + t.test('with long', async t => { + npm.config.set('long', true) + t.matchSnapshot(await npm.usage) + t.end() + }) + + t.test('set process.stdout.columns', async t => { + const { columns } = process.stdout + t.teardown(() => { + Object.defineProperty(process.stdout, 'columns', { + value: columns, + enumerable: true, + configurable: true, + writable: true, + }) + }) + const cases = [0, 90] + for (const cols of cases) { + t.test(`columns=${cols}`, async t => { + Object.defineProperty(process.stdout, 'columns', { + value: cols, + enumerable: true, + configurable: true, + writable: true, + }) + t.matchSnapshot(await npm.usage) + }) + } + }) +}) diff --git a/test/lib/utils/is-windows.js b/test/lib/utils/is-windows.js deleted file mode 100644 index a1d520f0629f6..0000000000000 --- a/test/lib/utils/is-windows.js +++ /dev/null @@ -1,39 +0,0 @@ -const t = require('tap') - -const mockGlobals = require('../../fixtures/mock-globals') - -t.test('is not windows', async t => { - mockGlobals(t, { 'process.platform': 'posix' }) - t.match({ - isWindows: false, - isWindowsShell: false, - }, t.mock('../../../lib/utils/is-windows.js')) -}) - -t.test('is windows, shell', async t => { - mockGlobals(t, { - 'process.platform': 'win32', - 'process.env': { - MSYSTEM: 'notmingw', - TERM: 'notcygwin', - }, - }) - t.match({ - isWindows: true, - isWindowsShell: true, - }, t.mock('../../../lib/utils/is-windows.js')) -}) - -t.test('is windows, not shell', async t => { - mockGlobals(t, { - 'process.platform': 'win32', - 'process.env': { - MSYSTEM: 'MINGW32', - TERM: 'cygwin', - }, - }) - t.match({ - isWindows: true, - isWindowsShell: false, - }, t.mock('../../../lib/utils/is-windows.js')) -}) diff --git a/test/lib/utils/npm-usage.js b/test/lib/utils/npm-usage.js deleted file mode 100644 index 035d4bbb21ef7..0000000000000 --- a/test/lib/utils/npm-usage.js +++ /dev/null @@ -1,60 +0,0 @@ -const t = require('tap') -const { load: loadMockNpm } = require('../../fixtures/mock-npm.js') - -t.test('usage', async t => { - const { npm } = await loadMockNpm(t) - t.afterEach(() => { - npm.config.set('viewer', null) - npm.config.set('long', false) - npm.config.set('userconfig', '/some/config/file/.npmrc') - }) - const { dirname } = require('path') - const basedir = dirname(dirname(dirname(__dirname))) - t.cleanSnapshot = str => str.split(basedir).join('{BASEDIR}') - .split(require('../../../package.json').version).join('{VERSION}') - - npm.config.set('viewer', null) - npm.config.set('long', false) - npm.config.set('userconfig', '/some/config/file/.npmrc') - - t.test('basic usage', async t => { - t.matchSnapshot(await npm.usage) - t.end() - }) - - t.test('with browser', async t => { - npm.config.set('viewer', 'browser') - t.matchSnapshot(await npm.usage) - t.end() - }) - - t.test('with long', async t => { - npm.config.set('long', true) - t.matchSnapshot(await npm.usage) - t.end() - }) - - t.test('set process.stdout.columns', async t => { - const { columns } = process.stdout - t.teardown(() => { - Object.defineProperty(process.stdout, 'columns', { - value: columns, - enumerable: true, - configurable: true, - writable: true, - }) - }) - const cases = [0, 90] - for (const cols of cases) { - t.test(`columns=${cols}`, async t => { - Object.defineProperty(process.stdout, 'columns', { - value: cols, - enumerable: true, - configurable: true, - writable: true, - }) - t.matchSnapshot(await npm.usage) - }) - } - }) -}) diff --git a/test/lib/utils/ping.js b/test/lib/utils/ping.js deleted file mode 100644 index 1bebfa69d2b78..0000000000000 --- a/test/lib/utils/ping.js +++ /dev/null @@ -1,35 +0,0 @@ -const t = require('tap') - -t.test('pings', async (t) => { - t.plan(3) - - const options = { fake: 'options' } - const response = { some: 'details' } - const ping = t.mock('../../../lib/utils/ping.js', { - 'npm-registry-fetch': (url, opts) => { - t.equal(url, '/-/ping?write=true', 'calls the correct url') - t.equal(opts, options, 'passes through options') - return { json: () => Promise.resolve(response) } - }, - }) - - const res = await ping(options) - t.match(res, response, 'returns json response') -}) - -t.test('catches errors and returns empty json', async (t) => { - t.plan(3) - - const options = { fake: 'options' } - const response = { some: 'details' } - const ping = t.mock('../../../lib/utils/ping.js', { - 'npm-registry-fetch': (url, opts) => { - t.equal(url, '/-/ping?write=true', 'calls the correct url') - t.equal(opts, options, 'passes through options') - return { json: () => Promise.reject(response) } - }, - }) - - const res = await ping(options) - t.match(res, {}, 'returns empty json response') -})