From d9b4673dbdd59f31146f089012cecd79b46b177f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Tue, 8 May 2018 16:33:19 -0700 Subject: [PATCH] Factor out `github.porcelain.asEntity` (#246) @wchargin suggested that the entity-wrapping logic in porcelain reference handling should be factored out as its own method. This was a great suggestion; it will be very useful for plugin consumers, and it also results in better test coverage. Test plan: The new unit tests are nice. For your own safety, do not question or quibble with the magnificent foo plugin. --- src/plugins/github/porcelain.js | 77 +++++++++++++++------------- src/plugins/github/porcelain.test.js | 39 +++++++++++++- 2 files changed, 80 insertions(+), 36 deletions(-) diff --git a/src/plugins/github/porcelain.js b/src/plugins/github/porcelain.js index 022bcb6..fafeea1 100644 --- a/src/plugins/github/porcelain.js +++ b/src/plugins/github/porcelain.js @@ -48,6 +48,8 @@ import { REFERENCES_EDGE_TYPE, } from "./types"; +import {PLUGIN_NAME} from "./pluginName"; + import {COMMIT_NODE_TYPE} from "../git/types"; export type Entity = @@ -67,6 +69,44 @@ function assertEntityType(e: Entity, t: NodeType) { } } +export function asEntity( + g: Graph, + addr: Address +): Entity { + const type: NodeType = (addr.type: any); + if (addr.pluginName !== PLUGIN_NAME) { + throw new Error( + `Tried to make GitHub porcelain, but got the wrong plugin name: ${stringify( + addr + )}` + ); + } + switch (type) { + case "ISSUE": + return new Issue(g, addr); + case "PULL_REQUEST": + return new PullRequest(g, addr); + case "COMMENT": + return new Comment(g, addr); + case "AUTHOR": + return new Author(g, addr); + case "PULL_REQUEST_REVIEW": + return new PullRequestReview(g, addr); + case "PULL_REQUEST_REVIEW_COMMENT": + return new PullRequestReviewComment(g, addr); + case "REPOSITORY": + return new Repository(g, addr); + default: + // eslint-disable-next-line no-unused-expressions + (type: empty); + throw new Error( + `Tried to make GitHub porcelain, but got invalid type: ${stringify( + addr + )}` + ); + } +} + export class Porcelain { graph: Graph; @@ -190,45 +230,12 @@ class Post< } references(): Entity[] { - const result: Entity[] = []; - this.graph + return this.graph .neighborhood(this.nodeAddress, { edgeType: REFERENCES_EDGE_TYPE, direction: "OUT", }) - .forEach(({neighbor}) => { - const type: NodeType = (neighbor.type: any); - switch (type) { - case "ISSUE": - result.push(new Issue(this.graph, neighbor)); - break; - case "PULL_REQUEST": - result.push(new PullRequest(this.graph, neighbor)); - break; - case "COMMENT": - result.push(new Comment(this.graph, neighbor)); - break; - case "AUTHOR": - result.push(new Author(this.graph, neighbor)); - break; - case "PULL_REQUEST_REVIEW": - result.push(new PullRequestReview(this.graph, neighbor)); - break; - case "PULL_REQUEST_REVIEW_COMMENT": - result.push(new PullRequestReviewComment(this.graph, neighbor)); - break; - case "REPOSITORY": - result.push(new Repository(this.graph, neighbor)); - break; - default: - // eslint-disable-next-line no-unused-expressions - (type: empty); - throw new Error( - `Attempted to parse reference to unknown entity type ${type}` - ); - } - }); - return result; + .map(({neighbor}) => asEntity(this.graph, neighbor)); } } diff --git a/src/plugins/github/porcelain.test.js b/src/plugins/github/porcelain.test.js index 6a5ea48..f80bd3c 100644 --- a/src/plugins/github/porcelain.test.js +++ b/src/plugins/github/porcelain.test.js @@ -1,8 +1,16 @@ // @flow +import type {Address} from "../../core/address"; import {parse} from "./parser"; import exampleRepoData from "./demoData/example-github.json"; -import {Porcelain, Issue, PullRequest, Comment, Author} from "./porcelain"; +import { + asEntity, + Porcelain, + Issue, + PullRequest, + Comment, + Author, +} from "./porcelain"; import { AUTHOR_NODE_TYPE, COMMENT_NODE_TYPE, @@ -12,6 +20,8 @@ import { PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE, } from "./types"; +import {PLUGIN_NAME} from "./pluginName"; + describe("GitHub porcelain", () => { const graph = parse(exampleRepoData); const porcelain = new Porcelain(graph); @@ -23,6 +33,26 @@ describe("GitHub porcelain", () => { } return result; } + describe("asEntity", () => { + // Note: In the "wrappers" block, we test that the asEntity method works + // for each wrapper type. Here, we just test that it fails as expected. + it("errors when given an address with the wrong plugin name", () => { + const addr: Address = { + pluginName: "the magnificent foo plugin", + id: "who are you to ask an id of the magnificent foo plugin?", + type: "ISSUE", + }; + expect(() => asEntity(graph, addr)).toThrow("wrong plugin name"); + }); + it("errors when given an address with a bad node type", () => { + const addr: Address = { + pluginName: PLUGIN_NAME, + id: "if you keep asking for my id you will make me angry", + type: "the foo plugin's magnificence extends to many plugins", + }; + expect(() => asEntity(graph, addr)).toThrow("invalid type"); + }); + }); describe("has repository finding", () => { it("which works for an existing repository", () => { expect(porcelain.repository("sourcecred", "example-github")).toEqual( @@ -40,6 +70,7 @@ describe("GitHub porcelain", () => { expect(repo.url()).toBe("https://github.com/sourcecred/example-github"); expect(repo.owner()).toBe("sourcecred"); expect(repo.name()).toBe("example-github"); + expect(repo).toEqual(asEntity(graph, repo.address())); }); it("Issues", () => { const issue = issueOrPRByNumber(1); @@ -53,6 +84,7 @@ describe("GitHub porcelain", () => { expect(issue.node()).toMatchSnapshot(); expect(issue.address()).toEqual(issue.node().address); expect(issue.authors().map((x) => x.login())).toEqual(["decentralion"]); + expect(issue).toEqual(asEntity(graph, issue.address())); }); describe("PullRequests", () => { it("Merged", () => { @@ -69,6 +101,7 @@ describe("GitHub porcelain", () => { expect(pullRequest.mergeCommitHash()).toEqual( "0a223346b4e6dec0127b1e6aa892c4ee0424b66a" ); + expect(pullRequest).toEqual(asEntity(graph, pullRequest.address())); }); it("Unmerged", () => { const pullRequest = PullRequest.from(issueOrPRByNumber(9)); @@ -82,6 +115,7 @@ describe("GitHub porcelain", () => { expect(reviews).toHaveLength(2); expect(reviews[0].state()).toBe("CHANGES_REQUESTED"); expect(reviews[1].state()).toBe("APPROVED"); + expect(reviews[0]).toEqual(asEntity(graph, reviews[0].address())); }); it("Pull Request Review Comments", () => { @@ -96,6 +130,7 @@ describe("GitHub porcelain", () => { ); expect(comment.body()).toBe("seems a bit capricious"); expect(comment.authors().map((a) => a.login())).toEqual(["wchargin"]); + expect(comment).toEqual(asEntity(graph, comment.address())); }); it("Comments", () => { @@ -111,6 +146,7 @@ describe("GitHub porcelain", () => { expect(comment.node()).toMatchSnapshot(); expect(comment.address()).toEqual(comment.node().address); expect(comment.authors().map((x) => x.login())).toEqual(["decentralion"]); + expect(comment).toEqual(asEntity(graph, comment.address())); }); it("Authors", () => { @@ -127,6 +163,7 @@ describe("GitHub porcelain", () => { expect(decentralion.subtype()).toBe("USER"); expect(decentralion.node()).toMatchSnapshot(); expect(decentralion.address()).toEqual(decentralion.node().address); + expect(decentralion).toEqual(asEntity(graph, decentralion.address())); }); });