Merge pull request snyk#779 from snyk/fix/remove-stringly-typed-boole…

…an-option-values fix: correctly document behavior of `--strict-out-of-sync` and avoid string typed booleans
Mattlk13 · Sep 30, 2019 · 825666d · 825666d
2 parents 3583a8c + 1efdee3
commit 825666d
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 8 deletions.
diff --git a/help/help.txt b/help/help.txt
@@ -89,11 +89,11 @@ Gradle options:
 
 Npm options:
   --strict-out-of-sync=<true|false>
-                       Allow testing out of sync lockfile. Defauls to false.
+                       Allow testing out of sync lockfile. Defauls to true.
 
 Yarn options:
   --strict-out-of-sync=<true|false>
-                       Allow testing out of sync lockfile. Defauls to false.
+                       Allow testing out of sync lockfile. Defauls to true.
 
 Python options:
   --command=<string>   Python interpreter to use. Default: "python", set to

diff --git a/src/cli/args.ts b/src/cli/args.ts
@@ -171,6 +171,14 @@ export function args(rawArgv: string[]): Args {
     }
   }
 
+  if (argv.strictOutOfSync !== undefined) {
+    if (argv.strictOutOfSync === 'false') {
+      argv.strictOutOfSync = false;
+    } else {
+      argv.strictOutOfSync = true;
+    }
+  }
+
   // Alias
   if (argv.gradleSubProject) {
     argv.subProject = argv.gradleSubProject;

diff --git a/src/lib/plugins/nodejs-plugin/npm-lock-parser.ts b/src/lib/plugins/nodejs-plugin/npm-lock-parser.ts
@@ -2,7 +2,6 @@ import * as baseDebug from 'debug';
 const debug = baseDebug('snyk');
 import * as path from 'path';
 import * as spinner from '../../spinner';
-import * as _ from 'lodash';
 import * as analytics from '../../analytics';
 import * as fs from 'fs';
 import * as lockFileParser from 'snyk-nodejs-lockfile-parser';
@@ -46,7 +45,7 @@ export async function parse(root: string, targetFile: string, options: Options):
   debug(resolveModuleSpinnerLabel);
   try {
     await spinner(resolveModuleSpinnerLabel);
-    const strictOutOfSync = _.get(options, 'strictOutOfSync') !== 'false';
+    const strictOutOfSync = options.strictOutOfSync !== false;
     return lockFileParser
       .buildDepTree(manifestFile, lockFile, options.dev, lockFileType, strictOutOfSync);
   } finally {

diff --git a/src/lib/plugins/types.ts b/src/lib/plugins/types.ts
@@ -12,7 +12,7 @@ export interface Options {
   docker?: boolean;
   traverseNodeModules?: boolean;
   dev?: boolean;
-  strictOutOfSync?: boolean | 'true' | 'false';
+  strictOutOfSync?: boolean;
   allSubProjects?: boolean;
   debug?: boolean;
   packageManager?: string;

diff --git a/test/acceptance/cli.acceptance.test.ts b/test/acceptance/cli.acceptance.test.ts
@@ -1063,7 +1063,7 @@ test('`test yarn-out-of-sync` out of sync fails', async (t) => {
 
 test('`test yarn-out-of-sync --strict-out-of-sync=false` passes', async (t) => {
   chdirWorkspaces();
-  await cli.test('yarn-out-of-sync', { dev: true, strictOutOfSync: 'false'});
+  await cli.test('yarn-out-of-sync', { dev: true, strictOutOfSync: false});
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
   const depGraph = req.body.depGraph;
@@ -1103,7 +1103,7 @@ test('`test yarn-out-of-sync --strict-out-of-sync=false` passes', async (t) => {
 
 test('`test npm-out-of-sync --strict-out-of-sync=false` passes', async (t) => {
   chdirWorkspaces();
-  await cli.test('npm-out-of-sync', { dev: true, strictOutOfSync: 'false' });
+  await cli.test('npm-out-of-sync', { dev: true, strictOutOfSync: false });
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
   const depGraph = req.body.depGraph;
@@ -2481,7 +2481,7 @@ test('`monitor npm-package`', async (t) => {
 
 test('`monitor npm-out-of-sync`', async (t) => {
   chdirWorkspaces();
-  await cli.monitor('npm-out-of-sync-graph', { 'experimental-dep-graph': true, strictOutOfSync: 'false' });
+  await cli.monitor('npm-out-of-sync-graph', { 'experimental-dep-graph': true, strictOutOfSync: false });
   const req = server.popRequest();
   t.match(req.url, '/monitor/npm/graph', 'puts at correct url');
   t.ok(req.body.depGraphJSON, 'sends depGraphJSON');

diff --git a/test/args.test.js b/test/args.test.js
@@ -78,3 +78,39 @@ test('test command line test --gradle-sub-project=foo', function(t) {
   t.equal(result.options.subProject, 'foo');
   t.end();
 });
+
+test('test command line test --strict-out-of-sync', function(t) {
+  t.plan(1);
+  var cliArgs = [ '/Users/dror/.nvm/versions/node/v6.9.2/bin/node',
+    '/Users/dror/work/snyk/snyk-internal/cli',
+    'test',
+    '--strict-out-of-sync',
+  ];
+  var result = args(cliArgs);
+  t.equal(result.options.strictOutOfSync, true);
+  t.end();
+});
+
+test('test command line test --strict-out-of-sync=true', function(t) {
+  t.plan(1);
+  var cliArgs = [ '/Users/dror/.nvm/versions/node/v6.9.2/bin/node',
+    '/Users/dror/work/snyk/snyk-internal/cli',
+    'test',
+    '--strict-out-of-sync=true',
+  ];
+  var result = args(cliArgs);
+  t.equal(result.options.strictOutOfSync, true);
+  t.end();
+});
+
+test('test command line test --strict-out-of-sync=false', function(t) {
+  t.plan(1);
+  var cliArgs = [ '/Users/dror/.nvm/versions/node/v6.9.2/bin/node',
+    '/Users/dror/work/snyk/snyk-internal/cli',
+    'test',
+    '--strict-out-of-sync=false',
+  ];
+  var result = args(cliArgs);
+  t.equal(result.options.strictOutOfSync, false);
+  t.end();
+});