feat: user external spinner in npm and monitor

src/cli/commands/monitor.ts

@@ -9,7 +9,7 @@ import * as config from '../../lib/config';
 import * as url from 'url';
 import chalk from 'chalk';
 import * as pathUtil from 'path';
-import * as spinner from 'cli-spinner';
+import * as spinner from '../../lib/spinner';
 
 import * as detect from '../../lib/detect';
 import * as plugins from '../../lib/plugins';

src/lib/plugins/nodejs-plugin/npm-modules-parser.ts

@@ -1,6 +1,6 @@
 import * as path from 'path';
 import * as snyk from '../../index';
-import * as spinner from '../../spinner';
+import { Spinner } from 'cli-spinner';
 import * as analytics from '../../analytics';
 import * as fs from 'then-fs';
 import { PkgTree } from 'snyk-nodejs-lockfile-parser';
@@ -33,8 +33,10 @@ export async function parse(
   const resolveModuleSpinnerLabel =
     'Analyzing npm dependencies for ' +
     path.dirname(path.resolve(root, targetFile));
+  const spinner = new Spinner(resolveModuleSpinnerLabel);
+  spinner.setSpinnerString('|/-\\');
   try {
-    await spinner(resolveModuleSpinnerLabel);
+    spinner.start();
     if (targetFile.endsWith('yarn.lock')) {
       options.file =
         options.file && options.file.replace('yarn.lock', 'package.json');
@@ -51,6 +53,6 @@ export async function parse(
       Object.assign({}, options, { noFromArrays: true }),
     );
   } finally {
-    await spinner.clear<void>(resolveModuleSpinnerLabel)();
+    await spinner.stop(true);
   }
 }

test/acceptance/cli.acceptance.test.ts

@@ -69,7 +69,6 @@ before('prime config', async (t) => {
 });
 
 test('test cli with multiple params: good and bad', async (t) => {
-  t.plan(6);
   try {
     await cli.test('/', 'semver', { registry: 'npm', org: 'EFF', json: true });
     t.fail('expect to err');
@@ -1428,7 +1427,6 @@ test('`test npm-out-of-sync --strict-out-of-sync=false` passes', async (t) => {
 });
 
 test('`test npm-package-shrinkwrap --file=package-lock.json ` with npm-shrinkwrap errors', async (t) => {
-  t.plan(1);
   chdirWorkspaces();
   try {
     await cli.test('npm-package-shrinkwrap', { file: 'package-lock.json' });
@@ -2855,8 +2853,6 @@ test('`test foo:latest --docker with binaries vulnerabilities`', async (t) => {
 });
 
 test('`test --policy-path`', async (tt) => {
-  tt.plan(3);
-
   tt.test('default policy', async (t) => {
     chdirWorkspaces('npm-package-policy');
     const expected = fs.readFileSync(path.join('.snyk'), 'utf8');
@@ -3009,7 +3005,6 @@ test('`test sbt-simple-struts`', async (t) => {
  * `monitor`
  */
 test('`monitor --policy-path`', async (tt) => {
-  tt.plan(2);
   chdirWorkspaces('npm-package-policy');
 
   tt.test('default policy', async (t) => {
@@ -3061,15 +3056,13 @@ test('`monitor non-existing`', async (t) => {
 });
 
 test('monitor for package with no name', async (t) => {
-  t.plan(1);
   await cli.monitor({
     file: __dirname + '/../fixtures/package-sans-name/package.json',
   });
   t.pass('succeed');
 });
 
 test('monitor for package with no name in lockfile', async (t) => {
-  t.plan(1);
   await cli.monitor({
     file:
       __dirname + '/../fixtures/package-sans-name-lockfile/package-lock.json',
@@ -3467,7 +3460,6 @@ test('`monitor gradle-app`', async (t) => {
 });
 
 test('`monitor gradle-app --all-sub-projects`', async (t) => {
-  t.plan(5);
   chdirWorkspaces();
   const plugin = {
     async inspect() {
@@ -3508,7 +3500,6 @@ test('`monitor gradle-app --all-sub-projects`', async (t) => {
 });
 
 test('`monitor gradle-app pip-app --all-sub-projects`', async (t) => {
-  t.plan(9);
   chdirWorkspaces();
   const plugin = {
     async inspect() {
@@ -3571,7 +3562,6 @@ test('`monitor gradle-app pip-app --all-sub-projects`', async (t) => {
 });
 
 test('`monitor gradle-app --all-sub-projects --project-name`', async (t) => {
-  t.plan(2);
   chdirWorkspaces();
   const plugin = {
     async inspect() {
@@ -4103,7 +4093,6 @@ test('`protect` for unsupported package managers', async (t) => {
 });
 
 test('`protect --policy-path`', async (tt) => {
-  tt.plan(2);
   chdirWorkspaces('npm-package-policy');
 
   tt.test('default policy', async (t) => {
@@ -4140,7 +4129,6 @@ test('`protect --policy-path`', async (tt) => {
 });
 
 test('`protect` with no policy', async (t) => {
-  t.plan(1);
   chdirWorkspaces('npm-with-dep-missing-policy');
 
   const vulns = require('./fixtures/npm-package-policy/vulns.json');
@@ -4158,7 +4146,6 @@ test('`protect` with no policy', async (t) => {
 });
 
 test('`test --insecure`', async (tt) => {
-  tt.plan(2);
   chdirWorkspaces('npm-package');
 
   tt.test('default (insecure false)', async (t) => {

test/acceptance/protect.test.ts

@@ -6,8 +6,6 @@ import { sep } from 'path';
 const main = './dist/cli/index.js'.replace(/\//g, sep);
 
 test('`protect` should not fail for unauthorized users', (t) => {
-  t.plan(1);
-
   const apiUserConfig = userConfig.get('api');
   // temporally remove api param in userConfig to test for unauthenticated users
   userConfig.delete('api');
@@ -16,13 +14,17 @@ test('`protect` should not fail for unauthorized users', (t) => {
     if (err) {
       throw err;
     }
-    t.equal(
+
+    t.match(
       stdout.trim(),
       'Successfully applied Snyk patches',
       'correct output for unauthenticated user',
     );
 
+    t.notOk(stderr, 'no errors present');
+
     // Restore api param
     userConfig.set('api', apiUserConfig);
+    t.end();
   });
 });

test/missing-node-modules.test.ts

@@ -41,7 +41,6 @@ test('setup', (t) => {
 });
 
 test('throws when missing node_modules', async (t) => {
-  t.plan(1);
   const dir = baseDir + 'npm/npm-3-no-node-modules';
   // ensure node_modules does not exist
   try {
@@ -54,13 +53,17 @@ test('throws when missing node_modules', async (t) => {
     await cli.test(dir);
     t.fail('should have thrown');
   } catch (e) {
-    t.matches(e.message, /Missing node_modules folder/);
+    t.equal(e.code, 500, 'correct error code');
+    t.equal(
+      e.userMessage,
+      "Missing node_modules folder: we can't test without dependencies.\n" +
+        "Please run 'npm install' first.",
+      'correct error message',
+    );
   }
 });
 
 test('teardown', (t) => {
-  t.plan(4);
-
   delete process.env.SNYK_API;
   delete process.env.SNYK_HOST;
   delete process.env.SNYK_PORT;

test/package-no-name.test.ts

@@ -1,4 +1,4 @@
-import * as test from 'tape';
+import { test } from 'tap';
 import * as snyk from '../src/lib';
 
 test('packages with no name read dir', async (t) => {

test/system/remote-package.test.ts

@@ -52,33 +52,40 @@ before('prime config', async (t) => {
   }
 });
 
-test('cli tests for online repos', async (t) => {
+test('cli tests for online repo semver@2', async (t) => {
   try {
     const res = await cli.test('semver@2');
     t.fail(res);
   } catch (error) {
     const res = error.message;
     const pos = res.toLowerCase().indexOf('vulnerability found');
-    t.pass(res);
-    t.notEqual(pos, -1, 'correctly found vulnerability: ' + res);
+    t.true(res, 'have res');
+    t.notEqual(pos, -1, 'correctly found a vulnerability in semver@2');
   }
+});
 
+test('cli tests for online repo semver@2 with --json', async (t) => {
   try {
     const res = await cli.test('semver@2', { json: true });
     t.fail(res);
   } catch (error) {
     const res = JSON.parse(error.message);
     const vuln = res.vulnerabilities[0];
-    t.pass(vuln.title);
+    t.equal(
+      vuln.title,
+      'semver Regular Expression Denial of Service',
+      'vuln title present',
+    );
+    t.equal(res.packageManager, 'npm', 'correct package manager');
     t.equal(
       vuln.id,
       'npm:semver:20150403',
-      'correctly found vulnerability: ' + vuln.id,
+      'correctly found a vulnerability in semver@2 with --json',
     );
   }
 });
 
-test('multiple test arguments', async (t) => {
+test('multiple test arguments semver@4, qs@6', async (t) => {
   try {
     const res = await cli.test('semver@4', 'qs@6');
     const lastLine = res
@@ -93,7 +100,9 @@ test('multiple test arguments', async (t) => {
   } catch (error) {
     t.fail(error);
   }
+});
 
+test('multiple test arguments semver@4, qs@1', async (t) => {
   try {
     const res = await cli.test('semver@4', 'qs@1');
     t.fail(res);
@@ -109,38 +118,6 @@ test('multiple test arguments', async (t) => {
       'successfully tested semver@4, qs@1',
     );
   }
-
-  try {
-    const res = await cli.test('semver@2', 'qs@6');
-    t.fail(res);
-  } catch (error) {
-    const res = error.message;
-    const lastLine = res
-      .trim()
-      .split('\n')
-      .pop();
-    t.equals(
-      lastLine,
-      'Tested 2 projects, 1 contained vulnerable paths.',
-      'successfully tested semver@2, qs@6',
-    );
-  }
-
-  try {
-    const res = await cli.test('semver@2', 'qs@1');
-    t.fail(res);
-  } catch (error) {
-    const res = error.message;
-    const lastLine = res
-      .trim()
-      .split('\n')
-      .pop();
-    t.equals(
-      lastLine,
-      'Tested 2 projects, 2 contained vulnerable paths.',
-      'successfully tested semver@2, qs@1',
-    );
-  }
 });
 
 test('test for existing remote package with dev-deps only with --dev', async (t) => {