fix(ls): do not exit with error when all problems are extraneous deps

PR-URL: #3086 Credit: @nlf Close: #3086 Reviewed-by: @wraithgar, @ruyadorno
npm · Apr 22, 2021 · 42ca59e · 42ca59e
1 parent a4e7f4e
commit 42ca59e
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 44 deletions.
diff --git 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
@@ -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: [email protected] {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/bar
-extraneous: [email protected] {CWD}/tap-testdir-ls-ls---parseable-missing-package.json/node_modules/foo
-extraneous: [email protected] {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 @@ [email protected] {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: [email protected] {CWD}/tap-testdir-ls-ls-missing-package.json/node_modules/bar
-extraneous: [email protected] {CWD}/tap-testdir-ls-ls-missing-package.json/node_modules/foo
-extraneous: [email protected] {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
 +-- [email protected] extraneous

diff --git 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: [email protected] {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: [email protected] {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: [email protected] {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),
         {