From feef1192502ee0160b60c7e916718c71171bd512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Fri, 8 Jun 2018 14:17:57 -0700 Subject: [PATCH] Add and implement `Graph.equals` (#369) It turns out we forgot to add this to the API, so I added it. I also implemented it. The tests are pretty thorough; as an added innovation over our previous tests (e.g. in #312 and #61), we now consistently test that equality is commutative. In contrast to our previous implementations, this one is massively simpler. That's an upside of using primitive ES6 data structures to store all of the graph's information... which is itself an upside of not trying to store arbitrary additional information in the graph. Now we can just do a deep equality check on the underlying nodes set and edges map! We might be able to performance tune this method by taking advantage of the structure of our nodes and edges. This should suffice for now, though. Paired with @wchargin Test plan: Unit tests were added. Run `yarn travis` --- src/v3/core/graph.js | 11 +++++ src/v3/core/graph.test.js | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/v3/core/graph.js b/src/v3/core/graph.js index 9729259..ced3b72 100644 --- a/src/v3/core/graph.js +++ b/src/v3/core/graph.js @@ -1,5 +1,7 @@ // @flow +import deepEqual from "lodash.isequal"; + import {makeAddressModule, type AddressModule} from "./address"; export opaque type NodeAddressT: string = string; @@ -292,6 +294,15 @@ export class Graph { this._checkForComodification(initialModificationCount); } + equals(that: Graph): boolean { + if (!(that instanceof Graph)) { + throw new Error(`Expected Graph, got ${String(that)}`); + } + return ( + deepEqual(this._nodes, that._nodes) && deepEqual(this._edges, that._edges) + ); + } + copy(): Graph { throw new Error("copy"); } diff --git a/src/v3/core/graph.test.js b/src/v3/core/graph.test.js index c628d07..283f3c4 100644 --- a/src/v3/core/graph.test.js +++ b/src/v3/core/graph.test.js @@ -703,6 +703,100 @@ describe("core/graph", () => { }); }); }); + + describe("equals", () => { + const src = NodeAddress.fromParts(["src"]); + const dst = NodeAddress.fromParts(["dst"]); + const edge1 = () => ({ + src, + dst, + address: EdgeAddress.fromParts(["edge"]), + }); + const conflictingEdge = () => ({ + src: dst, + dst: src, + address: EdgeAddress.fromParts(["edge"]), + }); + const edge2 = () => ({ + src: dst, + dst: src, + address: EdgeAddress.fromParts(["edge", "2"]), + }); + function expectEquality(g1, g2, isEqual: boolean) { + expect(g1.equals(g2)).toBe(isEqual); + expect(g2.equals(g1)).toBe(isEqual); + } + + it("empty graph equals itself", () => { + expectEquality(new Graph(), new Graph(), true); + }); + it("empty graph doesn't equal nonempty graph", () => { + const g = new Graph().addNode(src); + expectEquality(g, new Graph(), false); + }); + it("adding and removing a node doesn't change equality", () => { + const g = new Graph().addNode(src).removeNode(src); + expectEquality(g, new Graph(), true); + }); + it("adding an edge changes equality", () => { + const g1 = new Graph().addNode(src).addNode(dst); + const g2 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(edge1()); + expectEquality(g1, g2, false); + }); + it("adding nodes in different order doesn't change equality", () => { + const g1 = new Graph().addNode(src).addNode(dst); + const g2 = new Graph().addNode(dst).addNode(src); + expectEquality(g1, g2, true); + }); + it("graphs with conflicting edges are not equal", () => { + const g1 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(edge1()); + const g2 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(conflictingEdge()); + expectEquality(g1, g2, false); + }); + it("adding edges in different order doesn't change equality", () => { + const g1 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(edge1()) + .addEdge(edge2()); + const g2 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(edge2()) + .addEdge(edge1()); + expectEquality(g1, g2, true); + }); + it("adding and removing an edge doesn't change equality", () => { + const g1 = new Graph().addNode(src).addNode(dst); + const g2 = new Graph() + .addNode(src) + .addNode(dst) + .addEdge(edge1()) + .removeEdge(edge1().address); + expectEquality(g1, g2, true); + }); + it("throws error on null", () => { + // $ExpectFlowError + expect(() => new Graph().equals(null)).toThrow("null"); + }); + it("throws error on undefined", () => { + // $ExpectFlowError + expect(() => new Graph().equals(undefined)).toThrow("undefined"); + }); + it("throws error on non-graph object", () => { + // $ExpectFlowError + expect(() => new Graph().equals({})).toThrow("object"); + }); + }); }); describe("edgeToString", () => {