Merge pull request #46 from snyk/feat/update-tests-and-types

feat: update tests and correct type definition

Author: Konstantin Yegupov

README.md

@@ -49,21 +49,21 @@ export interface DepGraph {
   };
   readonly rootPkg: {
     name: string;
-    version: string | null;
+    version?: string;
   };
   // all unique packages in the graph (including root package)
   getPkgs(): Array<{
     name: string;
-    version: string | null;
+    version?: string;
   }>;
   // all unique packages in the graph, except the root package
   getDepPkgs(): Array<{
     name: string;
-    version: string | null;
+    version?: string;
   }>;
   pkgPathsToRoot(pkg: Pkg): Array<Array<{
     name: string;
-    version: string | null;
+    version?: string;
   }>>;
   countPathsToRoot(pkg: Pkg): number;
   toJSON(): DepGraphData;
@@ -89,7 +89,7 @@ export interface DepGraphData {
     id: string;
     info: {
       name: string;
-      version: string | null;
+      version?: string;
     };
   }>;
   graph: {

src/core/create-from-json.ts

@@ -22,8 +22,9 @@ export function createFromJSON(depGraphData: DepGraphData): DepGraph {
   const pkgNodes: {[pkgId: string]: Set<string>} = {};
 
   for (const { id, info } of depGraphData.pkgs) {
-    // TODO: avoid this, instead just use `info` as is
-    pkgs[id] = info.version ? info : { ...info, version: null } as any;
+    pkgs[id] = info.version
+      ? info
+      : { ...info, version: undefined };
   }
 
   for (const node of depGraphData.graph.nodes) {

src/core/dep-graph.ts

@@ -176,6 +176,10 @@ class DepGraphImpl implements types.DepGraphInternal {
       otherDepGraph = createFromJSON(other.toJSON()) as types.DepGraphInternal;
     }
 
+    // In theory, for the graphs created by standard means, `_.isEquals(this._data, otherDepGraph._data)`
+    // should suffice, since node IDs will be generated in a predictable way.
+    // However, there might be different versions of graph and inconsistencies
+    // in the ordering of the arrays, so we perform a deep comparison.
     return this.nodeEquals(this, this.rootNodeId, otherDepGraph, otherDepGraph.rootNodeId, compareRoot);
   }
 

src/legacy/index.ts

@@ -45,7 +45,7 @@ function addLabel(dep: DepTreeDep, key: string, value: string) {
 async function depTreeToGraph(depTree: DepTree, pkgManagerName: string): Promise<types.DepGraph> {
   const rootPkg = {
     name: depTree.name!,
-    version: depTree.version,
+    version: depTree.version || undefined,
   };
 
   const pkgManagerInfo: types.PkgManager = {

test/core/create-from-json.test.ts

@@ -17,26 +17,26 @@ describe('fromJSON simple', () => {
   });
 
   test('getPkgs()', () => {
-    expect(graph.getPkgs().sort(helpers.depSort)).toEqual([
+    helpers.expectSamePkgs(graph.getPkgs(), [
       { name: 'a', version: '1.0.0' },
       { name: 'b', version: '1.0.0' },
       { name: 'c', version: '1.0.0' },
       { name: 'd', version: '0.0.1' },
       { name: 'd', version: '0.0.2' },
       { name: 'e', version: '5.0.0' },
       { name: 'root', version: '0.0.0' },
-    ].sort(helpers.depSort));
+    ]);
   });
 
   test('getDepPkgs()', () => {
-    expect(graph.getDepPkgs().sort(helpers.depSort)).toEqual([
+    helpers.expectSamePkgs(graph.getDepPkgs(), [
       { name: 'a', version: '1.0.0' },
       { name: 'b', version: '1.0.0' },
       { name: 'c', version: '1.0.0' },
       { name: 'd', version: '0.0.1' },
       { name: 'd', version: '0.0.2' },
       { name: 'e', version: '5.0.0' },
-    ].sort(helpers.depSort));
+    ]);
   });
 
   test('getPathsToRoot', () => {
@@ -151,11 +151,11 @@ test('fromJSON a pkg and a node share same id', () => {
 
   const depGraph = depGraphLib.createFromJSON(graphJson);
 
-  expect(depGraph.getPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getPkgs(), [
     { name: 'toor', version: '1.0.0' },
     { name: 'foo', version: '2' },
   ].sort());
-  expect(depGraph.getDepPkgs()).toEqual([
+  helpers.expectSamePkgs(depGraph.getDepPkgs(), [
     { name: 'foo', version: '2' },
   ]);
 
@@ -363,13 +363,13 @@ test('fromJSON with a cycle', () => {
 
   const depGraph = depGraphLib.createFromJSON(graphJson);
 
-  expect(depGraph.getPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getPkgs(), [
     { name: 'toor', version: '1.0.0' },
     { name: 'foo', version: '2' },
     { name: 'bar', version: '3' },
     { name: 'baz', version: '4' },
   ]);
-  expect(depGraph.getDepPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getDepPkgs(), [
     { name: 'foo', version: '2' },
     { name: 'bar', version: '3' },
     { name: 'baz', version: '4' },
@@ -659,11 +659,11 @@ test('fromJSON root has several instances', () => {
   };
 
   const depGraph = depGraphLib.createFromJSON(graphJson);
-  expect(depGraph.getPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getPkgs(), [
     {name: 'toor', version: '1.0.0'},
     {name: 'foo', version: '2'},
   ].sort());
-  expect(depGraph.getDepPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getDepPkgs(), [
     {name: 'foo', version: '2'},
   ].sort());
   expect(depGraph.countPathsToRoot({name: 'toor', version: '1.0.0'})).toBe(2);
@@ -768,12 +768,12 @@ test('fromJSON a pkg missing version field', () => {
   };
 
   const depGraph = depGraphLib.createFromJSON(graphJson as any);
-  expect(depGraph.getPkgs().sort()).toEqual([
+  helpers.expectSamePkgs(depGraph.getPkgs(), [
     { name: 'toor', version: '1.0.0' },
-    { name: 'foo', version: null },
+    { name: 'foo' },
   ]);
-  expect(depGraph.getDepPkgs().sort()).toEqual([
-    { name: 'foo', version: null },
+  helpers.expectSamePkgs(depGraph.getDepPkgs(), [
+    { name: 'foo' },
   ]);
 });
 

test/helpers.ts

@@ -1,12 +1,13 @@
 import * as _ from 'lodash';
 import * as fs from 'fs';
 import * as path from 'path';
+import { PkgInfo } from '../src';
 
 export function loadFixture(name: string) {
   return JSON.parse(fs.readFileSync(path.join(__dirname, `fixtures/${name}`), 'utf8'));
 }
 
-export function depSort(a: any, b: any) {
+function depSort(a: any, b: any) {
   if (a.name < b.name) {
     return -1;
   } else if (a.name > b.name) {
@@ -20,6 +21,10 @@ export function depSort(a: any, b: any) {
   return 0;
 }
 
+export function expectSamePkgs(actual: PkgInfo[], expected: PkgInfo[]) {
+  return expect(actual.sort(depSort)).toEqual(expected.sort(depSort));
+}
+
 export function depTreesEqual(a: any, b: any) {
   if (a.name !== b.name || a.version !== b.version) {
     return false;

test/legacy/from-dep-tree.test.ts

@@ -29,23 +29,23 @@ describe('depTreeToGraph simple dysmorphic', () => {
   });
 
   test('getPkgs', async () => {
-    expect(depGraph.getPkgs().sort(helpers.depSort)).toEqual([
+    helpers.expectSamePkgs(depGraph.getPkgs(), [
       { name: 'a', version: '1.0.0' },
       { name: 'b', version: '1.0.0' },
       { name: 'c', version: '1.0.0' },
       { name: 'd', version: '0.0.1' },
       { name: 'd', version: '0.0.2' },
       { name: 'e', version: '5.0.0' },
       { name: 'root', version: '0.0.0' },
-    ].sort(helpers.depSort));
-    expect(depGraph.getDepPkgs().sort(helpers.depSort)).toEqual([
+    ]);
+    helpers.expectSamePkgs(depGraph.getDepPkgs(), [
       { name: 'a', version: '1.0.0' },
       { name: 'b', version: '1.0.0' },
       { name: 'c', version: '1.0.0' },
       { name: 'd', version: '0.0.1' },
       { name: 'd', version: '0.0.2' },
       { name: 'e', version: '5.0.0' },
-    ].sort(helpers.depSort));
+    ]);
   });
 
   test('getPathsToRoot', async () => {
@@ -228,8 +228,8 @@ describe('depTreeToGraph with funky pipes in the version', () => {
     const graphJson = depGraph.toJSON();
     const restoredGraph = await depGraphLib.createFromJSON(graphJson);
 
-    expect(restoredGraph.getPkgs().sort()).toEqual(depGraph.getPkgs().sort());
-    expect(restoredGraph.getDepPkgs().sort()).toEqual(depGraph.getDepPkgs().sort());
+    helpers.expectSamePkgs(restoredGraph.getPkgs(), depGraph.getPkgs());
+    helpers.expectSamePkgs(restoredGraph.getDepPkgs(), depGraph.getDepPkgs());
   });
 });