Merge pull request #1370 from snyk/fix/test-dependencies-scan-result-…

…type

fix: test dependencies scan result type

src/lib/ecosystems.ts

@@ -1,27 +1,26 @@
 import * as cppPlugin from 'snyk-cpp-plugin';
-import { Options } from './types';
-import { TestCommandResult } from '../cli/commands/types';
+import { DepGraphData } from '@snyk/dep-graph';
+import * as snyk from './index';
 import * as config from './config';
 import { isCI } from './is-ci';
-import * as snyk from './';
-import request = require('./request');
-import { DepGraphData } from '@snyk/dep-graph';
-
-interface Artifact {
+import { makeRequest } from './request/promise';
+import { Options } from './types';
+import { TestCommandResult } from '../cli/commands/types';
+export interface Artifact {
   type: string;
   data: any;
   meta: { [key: string]: any };
 }
 
-interface ScanResult {
+export interface ScanResult {
   type: string;
   artifacts: Artifact[];
   meta: {
     [key: string]: any;
   };
 }
 
-interface TestResults {
+export interface TestResult {
   depGraph: DepGraphData;
   affectedPkgs: {
     [pkgId: string]: {
@@ -49,7 +48,7 @@ export interface EcosystemPlugin {
   scan: (options: Options) => Promise<ScanResult[]>;
   display: (
     scanResults: ScanResult[],
-    testResults: TestResults[],
+    testResults: TestResult[],
     errors: string[],
   ) => Promise<string>;
 }
@@ -80,19 +79,19 @@ export async function testEcosystem(
 ): Promise<TestCommandResult> {
   const plugin = getPlugin(ecosystem);
   const scanResultsByPath: { [dir: string]: ScanResult[] } = {};
-  let scanResults: ScanResult[] = [];
   for (const path of paths) {
     options.path = path;
     const results = await plugin.scan(options);
     scanResultsByPath[path] = results;
-    scanResults = scanResults.concat(results);
   }
 
   const [testResults, errors] = await testDependencies(scanResultsByPath);
   const stringifiedData = JSON.stringify(testResults, null, 2);
   if (options.json) {
     return TestCommandResult.createJsonTestCommandResult(stringifiedData);
   }
+  const emptyResults: ScanResult[] = [];
+  const scanResults = emptyResults.concat(...Object.values(scanResultsByPath));
   const readableResult = await plugin.display(scanResults, testResults, errors);
 
   return TestCommandResult.createHumanReadableTestCommandResult(
@@ -103,8 +102,8 @@ export async function testEcosystem(
 
 export async function testDependencies(scans: {
   [dir: string]: ScanResult[];
-}): Promise<[TestResults[], string[]]> {
-  const results: TestResults[] = [];
+}): Promise<[TestResult[], string[]]> {
+  const results: TestResult[] = [];
   const errors: string[] = [];
   for (const [path, scanResults] of Object.entries(scans)) {
     for (const scanResult of scanResults) {
@@ -117,16 +116,16 @@ export async function testDependencies(scans: {
           authorization: 'token ' + snyk.api,
         },
         body: {
-          type: 'cpp',
+          type: scanResult.type,
           artifacts: scanResult.artifacts,
           meta: {},
         },
       };
       try {
-        const response = await makeRequest(payload);
+        const response = await makeRequest<TestResult>(payload);
         results.push(response);
       } catch (error) {
-        if (error.code !== 200) {
+        if (error.code >= 400 && error.code < 500) {
           throw new Error(error.message);
         }
         errors.push('Could not test dependencies in ' + path);
@@ -135,20 +134,3 @@ export async function testDependencies(scans: {
   }
   return [results, errors];
 }
-
-export async function makeRequest(payload: any): Promise<TestResults> {
-  return new Promise((resolve, reject) => {
-    request(payload, (error, res, body) => {
-      if (error) {
-        return reject(error);
-      }
-      if (res.statusCode !== 200) {
-        return reject({
-          code: res.statusCode,
-          message: res?.body?.message || 'Error testing dependencies',
-        });
-      }
-      resolve(body);
-    });
-  });
-}

src/lib/request/promise.ts

@@ -0,0 +1,18 @@
+import request = require('./index');
+
+export async function makeRequest<T>(payload: any): Promise<T> {
+  return new Promise((resolve, reject) => {
+    request(payload, (error, res, body) => {
+      if (error) {
+        return reject(error);
+      }
+      if (res.statusCode !== 200) {
+        return reject({
+          code: res.statusCode,
+          message: body?.message,
+        });
+      }
+      resolve(body);
+    });
+  });
+}

test/ecosystems.spec.ts

@@ -1,13 +1,14 @@
-import { Options } from '../src/lib/types';
-import * as cppPlugin from 'snyk-cpp-plugin';
-import * as path from 'path';
 import * as fs from 'fs';
+import * as path from 'path';
+import * as cppPlugin from 'snyk-cpp-plugin';
 import * as ecosystems from '../src/lib/ecosystems';
+import * as request from '../src/lib/request/promise';
+import { Options } from '../src/lib/types';
 import { TestCommandResult } from '../src/cli/commands/types';
 
 describe('ecosystems', () => {
   describe('getPlugin', () => {
-    it('should return c++ plugin when cpp ecosystem is given', () => {
+    it('should return cpp plugin when cpp ecosystem is given', () => {
       const actual = ecosystems.getPlugin('cpp');
       const expected = cppPlugin;
       expect(actual).toBe(expected);
@@ -21,7 +22,7 @@ describe('ecosystems', () => {
   });
 
   describe('getEcosystem', () => {
-    it('should return c++ ecosystem when options source is true', () => {
+    it('should return cpp ecosystem when options source is true', () => {
       const options: Options = {
         source: true,
         path: '',
@@ -30,91 +31,108 @@ describe('ecosystems', () => {
       const expected = 'cpp';
       expect(actual).toBe(expected);
     });
+    it('should return null when options source is false', () => {
+      const options: Options = {
+        source: false,
+        path: '',
+      };
+      const actual = ecosystems.getEcosystem(options);
+      const expected = null;
+      expect(actual).toBe(expected);
+    });
   });
 
-  it('should return null when options source is false', () => {
-    const options: Options = {
-      source: false,
-      path: '',
-    };
-    const actual = ecosystems.getEcosystem(options);
-    const expected = null;
-    expect(actual).toBe(expected);
-  });
-});
+  describe('testEcosystem', () => {
+    describe('cpp', () => {
+      const fixturePath = path.join(__dirname, 'fixtures', 'cpp-project');
+      const cwd = process.cwd();
 
-describe('testEcosystem', () => {
-  const fixturePath = path.join(__dirname, 'fixtures', 'cpp-project');
-  const cwd = process.cwd();
+      function readFixture(filename: string) {
+        const filePath = path.join(fixturePath, filename);
+        return fs.readFileSync(filePath, 'utf-8');
+      }
 
-  function readFixture(filename: string) {
-    const filePath = path.join(fixturePath, filename);
-    return fs.readFileSync(filePath, 'utf-8');
-  }
+      function readJsonFixture(filename: string) {
+        const contents = readFixture(filename);
+        return JSON.parse(contents);
+      }
 
-  beforeAll(() => {
-    process.chdir(fixturePath);
-  });
+      const displayTxt = readFixture('display.txt');
+      const errorTxt = readFixture('error.txt');
+      const testResult = readJsonFixture(
+        'testResults.json',
+      ) as ecosystems.TestResult;
+      const stringifyTestResults = JSON.stringify([testResult], null, 2);
 
-  beforeEach(() => {
-    jest.resetAllMocks();
-  });
+      beforeAll(() => {
+        process.chdir(fixturePath);
+      });
 
-  afterAll(() => {
-    process.chdir(cwd);
-  });
+      afterEach(() => {
+        jest.resetAllMocks();
+      });
 
-  it('should return human readable result when no json option given', async () => {
-    const display = readFixture('display.txt');
-    const testResults = readFixture('testResults.json');
-    const stringifiedData = JSON.stringify(JSON.parse(testResults), null, 2);
-    const expected = TestCommandResult.createHumanReadableTestCommandResult(
-      display,
-      stringifiedData,
-    );
+      afterAll(() => {
+        process.chdir(cwd);
+      });
 
-    const actual = await ecosystems.testEcosystem('cpp', ['.'], { path: '' });
-    expect(actual).toEqual(expected);
-  });
+      it('should return human readable result when no json option given', async () => {
+        const mock = jest
+          .spyOn(request, 'makeRequest')
+          .mockResolvedValue(testResult);
+        const expected = TestCommandResult.createHumanReadableTestCommandResult(
+          displayTxt,
+          stringifyTestResults,
+        );
+        const actual = await ecosystems.testEcosystem('cpp', ['.'], {
+          path: '',
+        });
+        expect(mock).toHaveBeenCalled();
+        expect(actual).toEqual(expected);
+      });
 
-  it('should return json result when json option', async () => {
-    const testResults = readFixture('testResults.json');
-    const stringifiedData = JSON.stringify(JSON.parse(testResults), null, 2);
-    const expected = TestCommandResult.createJsonTestCommandResult(
-      stringifiedData,
-    );
-    const actual = await ecosystems.testEcosystem('cpp', ['.'], {
-      path: '',
-      json: true,
-    });
-    expect(actual).toEqual(expected);
-  });
+      it('should return json result when json option', async () => {
+        const mock = jest
+          .spyOn(request, 'makeRequest')
+          .mockResolvedValue(testResult);
+        const expected = TestCommandResult.createJsonTestCommandResult(
+          stringifyTestResults,
+        );
+        const actual = await ecosystems.testEcosystem('cpp', ['.'], {
+          path: '',
+          json: true,
+        });
+        expect(mock).toHaveBeenCalled();
+        expect(actual).toEqual(expected);
+      });
 
-  it('should throw error when response code is not 200', async () => {
-    const expected = { code: 401, message: 'Invalid auth token' };
-    jest.spyOn(ecosystems, 'testEcosystem').mockRejectedValue(expected);
-    expect.assertions(1);
-    try {
-      await ecosystems.testEcosystem('cpp', ['.'], {
-        path: '',
+      it('should throw error when response code is not 200', async () => {
+        const error = { code: 401, message: 'Invalid auth token' };
+        jest.spyOn(request, 'makeRequest').mockRejectedValue(error);
+        const expected = new Error(error.message);
+        expect.assertions(1);
+        try {
+          await ecosystems.testEcosystem('cpp', ['.'], {
+            path: '',
+          });
+        } catch (error) {
+          expect(error).toEqual(expected);
+        }
       });
-    } catch (error) {
-      expect(error).toEqual(expected);
-    }
-  });
 
-  it.skip('should return error when there was a problem testing dependencies', async () => {
-    //@boost: TODO finish up my implementation
-    // const makeRequestSpy = jest
-    //   .spyOn(ecosystems, 'makeRequest')
-    //   .mockRejectedValue('Something went wrong');
-    // const ecosystemDisplaySpy = jest.spyOn(cppPlugin, 'display');
-    const commandResult = await ecosystems.testEcosystem('cpp', ['.'], {
-      path: '',
-      json: true,
+      it('should return error when there was a problem testing dependencies', async () => {
+        jest
+          .spyOn(request, 'makeRequest')
+          .mockRejectedValue('Something went wrong');
+        const expected = TestCommandResult.createHumanReadableTestCommandResult(
+          errorTxt,
+          '[]',
+        );
+        const actual = await ecosystems.testEcosystem('cpp', ['.'], {
+          path: '',
+        });
+        expect(actual).toEqual(expected);
+      });
     });
-    console.log(commandResult);
-    expect(commandResult).toEqual('');
-    // expect(ecosystemDisplaySpy).toHaveBeenCalledWith({});
   });
 });

test/fixtures/cpp-project/display.txt

@@ -4,6 +4,12 @@ Dependency Fingerprints
 aeca71a6e39f99a24ecf4c088eee9cb8 add.h
 ad3365b3370ef6b1c3e778f875055f19 main.cpp
 
+Dependencies
+------------
+add@1.2.3
+
 Issues
 ------
-Tested 0 dependencies for known issues, found 0 issues.
+Tested 1 dependency for known issues, found 1 issue.
+
+✗ Cross-site Scripting (XSS) [medium severity][https://snyk.io/vuln/cpp:add:20161130] in add@1.2.3

test/fixtures/cpp-project/error.txt

@@ -0,0 +1,10 @@
+Dependency Fingerprints
+-----------------------
+52d1b046047db9ea0c581cafd4c68fe5 add.cpp
+aeca71a6e39f99a24ecf4c088eee9cb8 add.h
+ad3365b3370ef6b1c3e778f875055f19 main.cpp
+
+
+Errors
+------
+Could not test dependencies in .