fix: print quick audit report for human output

This was broken when the support/funding functionality changed the
return value to no longer track the promise for the quick audit
printing.

It was not caught by tests, because they were only running against the
--json output, and not verifying the quick audit results in any way.

Added a test to track the --json quick audit results (which were not
broken, but someday could become so) and the human printed quick audit
results (which were broken).

Paired with @ruyadorno @mikemimik

PR-URL: #486
Credit: @isaacs
Close: #486
Reviewed-by: @claudiahdz

Author: isaacs

lib/install.js

@@ -878,9 +878,6 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) {
   report += ' in ' + ((Date.now() - this.started) / 1000) + 's'
 
   output(report)
-  if (auditResult) {
-    audit.printInstallReport(auditResult)
-  }
 
   function packages (num) {
     return num + ' package' + (num > 1 ? 's' : '')
@@ -911,6 +908,10 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) {
   if (printFundingReport.length) {
     output(printFundingReport)
   }
+
+  if (auditResult) {
+    return audit.printInstallReport(auditResult)
+  }
 }
 
 Installer.prototype.printInstalledForJSON = function (diffs, auditResult) {

lib/install/fund.js

@@ -39,8 +39,8 @@ function getPrintFundingReport ({ fund, idealTree }, opts) {
 
   return padding('') + length + ' ' +
     packageQuantity(length) +
-    ' looking for funding.' +
-    padding('Run "npm fund" to find out more.')
+    ' looking for funding' +
+    padding('  run `npm fund` for details\n')
 }
 
 function getPrintFundingReportJSON ({ fund, idealTree }) {

test/tap/audit.js

@@ -27,6 +27,66 @@ function tmock (t) {
   })
 }
 
+const quickAuditResult = {
+  actions: [],
+  advisories: {
+    '1316': {
+      findings: [
+        {
+          version: '1.0.0',
+          paths: [
+            'baddep'
+          ]
+        }
+      ],
+      'id': 1316,
+      'created': '2019-11-14T15:29:41.991Z',
+      'updated': '2019-11-14T19:35:30.677Z',
+      'deleted': null,
+      'title': 'Arbitrary Code Execution',
+      'found_by': {
+        'link': '',
+        'name': 'François Lajeunesse-Robert',
+        'email': ''
+      },
+      'reported_by': {
+        'link': '',
+        'name': 'François Lajeunesse-Robert',
+        'email': ''
+      },
+      'module_name': 'baddep',
+      'cves': [],
+      'vulnerable_versions': '<4.5.2',
+      'patched_versions': '>=4.5.2',
+      'overview': 'a nice overview of the advisory',
+      'recommendation': 'how you should fix it',
+      'references': '',
+      'access': 'public',
+      'severity': 'high',
+      'cwe': 'CWE-79',
+      'metadata': {
+        'module_type': '',
+        'exploitability': 6,
+        'affected_components': ''
+      },
+      'url': 'https://npmjs.com/advisories/1234542069'
+    }
+  },
+  'muted': [],
+  'metadata': {
+    'vulnerabilities': {
+      'info': 0,
+      'low': 0,
+      'moderate': 0,
+      'high': 1,
+      'critical': 0
+    },
+    'dependencies': 1,
+    'devDependencies': 0,
+    'totalDependencies': 1
+  }
+}
+
 test('exits with zero exit code for vulnerabilities below the `audit-level` flag', t => {
   const fixture = new Tacks(new Dir({
     'package.json': new File({
@@ -40,7 +100,7 @@ test('exits with zero exit code for vulnerabilities below the `audit-level` flag
   fixture.create(testDir)
   return tmock(t).then(srv => {
     srv.filteringRequestBody(req => 'ok')
-    srv.post('/-/npm/v1/security/audits/quick', 'ok').reply(200, 'yeah')
+    srv.post('/-/npm/v1/security/audits/quick', 'ok').reply(200, quickAuditResult)
     srv.get('/baddep').twice().reply(200, {
       name: 'baddep',
       'dist-tags': {
@@ -75,6 +135,8 @@ test('exits with zero exit code for vulnerabilities below the `audit-level` flag
       '--registry', common.registry,
       '--cache', path.join(testDir, 'npm-cache')
     ], EXEC_OPTS).then(([code, stdout, stderr]) => {
+      const result = JSON.parse(stdout)
+      t.same(result.audit, quickAuditResult, 'printed quick audit result')
       srv.filteringRequestBody(req => 'ok')
       srv.post('/-/npm/v1/security/audits', 'ok').reply(200, {
         actions: [{
@@ -102,6 +164,62 @@ test('exits with zero exit code for vulnerabilities below the `audit-level` flag
   })
 })
 
+test('shows quick audit results summary for human', t => {
+  const fixture = new Tacks(new Dir({
+    'package.json': new File({
+      name: 'foo',
+      version: '1.0.0',
+      dependencies: {
+        baddep: '1.0.0'
+      }
+    })
+  }))
+  fixture.create(testDir)
+  return tmock(t).then(srv => {
+    srv.filteringRequestBody(req => 'ok')
+    srv.post('/-/npm/v1/security/audits/quick', 'ok').reply(200, quickAuditResult)
+    srv.get('/baddep').twice().reply(200, {
+      name: 'baddep',
+      'dist-tags': {
+        'latest': '1.2.3'
+      },
+      versions: {
+        '1.0.0': {
+          name: 'baddep',
+          version: '1.0.0',
+          _hasShrinkwrap: false,
+          dist: {
+            shasum: 'deadbeef',
+            tarball: common.registry + '/idk/-/idk-1.0.0.tgz'
+          }
+        },
+        '1.2.3': {
+          name: 'baddep',
+          version: '1.2.3',
+          _hasShrinkwrap: false,
+          dist: {
+            shasum: 'deadbeef',
+            tarball: common.registry + '/idk/-/idk-1.2.3.tgz'
+          }
+        }
+      }
+    })
+    return common.npm([
+      'install',
+      '--audit',
+      '--no-json',
+      '--package-lock-only',
+      '--registry', common.registry,
+      '--cache', path.join(testDir, 'npm-cache')
+    ], EXEC_OPTS).then(([code, stdout, stderr]) => {
+      t.match(stdout, new RegExp('added 1 package and audited 1 package in .*\\n' +
+        'found 1 high severity vulnerability\\n' +
+        '  run `npm audit fix` to fix them, or `npm audit` for details\\n'),
+      'shows quick audit result')
+    })
+  })
+})
+
 test('exits with non-zero exit code for vulnerabilities at the `audit-level` flag', t => {
   const fixture = new Tacks(new Dir({
     'package.json': new File({

test/tap/install-mention-funding.js

@@ -68,8 +68,8 @@ test('mention npm fund upon installing single dependency', function (t) {
     if (err) throw err
     t.is(code, 0, 'installed successfully')
     t.is(stderr, '', 'no warnings')
-    t.includes(stdout, '1 package is looking for funding.', 'should print amount of packages needing funding')
-    t.includes(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention')
+    t.includes(stdout, '1 package is looking for funding', 'should print amount of packages needing funding')
+    t.includes(stdout, '  run `npm fund` for details', 'should print npm fund mention')
     t.end()
   })
 })
@@ -80,8 +80,8 @@ test('mention npm fund upon installing multiple dependencies', function (t) {
     if (err) throw err
     t.is(code, 0, 'installed successfully')
     t.is(stderr, '', 'no warnings')
-    t.includes(stdout, '4 packages are looking for funding.', 'should print amount of packages needing funding')
-    t.includes(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention')
+    t.includes(stdout, '4 packages are looking for funding', 'should print amount of packages needing funding')
+    t.includes(stdout, '  run `npm fund` for details', 'should print npm fund mention')
     t.end()
   })
 })
@@ -92,8 +92,8 @@ test('skips mention npm fund using --no-fund option', function (t) {
     if (err) throw err
     t.is(code, 0, 'installed successfully')
     t.is(stderr, '', 'no warnings')
-    t.doesNotHave(stdout, '4 packages are looking for funding.', 'should print amount of packages needing funding')
-    t.doesNotHave(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention')
+    t.doesNotHave(stdout, '4 packages are looking for funding', 'should print amount of packages needing funding')
+    t.doesNotHave(stdout, '  run `npm fund` for details', 'should print npm fund mention')
     t.end()
   })
 })
@@ -105,7 +105,7 @@ test('mention packages looking for funding using --json', function (t) {
     t.is(code, 0, 'installed successfully')
     t.is(stderr, '', 'no warnings')
     const res = JSON.parse(stdout)
-    t.match(res.funding, '4 packages are looking for funding.', 'should print amount of packages needing funding')
+    t.match(res.funding, '4 packages are looking for funding', 'should print amount of packages needing funding')
     t.end()
   })
 })

test/tap/install.fund.js

@@ -1,16 +1,15 @@
 'use strict'
 
-const { EOL } = require('os')
 const { test } = require('tap')
 const { getPrintFundingReport } = require('../../lib/install/fund')
 
 test('message when there are no funding found', (t) => {
-  t.deepEqual(
+  t.equal(
     getPrintFundingReport({}),
     '',
     'should not print any message if missing info'
   )
-  t.deepEqual(
+  t.equal(
     getPrintFundingReport({
       name: 'foo',
       version: '1.0.0',
@@ -19,7 +18,7 @@ test('message when there are no funding found', (t) => {
     '',
     'should not print any message if package has no dependencies'
   )
-  t.deepEqual(
+  t.equal(
     getPrintFundingReport({
       fund: true,
       idealTree: {
@@ -38,7 +37,7 @@ test('message when there are no funding found', (t) => {
 })
 
 test('print appropriate message for a single package', (t) => {
-  t.deepEqual(
+  t.equal(
     getPrintFundingReport({
       fund: true,
       idealTree: {
@@ -54,15 +53,15 @@ test('print appropriate message for a single package', (t) => {
           }
         ]
       }
-    }),
-    `${EOL}1 package is looking for funding.${EOL}Run "npm fund" to find out more.`,
+    }).replace(/[\r\n]+/g, '\n'),
+    `\n1 package is looking for funding\n  run \`npm fund\` for details\n`,
     'should print single package message'
   )
   t.end()
 })
 
 test('print appropriate message for many packages', (t) => {
-  t.deepEqual(
+  t.equal(
     getPrintFundingReport({
       fund: true,
       idealTree: {
@@ -92,8 +91,8 @@ test('print appropriate message for many packages', (t) => {
           }
         ]
       }
-    }),
-    `${EOL}3 packages are looking for funding.${EOL}Run "npm fund" to find out more.`,
+    }).replace(/[\r\n]+/g, '\n'),
+    `\n3 packages are looking for funding\n  run \`npm fund\` for details\n`,
     'should print many package message'
   )
   t.end()