From f445e0da759e49508113cfb70fb80b79f61aa61c Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 9 Nov 2021 07:18:16 -0800 Subject: [PATCH] fix(install): command completion with single match During a refactoring of the tests a bug was found in the install command completion that would return nothing if there was a valid match, this fixes that bug and also makes the tests actually test things. --- lib/commands/install.js | 39 ++--- test/lib/commands/install.js | 299 ++++++++++++++++------------------- 2 files changed, 154 insertions(+), 184 deletions(-) diff --git a/lib/commands/install.js b/lib/commands/install.js index 95b5a5bac1d38..fa24b4b54cc24 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -77,38 +77,33 @@ class Install extends ArboristWorkspaceCmd { const partialName = partialWord.slice(lastSlashIdx + 1) const partialPath = partialWord.slice(0, lastSlashIdx) || '/' - const annotatePackageDirMatch = async sibling => { - const fullPath = join(partialPath, sibling) + const isDirMatch = async sibling => { if (sibling.slice(0, partialName.length) !== partialName) { - return null - } // not name match + return false + } try { - const contents = await readdir(fullPath) - return { - fullPath, - isPackage: contents.indexOf('package.json') !== -1, - } + const contents = await readdir(join(partialPath, sibling)) + const result = (contents.indexOf('package.json') !== -1) + return result } catch (er) { - return { isPackage: false } + return false } } try { const siblings = await readdir(partialPath) - const matches = await Promise.all( - siblings.map(async sibling => { - return await annotatePackageDirMatch(sibling) - }) - ) - const match = matches.filter(el => !el || el.isPackage).pop() - if (match) { - // Success - only one match and it is a package dir - return [match.fullPath] - } else { - // no matches - return [] + const matches = [] + for (const sibling of siblings) { + if (await isDirMatch(sibling)) { + matches.push(sibling) + } + } + if (matches.length === 1) { + return [join(partialPath, matches[0])] } + // no matches + return [] } catch (er) { return [] // invalid dir: no matching } diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 9de2ae2781c12..1529d1d43488f 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -1,21 +1,17 @@ const t = require('tap') -const Install = require('../../../lib/commands/install.js') -const { fake: mockNpm } = require('../../fixtures/mock-npm') +const { real: mockNpm } = require('../../fixtures/mock-npm') -t.test('should install using Arborist', (t) => { +t.test('should install using Arborist', async t => { const SCRIPTS = [] let ARB_ARGS = null let REIFY_CALLED = false let ARB_OBJ = null - const Install = t.mock('../../../lib/commands/install.js', { + const { Npm } = mockNpm(t, { '@npmcli/run-script': ({ event }) => { SCRIPTS.push(event) }, - npmlog: { - warn: () => {}, - }, '@npmcli/arborist': function (args) { ARB_ARGS = args ARB_OBJ = this @@ -23,33 +19,31 @@ t.test('should install using Arborist', (t) => { REIFY_CALLED = true } }, - '../../../lib/utils/reify-finish.js': (npm, arb) => { + '../../lib/utils/reify-finish.js': (npm, arb) => { if (arb !== ARB_OBJ) { throw new Error('got wrong object passed to reify-finish') } }, }) - const npm = mockNpm({ - config: { dev: true }, - flatOptions: { global: false, auditLevel: 'low' }, - globalDir: 'path/to/node_modules/', - prefix: 'foo', - }) - const install = new Install(npm) + const npm = new Npm() + await npm.load() + npm.config.set('dev', true) + npm.config.set('audit-level', 'low') + npm.prefix = t.testdir({}) t.test('with args', async t => { - await install.exec(['fizzbuzz']) + await npm.exec('install', ['fizzbuzz']) t.match(ARB_ARGS, - { global: false, path: 'foo', auditLevel: null }, + { global: false, path: npm.prefix, auditLevel: null }, 'Arborist gets correct args and ignores auditLevel') t.equal(REIFY_CALLED, true, 'called reify') t.strictSame(SCRIPTS, [], 'no scripts when adding dep') }) t.test('just a local npm install', async t => { - await install.exec([]) - t.match(ARB_ARGS, { global: false, path: 'foo' }) + await npm.exec('install', []) + t.match(ARB_ARGS, { global: false, path: npm.prefix }) t.equal(REIFY_CALLED, true, 'called reify') t.strictSame(SCRIPTS, [ 'preinstall', @@ -61,15 +55,13 @@ t.test('should install using Arborist', (t) => { 'postprepare', ], 'exec scripts when doing local build') }) - - t.end() }) t.test('should ignore scripts with --ignore-scripts', async t => { const SCRIPTS = [] let REIFY_CALLED = false - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, + const { Npm } = mockNpm(t, { + '../../lib/utils/reify-finish.js': async () => {}, '@npmcli/run-script': ({ event }) => { SCRIPTS.push(event) }, @@ -79,41 +71,47 @@ t.test('should ignore scripts with --ignore-scripts', async t => { } }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - prefix: 'foo', - flatOptions: { global: false }, - config: { - global: false, - 'ignore-scripts': true, - }, - }) - const install = new Install(npm) - await install.exec([]) + const npm = new Npm() + await npm.load() + npm.config.set('ignore-scripts', true) + npm.prefix = t.testdir({}) + await npm.exec('install', []) t.equal(REIFY_CALLED, true, 'called reify') t.strictSame(SCRIPTS, [], 'no scripts when adding dep') }) t.test('should install globally using Arborist', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, - '@npmcli/arborist': function () { - this.reify = () => {} + const SCRIPTS = [] + let ARB_ARGS = null + let REIFY_CALLED + const { Npm } = mockNpm(t, { + '@npmcli/run-script': ({ event }) => { + SCRIPTS.push(event) + }, + '../../lib/utils/reify-finish.js': async () => {}, + '@npmcli/arborist': function (args) { + ARB_ARGS = args + this.reify = () => { + REIFY_CALLED = true + } }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - prefix: 'foo', - config: { global: true }, - flatOptions: { global: true }, - }) - const install = new Install(npm) - await install.exec([]) + const npm = new Npm() + await npm.load() + npm.config.set('global', true) + npm.globalPrefix = t.testdir({}) + await npm.exec('install', []) + t.match( + ARB_ARGS, + { global: true, path: npm.globalPrefix } + ) + t.equal(REIFY_CALLED, true, 'called reify') + t.strictSame(SCRIPTS, [], 'no scripts when installing globally') }) t.test('npm i -g npm engines check success', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, + const { Npm } = mockNpm(t, { + '../../lib/utils/reify-finish.js': async () => {}, '@npmcli/arborist': function () { this.reify = () => {} }, @@ -128,18 +126,16 @@ t.test('npm i -g npm engines check success', async t => { }, }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - config: { - global: true, - }, - }) - const install = new Install(npm) - await install.exec(['npm']) + const npm = new Npm() + await npm.load() + npm.globalDir = t.testdir({}) + npm.config.set('global', true) + await npm.exec('install', ['npm']) + t.ok('No exceptions happen') }) t.test('npm i -g npm engines check failure', async t => { - const Install = t.mock('../../../lib/commands/install.js', { + const { Npm } = mockNpm(t, { pacote: { manifest: () => { return { @@ -152,15 +148,12 @@ t.test('npm i -g npm engines check failure', async t => { }, }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - config: { - global: true, - }, - }) - const install = new Install(npm) + const npm = new Npm() + await npm.load() + npm.globalDir = t.testdir({}) + npm.config.set('global', true) await t.rejects( - install.exec(['npm']), + npm.exec('install', ['npm']), { message: 'Unsupported engine', pkgid: 'npm@1.2.3', @@ -177,8 +170,8 @@ t.test('npm i -g npm engines check failure', async t => { }) t.test('npm i -g npm engines check failure forced override', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, + const { Npm } = mockNpm(t, { + '../../lib/utils/reify-finish.js': async () => {}, '@npmcli/arborist': function () { this.reify = () => {} }, @@ -194,19 +187,17 @@ t.test('npm i -g npm engines check failure forced override', async t => { }, }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - config: { - force: true, - global: true, - }, - }) - const install = new Install(npm) - await install.exec(['npm']) + const npm = new Npm() + await npm.load() + npm.globalDir = t.testdir({}) + npm.config.set('global', true) + npm.config.set('force', true) + await npm.exec('install', ['npm']) + t.ok('Does not throw') }) t.test('npm i -g npm@version engines check failure', async t => { - const Install = t.mock('../../../lib/commands/install.js', { + const { Npm } = mockNpm(t, { pacote: { manifest: () => { return { @@ -219,15 +210,12 @@ t.test('npm i -g npm@version engines check failure', async t => { }, }, }) - const npm = mockNpm({ - globalDir: 'path/to/node_modules/', - config: { - global: true, - }, - }) - const install = new Install(npm) + const npm = new Npm() + await npm.load() + npm.globalDir = t.testdir({}) + npm.config.set('global', true) await t.rejects( - install.exec(['npm@100']), + npm.exec('install', ['npm@100']), { message: 'Unsupported engine', pkgid: 'npm@1.2.3', @@ -243,91 +231,78 @@ t.test('npm i -g npm@version engines check failure', async t => { ) }) -t.test('completion to folder', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, - util: { - promisify: (fn) => fn, - }, - fs: { - readdir: (path) => { - if (path === '/') { - return ['arborist'] - } else { - return ['package.json'] - } - }, +t.test('completion', async t => { + const cwd = process.cwd() + const testdir = t.testdir({ + arborist: { + 'package.json': '{}', }, + 'arborist.txt': 'just a file', + other: {}, + }) + t.afterEach(() => { + process.chdir(cwd) }) - const install = new Install({}) - const res = await install.completion({ partialWord: '/ar' }) - const expect = process.platform === 'win32' ? '\\arborist' : '/arborist' - t.strictSame(res, [expect], 'package dir match') -}) -t.test('completion to folder - invalid dir', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, - util: { - promisify: (fn) => fn, - }, - fs: { - readdir: () => { - throw new Error('EONT') - }, - }, + t.test('completion to folder - has a match', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: './ar' }) + t.strictSame(res, ['arborist'], 'package dir match') }) - const install = new Install({}) - const res = await install.completion({ partialWord: 'path/to/folder' }) - t.strictSame(res, [], 'invalid dir: no matching') -}) -t.test('completion to folder - no matches', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, - util: { - promisify: (fn) => fn, - }, - fs: { - readdir: (path) => { - return ['foobar'] - }, - }, + t.test('completion to folder - invalid dir', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + const res = await install.completion({ partialWord: '/does/not/exist' }) + t.strictSame(res, [], 'invalid dir: no matching') }) - const install = new Install({}) - const res = await install.completion({ partialWord: '/pa' }) - t.strictSame(res, [], 'no name match') -}) -t.test('completion to folder - match is not a package', async t => { - const Install = t.mock('../../../lib/commands/install.js', { - '../../../lib/utils/reify-finish.js': async () => {}, - util: { - promisify: (fn) => fn, - }, - fs: { - readdir: (path) => { - if (path === '/') { - return ['arborist'] - } else { - throw new Error('EONT') - } - }, - }, + t.test('completion to folder - no matches', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: './pa' }) + t.strictSame(res, [], 'no name match') }) - const install = new Install({}) - const res = await install.completion({ partialWord: '/ar' }) - t.strictSame(res, [], 'no name match') -}) -t.test('completion to url', async t => { - const install = new Install({}) - const res = await install.completion({ partialWord: 'http://path/to/url' }) - t.strictSame(res, []) -}) + t.test('completion to folder - match is not a package', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: './othe' }) + t.strictSame(res, [], 'no name match') + }) -t.test('completion', async t => { - const install = new Install({}) - const res = await install.completion({ partialWord: 'toto' }) - t.notOk(res) + t.test('completion to url', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: 'http://path/to/url' }) + t.strictSame(res, []) + }) + + t.test('no /', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: 'toto' }) + t.notOk(res) + }) + + t.test('only /', async t => { + const { Npm } = mockNpm(t) + const npm = new Npm() + const install = await npm.cmd('install') + process.chdir(testdir) + const res = await install.completion({ partialWord: '/' }) + t.strictSame(res, []) + }) })