From 42ca59eeedd3e402aa1c606941f7f52864e6039b Mon Sep 17 00:00:00 2001 From: nlf Date: Thu, 15 Apr 2021 10:35:51 -0700 Subject: [PATCH] fix(ls): do not exit with error when all problems are extraneous deps PR-URL: https://github.com/npm/cli/pull/3086 Credit: @nlf Close: #3086 Reviewed-by: @wraithgar, @ruyadorno --- lib/ls.js | 5 +++- tap-snapshots/test/lib/ls.js.test.cjs | 12 --------- test/lib/ls.js | 39 ++++++--------------------- 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/lib/ls.js b/lib/ls.js index 65b3ddfe7611b..56d6579459598 100644 --- a/lib/ls.js +++ b/lib/ls.js @@ -166,7 +166,10 @@ class LS extends BaseCommand { ) } - if (problems.size) { + const shouldThrow = problems.size && + ![...problems].every(problem => problem.startsWith('extraneous:')) + + if (shouldThrow) { throw Object.assign( new Error([...problems].join(EOL)), { code: 'ELSPROBLEMS' } diff --git a/tap-snapshots/test/lib/ls.js.test.cjs b/tap-snapshots/test/lib/ls.js.test.cjs index c210ce928b7b3..77226971e3310 100644 --- a/tap-snapshots/test/lib/ls.js.test.cjs +++ b/tap-snapshots/test/lib/ls.js.test.cjs @@ -226,12 +226,6 @@ exports[`test/lib/ls.js TAP ls --parseable json read problems > should print emp {CWD}/tap-testdir-ls-ls---parseable-json-read-problems ` -exports[`test/lib/ls.js TAP ls --parseable missing package.json > should log all extraneous deps on error msg 1`] = ` -extraneous: bar@1.0.0 {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/bar -extraneous: foo@1.0.0 {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/foo -extraneous: lorem@1.0.0 {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/lorem -` - exports[`test/lib/ls.js TAP ls --parseable missing package.json > should output parseable missing name/version of top-level package 1`] = ` {CWD}/tap-testdir-ls-ls---parseable-missing-package.json {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/bar @@ -458,12 +452,6 @@ filter-by-child-of-missing-dep@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-cont ` -exports[`test/lib/ls.js TAP ls missing package.json > should log all extraneous deps on error msg 1`] = ` -extraneous: bar@1.0.0 {CWD}/tap-testdir-ls-ls-missing-package.json/node_modules/bar -extraneous: foo@1.0.0 {CWD}/tap-testdir-ls-ls-missing-package.json/node_modules/foo -extraneous: lorem@1.0.0 {CWD}/tap-testdir-ls-ls-missing-package.json/node_modules/lorem -` - exports[`test/lib/ls.js TAP ls missing package.json > should output tree missing name/version of top-level package 1`] = ` {CWD}/tap-testdir-ls-ls-missing-package.json +-- bar@1.0.0 extraneous diff --git a/test/lib/ls.js b/test/lib/ls.js index 6eab0b05b8da9..5b13f808d688f 100644 --- a/test/lib/ls.js +++ b/test/lib/ls.js @@ -149,11 +149,7 @@ t.test('ls', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.match(err.code, 'ELSPROBLEMS', 'should have ELSPROBLEMS error code') - t.matchSnapshot( - redactCwd(err.message), - 'should log all extraneous deps on error msg' - ) + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should output tree missing name/version of top-level package') t.end() }) @@ -171,12 +167,7 @@ t.test('ls', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.equal(err.code, 'ELSPROBLEMS', 'should have error code') - t.equal( - redactCwd(err.message), - 'extraneous: lorem@1.0.0 {CWD}/tap-testdir-ls-ls-extraneous-deps/node_modules/lorem', - 'should log extraneous dep as error' - ) + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should output containing problems info') t.end() }) @@ -1410,7 +1401,7 @@ t.test('ls', (t) => { }) ls.exec(['c'], (err) => { - t.match(err.code, 'ELSPROBLEMS', 'should have ELSPROBLEMS error code') + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should print tree and not duplicate child of missing items') t.end() }) @@ -1570,11 +1561,7 @@ t.test('ls --parseable', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.match(err.code, 'ELSPROBLEMS', 'should have ELSPROBLEMS error code') - t.matchSnapshot( - redactCwd(err.message), - 'should log all extraneous deps on error msg' - ) + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should output parseable missing name/version of top-level package') t.end() }) @@ -1592,7 +1579,7 @@ t.test('ls --parseable', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.equal(err.code, 'ELSPROBLEMS', 'should have error code') + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should output containing problems info') t.end() }) @@ -1973,8 +1960,7 @@ t.test('ls --parseable', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.equal(err.code, 'ELSPROBLEMS', 'should have error code') - t.match(redactCwd(err.message), 'extraneous: lorem@1.0.0 {CWD}/tap-testdir-ls-ls---parseable---long-with-extraneous-deps/node_modules/lorem', 'should have error code') + t.error(err) // should not error for extraneous t.matchSnapshot(redactCwd(result), 'should output long parseable output with extraneous info') t.end() }) @@ -2414,7 +2400,7 @@ t.test('ls --json', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.match(err, { code: 'ELSPROBLEMS' }, 'should list dep problems') + t.error(err) // should not error for extraneous t.same( jsonParse(result), { @@ -2470,16 +2456,7 @@ t.test('ls --json', (t) => { ...simpleNmFixture, }) ls.exec([], (err) => { - t.equal( - redactCwd(err.message), - 'extraneous: lorem@1.0.0 {CWD}/tap-testdir-ls-ls---json-extraneous-deps/node_modules/lorem', - 'should log extraneous dep as error' - ) - t.equal( - err.code, - 'ELSPROBLEMS', - 'should have ELSPROBLEMS error code' - ) + t.error(err) // should not error for extraneous t.same( jsonParse(result), {