From c9cb52282813e88aaa11cf2b94cf1156414eda2e Mon Sep 17 00:00:00 2001 From: gfyoung Date: Sun, 26 Sep 2021 03:18:17 +0000 Subject: [PATCH 1/2] fix: restore exit code on "npm outdated" closes: https://github.com/npm/cli/issues/2556 xref: https://github.com/npm/cli/pull/1750 The xref'ed PR apparently dropped this behavior without any explanation. --- lib/outdated.js | 2 ++ test/lib/outdated.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lib/outdated.js b/lib/outdated.js index 01e268fe96aee..e583add014005 100644 --- a/lib/outdated.js +++ b/lib/outdated.js @@ -87,6 +87,8 @@ class Outdated extends ArboristWorkspaceCmd { // sorts list alphabetically const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name, 'en')) + process.exitCode = outdated.length ? 1 : 0 + // return if no outdated packages if (outdated.length === 0 && !this.npm.config.get('json')) return diff --git a/test/lib/outdated.js b/test/lib/outdated.js index 34a0aa6c9e03e..3c5c2b9c3dd63 100644 --- a/test/lib/outdated.js +++ b/test/lib/outdated.js @@ -102,6 +102,10 @@ const outdated = (dir, opts) => { t.beforeEach(() => logs = '') +const { exitCode } = process + +t.afterEach(() => process.exitCode = exitCode) + const redactCwd = (path) => { const normalizePath = p => p .replace(/\\+/g, '/') @@ -175,6 +179,7 @@ t.test('should display outdated deps', t => { outdated(null, { config: { global: true }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -187,6 +192,7 @@ t.test('should display outdated deps', t => { }, color: true, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -200,6 +206,7 @@ t.test('should display outdated deps', t => { }, color: true, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -213,6 +220,7 @@ t.test('should display outdated deps', t => { }, color: true, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -226,6 +234,7 @@ t.test('should display outdated deps', t => { }, color: true, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -238,6 +247,7 @@ t.test('should display outdated deps', t => { long: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -250,6 +260,7 @@ t.test('should display outdated deps', t => { json: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -263,6 +274,7 @@ t.test('should display outdated deps', t => { long: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -275,6 +287,7 @@ t.test('should display outdated deps', t => { parseable: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -288,6 +301,7 @@ t.test('should display outdated deps', t => { long: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -299,6 +313,7 @@ t.test('should display outdated deps', t => { all: true, }, }).exec([], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -310,6 +325,7 @@ t.test('should display outdated deps', t => { global: false, }, }).exec(['cat'], () => { + t.equal(process.exitCode, 1) t.matchSnapshot(logs) t.end() }) @@ -341,6 +357,7 @@ t.test('should return if no outdated deps', t => { global: false, }).exec([], () => { t.equal(logs.length, 0, 'no logs') + t.equal(process.exitCode, 0) t.end() }) }) @@ -388,6 +405,7 @@ t.test('should skip missing non-prod deps', t => { global: false, }).exec([], () => { t.equal(logs.length, 0, 'no logs') + t.equal(process.exitCode, 0) t.end() }) }) @@ -413,6 +431,7 @@ t.test('should skip invalid pkg ranges', t => { outdated(testDir, {}).exec([], () => { t.equal(logs.length, 0, 'no logs') + t.equal(process.exitCode, 0) t.end() }) }) @@ -438,6 +457,7 @@ t.test('should skip git specs', t => { outdated(testDir, {}).exec([], () => { t.equal(logs.length, 0, 'no logs') + t.equal(process.exitCode, 0) t.end() }) }) @@ -540,6 +560,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display ws outdated deps human output') + t.equal(process.exitCode, 1) res() }) }) @@ -554,6 +575,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display ws outdated deps json output') + t.equal(process.exitCode, 1) res() }) }) @@ -568,6 +590,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display ws outdated deps parseable output') + t.equal(process.exitCode, 1) res() }) }) @@ -582,6 +605,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display all dependencies') + t.equal(process.exitCode, 1) res() }) }) @@ -594,6 +618,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should highlight ws in dependend by section') + t.equal(process.exitCode, 1) res() }) }) @@ -604,6 +629,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display results filtered by ws') + t.equal(process.exitCode, 1) res() }) }) @@ -618,6 +644,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display json results filtered by ws') + t.equal(process.exitCode, 1) res() }) }) @@ -632,6 +659,7 @@ t.test('workspaces', async t => { rej(err) t.matchSnapshot(logs, 'should display parseable results filtered by ws') + t.equal(process.exitCode, 1) res() }) }) @@ -647,6 +675,7 @@ t.test('workspaces', async t => { t.matchSnapshot(logs, 'should display nested deps when filtering by ws and using --all') + t.equal(process.exitCode, 1) res() }) }) @@ -658,6 +687,7 @@ t.test('workspaces', async t => { t.matchSnapshot(logs, 'should display no results if ws has no deps to display') + t.equal(process.exitCode, 0) res() }) }) @@ -669,6 +699,7 @@ t.test('workspaces', async t => { t.matchSnapshot(logs, 'should display missing deps when filtering by ws') + t.equal(process.exitCode, 1) res() }) }) From 41cc10c3ef017f16326cbc292873d594a38019f2 Mon Sep 17 00:00:00 2001 From: gfyoung Date: Mon, 27 Sep 2021 16:22:46 +0000 Subject: [PATCH 2/2] Feedback updates * More compact exitCode check * Update smoke-test to check exit code --- lib/outdated.js | 3 ++- smoke-tests/index.js | 8 ++++++-- test/lib/outdated.js | 5 ----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/outdated.js b/lib/outdated.js index e583add014005..a5be13cdaf108 100644 --- a/lib/outdated.js +++ b/lib/outdated.js @@ -87,7 +87,8 @@ class Outdated extends ArboristWorkspaceCmd { // sorts list alphabetically const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name, 'en')) - process.exitCode = outdated.length ? 1 : 0 + if (outdated.length > 0) + process.exitCode = 1 // return if no outdated packages if (outdated.length === 0 && !this.npm.config.get('json')) diff --git a/smoke-tests/index.js b/smoke-tests/index.js index 5e2d5e071ae12..076c53e78a0b7 100644 --- a/smoke-tests/index.js +++ b/smoke-tests/index.js @@ -174,9 +174,13 @@ t.test('npm diff', async t => { t.test('npm outdated', async t => { const cmd = `${npmBin} outdated` - const cmdRes = await exec(cmd) + const cmdRes = await exec(cmd).catch(err => { + t.equal(err.code, 1, 'should exit with error code') + return err + }) - t.matchSnapshot(cmdRes, + t.not(cmdRes.stderr, '', 'should have stderr output') + t.matchSnapshot(String(cmdRes.stdout), 'should have expected outdated output') }) diff --git a/test/lib/outdated.js b/test/lib/outdated.js index 3c5c2b9c3dd63..518436d0af4ec 100644 --- a/test/lib/outdated.js +++ b/test/lib/outdated.js @@ -357,7 +357,6 @@ t.test('should return if no outdated deps', t => { global: false, }).exec([], () => { t.equal(logs.length, 0, 'no logs') - t.equal(process.exitCode, 0) t.end() }) }) @@ -405,7 +404,6 @@ t.test('should skip missing non-prod deps', t => { global: false, }).exec([], () => { t.equal(logs.length, 0, 'no logs') - t.equal(process.exitCode, 0) t.end() }) }) @@ -431,7 +429,6 @@ t.test('should skip invalid pkg ranges', t => { outdated(testDir, {}).exec([], () => { t.equal(logs.length, 0, 'no logs') - t.equal(process.exitCode, 0) t.end() }) }) @@ -457,7 +454,6 @@ t.test('should skip git specs', t => { outdated(testDir, {}).exec([], () => { t.equal(logs.length, 0, 'no logs') - t.equal(process.exitCode, 0) t.end() }) }) @@ -687,7 +683,6 @@ t.test('workspaces', async t => { t.matchSnapshot(logs, 'should display no results if ws has no deps to display') - t.equal(process.exitCode, 0) res() }) })