From 24273a862e54abfd022df9fc4b8c250bfe77817c Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 27 Jul 2021 15:38:07 -0700 Subject: feat(workspaces): add --include-workspace-root and explicit --no-workspaces Adds a new config item that includes the workspace root. This also changes --workspaces to a trinary, so that setting it to false will explicitly exclude workspaces altogether. PR-URL: https://github.com/npm/cli/pull/3890 Credit: @fritzy Close: #3890 Reviewed-by: @wraithgar --- test/lib/explain.js | 79 ++++++++++++++++++- test/lib/init.js | 23 ++++++ test/lib/ls.js | 20 +++++ test/lib/npm.js | 38 +++++++++ test/lib/outdated.js | 20 +++++ test/lib/publish.js | 1 + test/lib/repo.js | 118 ++++++++++++++-------------- test/lib/utils/completion/installed-deep.js | 1 + test/lib/utils/config/definitions.js | 17 ++++ test/lib/workspaces/get-workspaces.js | 11 +++ 10 files changed, 270 insertions(+), 58 deletions(-) (limited to 'test') diff --git a/test/lib/explain.js b/test/lib/explain.js index f690aeb2c..ebec13619 100644 --- a/test/lib/explain.js +++ b/test/lib/explain.js @@ -2,7 +2,7 @@ const t = require('tap') const npm = { prefix: null, color: true, - flatOptions: {}, + flatOptions: { workspacesEnabled: true }, output: (...args) => { OUTPUT.push(args) }, @@ -301,3 +301,80 @@ t.test('workspaces', async t => { }) }) }) + +t.test('workspaces disabled', async t => { + npm.localPrefix = npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'workspaces-project', + version: '1.0.0', + workspaces: ['packages/*'], + dependencies: { + abbrev: '^1.0.0', + }, + }), + node_modules: { + a: t.fixture('symlink', '../packages/a'), + b: t.fixture('symlink', '../packages/b'), + c: t.fixture('symlink', '../packages/c'), + once: { + 'package.json': JSON.stringify({ + name: 'once', + version: '1.0.0', + dependencies: { + wrappy: '2.0.0', + }, + }), + }, + abbrev: { + 'package.json': JSON.stringify({ + name: 'abbrev', + version: '1.0.0', + }), + }, + wrappy: { + 'package.json': JSON.stringify({ + name: 'wrappy', + version: '2.0.0', + }), + }, + }, + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { + once: '1.0.0', + }, + }), + }, + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '1.0.0', + dependencies: { + abbrev: '^1.0.0', + }, + }), + }, + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + }), + }, + }, + }) + + await new Promise((res, rej) => { + explain.npm.flatOptions.workspacesEnabled = false + explain.exec(['once'], err => { + t.equal( + err, + 'No dependencies found matching once', + 'should throw usage if dep not found when excluding ws' + ) + res() + }) + }) +}) diff --git a/test/lib/init.js b/test/lib/init.js index 1cefb1fc9..f11ce356f 100644 --- a/test/lib/init.js +++ b/test/lib/init.js @@ -513,3 +513,26 @@ t.test('workspaces', t => { t.end() }) +t.test('npm init workspces with root', t => { + t.teardown(() => { + npm._mockOutputs.length = 0 + }) + npm.localPrefix = t.testdir({}) + npm.flatOptions.includeWorkspaceRoot = true + + // init-package-json prints directly to console.log + // this avoids poluting test output with those logs + console.log = noop + + process.chdir(npm.localPrefix) + init.execWorkspaces([], ['packages/a'], err => { + if (err) + throw err + + const pkg = require(resolve(npm.localPrefix, 'package.json')) + t.equal(pkg.version, '1.0.0') + t.equal(pkg.license, 'ISC') + t.matchSnapshot(npm._mockOutputs, 'does not print helper info') + t.end() + }) +}) diff --git a/test/lib/ls.js b/test/lib/ls.js index 5f196501e..46dfd7fba 100644 --- a/test/lib/ls.js +++ b/test/lib/ls.js @@ -110,6 +110,7 @@ const config = { 'package-lock-only': false, } const flatOptions = { + workspacesEnabled: true, } const npm = mockNpm({ config, @@ -1530,6 +1531,25 @@ t.test('ls', (t) => { }) }) + await new Promise((res, rej) => { + config.all = false + config.depth = 0 + npm.color = true + npm.flatOptions.workspacesEnabled = false + ls.exec([], (err) => { + if (err) + rej(err) + + t.matchSnapshot(redactCwd(result), + 'should not list workspaces with --no-workspaces') + config.all = true + config.depth = Infinity + npm.color = false + npm.flatOptions.workspacesEnabled = true + res() + }) + }) + // --all await new Promise((res, rej) => { ls.exec([], (err) => { diff --git a/test/lib/npm.js b/test/lib/npm.js index 7d6176247..1451cd879 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -292,6 +292,44 @@ t.test('npm.load', t => { await new Promise((res) => setTimeout(res)) }) + t.test('--no-workspaces with --workspace', async t => { + const dir = t.testdir({ + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { test: 'echo test a' }, + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['./packages/*'], + }), + }) + process.argv = [ + process.execPath, + process.argv[1], + '--userconfig', resolve(dir, '.npmrc'), + '--color', 'false', + '--workspaces', 'false', + '--workspace', 'a', + ] + const { npm } = mockNpm(t) + await npm.load() + npm.localPrefix = dir + await new Promise((res, rej) => { + npm.commands.run([], er => { + if (!er) + return rej(new Error('Expected an error')) + t.match(er.message, 'Can not use --no-workspaces and --workspace at the same time') + res() + }) + }) + }) + t.test('workspace-aware configs and commands', async t => { const dir = t.testdir({ packages: { diff --git a/test/lib/outdated.js b/test/lib/outdated.js index 518436d0a..acd5c25e8 100644 --- a/test/lib/outdated.js +++ b/test/lib/outdated.js @@ -83,6 +83,10 @@ const globalDir = t.testdir({ }, }) +const flatOptions = { + workspacesEnabled: true, +} + const outdated = (dir, opts) => { logs = '' const Outdated = t.mock('../../lib/outdated.js', { @@ -94,6 +98,7 @@ const outdated = (dir, opts) => { ...opts, localPrefix: dir, prefix: dir, + flatOptions, globalDir: `${globalDir}/node_modules`, output, }) @@ -561,6 +566,21 @@ t.test('workspaces', async t => { }) }) + await new Promise((res, rej) => { + flatOptions.workspacesEnabled = false + outdated(testDir, {}).exec([], err => { + if (err) + rej(err) + + // TODO: This should display dog, but doesn't because arborist filters + // workspace deps even if they're also root deps + // This will be fixed in a future arborist version + t.matchSnapshot(logs, 'should display only root outdated when ws disabled') + flatOptions.workspacesEnabled = true + res() + }) + }) + await new Promise((res, rej) => { outdated(testDir, { config: { diff --git a/test/lib/publish.js b/test/lib/publish.js index df73b6863..6b0021db6 100644 --- a/test/lib/publish.js +++ b/test/lib/publish.js @@ -57,6 +57,7 @@ t.test('should publish with libnpmpublish, passing through flatOptions and respe const npm = mockNpm({ flatOptions: { customValue: true, + workspacesEnabled: true, }, }) npm.config.getCredentialsByURI = (uri) => { diff --git a/test/lib/repo.js b/test/lib/repo.js index e1ac90b1e..41bff2744 100644 --- a/test/lib/repo.js +++ b/test/lib/repo.js @@ -1,5 +1,5 @@ const t = require('tap') -const { fake: mockNpm } = require('../fixtures/mock-npm.js') +const { real: mockNpm } = require('../fixtures/mock-npm.js') const { join, sep } = require('path') const pkgDirs = t.testdir({ @@ -154,6 +154,7 @@ const pkgDirs = t.testdir({ name: 'workspaces-test', version: '1.2.3-test', workspaces: ['workspace-a', 'workspace-b', 'workspace-c'], + repository: 'https://github.com/npm/workspaces-test', }), 'workspace-a': { 'package.json': JSON.stringify({ @@ -185,19 +186,17 @@ const openUrl = async (npm, url, errMsg) => { opened[url]++ } -const Repo = t.mock('../../lib/repo.js', { +const { command, npm } = mockNpm(t, { '../../lib/utils/open-url.js': openUrl, }) -const flatOptions = {} -const npm = mockNpm({ flatOptions }) -const repo = new Repo(npm) +t.before(async () => { + await npm.load() +}) t.afterEach(() => opened = {}) t.test('open repo urls', t => { - // XXX It is very odd that `where` is how pacote knows to look anywhere other - // than the cwd. I would think npm.localPrefix would factor in somehow - flatOptions.where = pkgDirs + npm.localPrefix = pkgDirs const expect = { hostedgit: 'https://github.com/foo/hostedgit', hostedgitat: 'https://github.com/foo/hostedgitat', @@ -227,22 +226,19 @@ t.test('open repo urls', t => { const keys = Object.keys(expect) t.plan(keys.length) keys.forEach(pkg => { - t.test(pkg, t => { - repo.exec([['.', pkg].join(sep)], (err) => { - if (err) - throw err - const url = expect[pkg] - t.match({ - [url]: 1, - }, opened, `opened ${url}`, {opened}) - t.end() - }) + t.test(pkg, async t => { + await command('repo', [['.', pkg].join(sep)]) + const url = expect[pkg] + t.match({ + [url]: 1, + }, opened, `opened ${url}`, {opened}) + t.end() }) }) }) t.test('fail if cannot figure out repo url', t => { - flatOptions.where = pkgDirs + npm.localPrefix = pkgDirs const cases = [ 'norepo', 'repoobbj-nourl', @@ -253,57 +249,65 @@ t.test('fail if cannot figure out repo url', t => { t.plan(cases.length) cases.forEach(pkg => { - t.test(pkg, t => { - repo.exec([['.', pkg].join(sep)], (err) => { - t.match(err, { pkgid: pkg }) - t.end() - }) + t.test(pkg, async t => { + t.rejects( + command('repo', [['.', pkg].join(sep)]), + { pkgid: pkg } + ) }) }) }) -t.test('open default package if none specified', t => { - flatOptions.where = pkgDirs - repo.exec([], (er) => { - if (er) - throw er - t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened}) - t.end() - }) +t.test('open default package if none specified', async t => { + npm.localPrefix = pkgDirs + await command('repo', []) + t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened}) }) t.test('workspaces', t => { - flatOptions.where = undefined npm.localPrefix = join(pkgDirs, 'workspaces') - t.test('all workspaces', (t) => { - repo.execWorkspaces([], [], (err) => { - t.notOk(err) - t.match({ - 'https://repo.workspace-a/': 1, // Gets translated to https! - 'https://github.com/npm/workspace-b': 1, - }, opened, 'opened two valid repo urls') - t.end() - }) + t.afterEach(() => { + npm.config.set('workspaces', null) + npm.config.set('workspace', []) + npm.config.set('include-workspace-root', false) }) - t.test('one workspace', (t) => { - repo.execWorkspaces([], ['workspace-a'], (err) => { - t.notOk(err) - t.match({ - 'https://repo.workspace-a/': 1, - }, opened, 'opened one requested repo urls') - t.end() - }) + t.test('include workspace root', async (t) => { + npm.config.set('workspaces', true) + npm.config.set('include-workspace-root', true) + await command('repo', []) + t.match({ + 'https://github.com/npm/workspaces-test': 1, + 'https://repo.workspace-a/': 1, // Gets translated to https! + 'https://github.com/npm/workspace-b': 1, + }, opened, 'opened two valid repo urls') }) - t.test('invalid workspace', (t) => { - repo.execWorkspaces([], ['workspace-x'], (err) => { - t.match(err, /No workspaces found/) - t.match(err, /workspace-x/) - t.match({}, opened, 'opened no repo urls') - t.end() - }) + t.test('all workspaces', async (t) => { + npm.config.set('workspaces', true) + await command('repo', []) + t.match({ + 'https://repo.workspace-a/': 1, // Gets translated to https! + 'https://github.com/npm/workspace-b': 1, + }, opened, 'opened two valid repo urls') + }) + + t.test('one workspace', async (t) => { + npm.config.set('workspace', ['workspace-a']) + await command('repo', []) + t.match({ + 'https://repo.workspace-a/': 1, + }, opened, 'opened one requested repo urls') + }) + + t.test('invalid workspace', async (t) => { + npm.config.set('workspace', ['workspace-x']) + await t.rejects( + command('repo', []), + /workspace-x/ + ) + t.match({}, opened, 'opened no repo urls') }) t.end() }) diff --git a/test/lib/utils/completion/installed-deep.js b/test/lib/utils/completion/installed-deep.js index aa0d85ec1..f0e36faee 100644 --- a/test/lib/utils/completion/installed-deep.js +++ b/test/lib/utils/completion/installed-deep.js @@ -6,6 +6,7 @@ let globalDir = 'MISSING_GLOBAL_DIR' const _flatOptions = { depth: Infinity, global: false, + workspacesEnabled: true, get prefix () { return prefix }, diff --git a/test/lib/utils/config/definitions.js b/test/lib/utils/config/definitions.js index 88993303b..622e603bc 100644 --- a/test/lib/utils/config/definitions.js +++ b/test/lib/utils/config/definitions.js @@ -875,3 +875,20 @@ t.test('workspace', t => { t.match(flat.userAgent, /workspaces\/true/) t.end() }) + +t.test('workspaces derived', t => { + const obj = { + workspaces: ['a'], + 'user-agent': definitions['user-agent'].default, + } + const flat = {} + definitions.workspaces.flatten('workspaces', obj, flat) + t.equal(flat.workspacesEnabled, true) + obj.workspaces = null + definitions.workspaces.flatten('workspaces', obj, flat) + t.equal(flat.workspacesEnabled, true) + obj.workspaces = false + definitions.workspaces.flatten('workspaces', obj, flat) + t.equal(flat.workspacesEnabled, false) + t.end() +}) diff --git a/test/lib/workspaces/get-workspaces.js b/test/lib/workspaces/get-workspaces.js index 4ea055e02..0f51d95fc 100644 --- a/test/lib/workspaces/get-workspaces.js +++ b/test/lib/workspaces/get-workspaces.js @@ -86,6 +86,17 @@ t.test('get-workspaces', async t => { 'should filter by package name' ) + workspaces = await getWorkspaces(['a', 'b'], { path, includeWorkspaceRoot: true }) + t.same( + clean(workspaces, path), + new Map(Object.entries({ + x: '{PATH}', + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + })), + 'include rootspace root' + ) + workspaces = await getWorkspaces(['./packages/c'], { path }) t.same( clean(workspaces, path), -- cgit v1.2.3