From 10f704ebd2f004530c95036fec70c0fb8945d450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Thu, 2 Aug 2018 20:16:55 -0700 Subject: [PATCH] Use AppStateTransitionMachine to drive App (#583) Pull #579 reifies the cred explorer state as an explicit state transition machine, with a well-tested implementation. This pull re-writes `credExplorer/App.js` to use that implementation, and thoroughly tests it. The result is that `credExplorer/App.js` has much simpler code (because it just binds the rendered components to the state machine), and is much more thoroughly tested. To ensure easy testability of the `App` class, it was refactored so that the module exports a factory function which takes a method for creating the `AppStateTransitionMachine` and returns an `App` class. This ensures that in test code, we can easily mock the state transition machine. This had no effect on external callers, since the higher-level `` class, which already wraps over `LocalStore` choice, was already the preferred call site. I also added a loading indicator component, which presently displays a status text corresponding to the state, such as "Loading graph...", or "Error while running PageRank". This way, there is always some user feedback during loading states (which could take a while). Test plan: Visual inspection, and the very thorough included unit tests. --- src/app/credExplorer/App.js | 198 ++++++++------- src/app/credExplorer/App.test.js | 421 +++++++++++++++++++++++-------- 2 files changed, 433 insertions(+), 186 deletions(-) diff --git a/src/app/credExplorer/App.js b/src/app/credExplorer/App.js index 2edb70c..b3f9299 100644 --- a/src/app/credExplorer/App.js +++ b/src/app/credExplorer/App.js @@ -6,19 +6,15 @@ import type {LocalStore} from "../localStore"; import CheckedLocalStore from "../checkedLocalStore"; import BrowserLocalStore from "../browserLocalStore"; -import {StaticPluginAdapter as GithubAdapter} from "../../plugins/github/pluginAdapter"; -import {StaticPluginAdapter as GitAdapter} from "../../plugins/git/pluginAdapter"; -import {Graph} from "../../core/graph"; -import {pagerank} from "../../core/attribution/pagerank"; import {PagerankTable} from "./PagerankTable"; -import type {DynamicPluginAdapter} from "../pluginAdapter"; -import {type EdgeEvaluator} from "../../core/attribution/pagerank"; import {WeightConfig} from "./WeightConfig"; -import type {PagerankNodeDecomposition} from "../../core/attribution/pagerankNodeDecomposition"; import RepositorySelect from "./RepositorySelect"; -import type {Repo} from "../../core/repo"; - -import * as NullUtil from "../../util/null"; +import { + createStateTransitionMachine, + type AppState, + type StateTransitionMachineInterface, + initialState, +} from "./state"; export default class AppPage extends React.Component<{||}> { static _LOCAL_STORE = new CheckedLocalStore( @@ -29,109 +25,139 @@ export default class AppPage extends React.Component<{||}> { ); render() { + const App = createApp(createStateTransitionMachine); return ; } } type Props = {|+localStore: LocalStore|}; -type State = { - selectedRepo: ?Repo, - data: {| - graphWithAdapters: ?{| - +graph: Graph, - +adapters: $ReadOnlyArray, - |}, - +pnd: ?PagerankNodeDecomposition, - |}, - edgeEvaluator: ?EdgeEvaluator, -}; +type State = {| + appState: AppState, +|}; -const MAX_ENTRIES_PER_LIST = 100; +export function createApp( + createSTM: ( + getState: () => AppState, + setState: (AppState) => void + ) => StateTransitionMachineInterface +) { + return class App extends React.Component { + stateTransitionMachine: StateTransitionMachineInterface; -export class App extends React.Component { - constructor(props: Props) { - super(props); - this.state = { - selectedRepo: null, - data: {graphWithAdapters: null, pnd: null}, - edgeEvaluator: null, - }; - } + constructor(props: Props) { + super(props); + this.state = { + appState: initialState(), + }; + this.stateTransitionMachine = createSTM( + () => this.state.appState, + (appState) => this.setState({appState}) + ); + } - render() { - const {localStore} = this.props; - const {edgeEvaluator, selectedRepo} = this.state; - const {graphWithAdapters, pnd} = this.state.data; - return ( -
-
+ render() { + const {localStore} = this.props; + const {appState} = this.state; + const subType = + appState.type === "INITIALIZED" ? appState.substate.type : null; + const loadingState = + appState.type === "INITIALIZED" ? appState.substate.loading : null; + let pagerankTable; + if ( + appState.type === "INITIALIZED" && + appState.substate.type === "PAGERANK_EVALUATED" + ) { + const adapters = appState.substate.graphWithAdapters.adapters; + const pnd = appState.substate.pagerankNodeDecomposition; + pagerankTable = ( + + ); + } + return ( +
this.setState({selectedRepo})} + onChange={(repo) => this.stateTransitionMachine.setRepo(repo)} />
this.setState({edgeEvaluator: ee})} - /> - x.adapters)} - pnd={pnd} - maxEntriesPerList={MAX_ENTRIES_PER_LIST} + onChange={(ee) => this.stateTransitionMachine.setEdgeEvaluator(ee)} /> + + {pagerankTable}
-
+ ); + } + }; +} + +export class LoadingIndicator extends React.PureComponent<{| + +appState: AppState, +|}> { + render() { + return ( + {loadingText(this.props.appState)} ); } +} - loadData() { - const {selectedRepo} = this.state; - if (selectedRepo == null) { - throw new Error(`Impossible`); +export function loadingText(state: AppState) { + switch (state.type) { + case "UNINITIALIZED": { + return "Initializing..."; } - - const statics = [new GithubAdapter(), new GitAdapter()]; - Promise.all(statics.map((a) => a.load(selectedRepo))).then((adapters) => { - const graph = Graph.merge(adapters.map((x) => x.graph())); - const data = { - graphWithAdapters: { - graph, - adapters, - }, - pnd: null, - }; - this.setState({data}); - }); + case "INITIALIZED": { + switch (state.substate.type) { + case "READY_TO_LOAD_GRAPH": { + return { + LOADING: "Loading graph...", + NOT_LOADING: "Ready to load graph", + FAILED: "Error while loading graph", + }[state.substate.loading]; + } + case "READY_TO_RUN_PAGERANK": { + return { + LOADING: "Running PageRank...", + NOT_LOADING: "Ready to run PageRank", + FAILED: "Error while running PageRank", + }[state.substate.loading]; + } + case "PAGERANK_EVALUATED": { + return { + LOADING: "Re-running PageRank...", + NOT_LOADING: "", + FAILED: "Error while running PageRank", + }[state.substate.loading]; + } + default: + throw new Error((state.substate.type: empty)); + } + } + default: + throw new Error((state.type: empty)); } } diff --git a/src/app/credExplorer/App.test.js b/src/app/credExplorer/App.test.js index 9c950c2..3c859ec 100644 --- a/src/app/credExplorer/App.test.js +++ b/src/app/credExplorer/App.test.js @@ -1,114 +1,335 @@ // @flow + import React from "react"; import {shallow} from "enzyme"; +import {Graph} from "../../core/graph"; +import {makeRepo} from "../../core/repo"; import testLocalStore from "../testLocalStore"; -import {pagerank} from "../../core/attribution/pagerank"; -import {App} from "./App"; -import {Graph, NodeAddress, EdgeAddress} from "../../core/graph"; +import RepositorySelect from "./RepositorySelect"; +import {PagerankTable} from "./PagerankTable"; +import {WeightConfig} from "./WeightConfig"; +import {createApp, LoadingIndicator} from "./App"; +import {initialState} from "./state"; require("../testUtil").configureEnzyme(); -require("../testUtil").configureAphrodite(); - -function example() { - const graph = new Graph(); - const nodes = { - fooAlpha: NodeAddress.fromParts(["foo", "a", "1"]), - fooBeta: NodeAddress.fromParts(["foo", "b", "2"]), - bar1: NodeAddress.fromParts(["bar", "a", "1"]), - bar2: NodeAddress.fromParts(["bar", "2"]), - xox: NodeAddress.fromParts(["xox"]), - empty: NodeAddress.empty, - }; - Object.values(nodes).forEach((n) => graph.addNode((n: any))); - - function addEdge(parts, src, dst) { - const edge = {address: EdgeAddress.fromParts(parts), src, dst}; - graph.addEdge(edge); - } - - addEdge(["a"], nodes.fooAlpha, nodes.fooBeta); - addEdge(["b"], nodes.fooAlpha, nodes.bar1); - addEdge(["c"], nodes.fooAlpha, nodes.xox); - addEdge(["d"], nodes.bar1, nodes.bar1); - addEdge(["e"], nodes.bar1, nodes.xox); - addEdge(["e'"], nodes.bar1, nodes.xox); - - const adapters = [ - { - name: () => "foo", - graph: () => { - throw new Error("unused"); - }, - renderer: () => ({ - nodeDescription: (x) => `foo: ${NodeAddress.toString(x)}`, - }), - nodePrefix: () => NodeAddress.fromParts(["foo"]), - nodeTypes: () => [ - {name: "alpha", prefix: NodeAddress.fromParts(["foo", "a"])}, - {name: "beta", prefix: NodeAddress.fromParts(["foo", "b"])}, - ], - }, - { - name: () => "bar", - graph: () => { - throw new Error("unused"); - }, - renderer: () => ({ - nodeDescription: (x) => `bar: ${NodeAddress.toString(x)}`, - }), - nodePrefix: () => NodeAddress.fromParts(["bar"]), - nodeTypes: () => [ - {name: "alpha", prefix: NodeAddress.fromParts(["bar", "a"])}, - ], - }, - { - name: () => "xox", - graph: () => { - throw new Error("unused"); - }, - renderer: () => ({ - nodeDescription: (_unused_arg) => `xox node!`, - }), - nodePrefix: () => NodeAddress.fromParts(["xox"]), - nodeTypes: () => [], - }, - { - name: () => "unused", - graph: () => { - throw new Error("unused"); - }, - renderer: () => { - throw new Error("Impossible!"); - }, - nodePrefix: () => NodeAddress.fromParts(["unused"]), - nodeTypes: () => [], - }, - ]; - - const pagerankResult = pagerank(graph, (_unused_Edge) => ({ - toWeight: 1, - froWeight: 1, - })); - - return {adapters, nodes, graph, pagerankResult}; -} describe("app/credExplorer/App", () => { - it("renders with clean state", () => { - shallow(); + function example() { + let setState, getState; + const setRepo = jest.fn(); + const setEdgeEvaluator = jest.fn(); + const loadGraph = jest.fn(); + const runPagerank = jest.fn(); + const localStore = testLocalStore(); + function createMockSTM(_getState, _setState) { + setState = _setState; + getState = _getState; + return { + setRepo, + setEdgeEvaluator, + loadGraph, + runPagerank, + }; + } + const App = createApp(createMockSTM); + const el = shallow(); + if (setState == null || getState == null) { + throw new Error("Initialization problems"); + } + return { + el, + setState, + getState, + setRepo, + setEdgeEvaluator, + loadGraph, + runPagerank, + localStore, + }; + } + + function createEvaluator() { + return function(_unused_edge) { + return {toWeight: 1, froWeight: 1}; + }; + } + + function initialized(substate) { + return { + type: "INITIALIZED", + repo: makeRepo("foo", "bar"), + edgeEvaluator: createEvaluator(), + substate, + }; + } + + const exampleStates = { + uninitialized: initialState, + readyToLoadGraph: (loadingState) => { + return () => + initialized({ + type: "READY_TO_LOAD_GRAPH", + loading: loadingState, + }); + }, + readyToRunPagerank: (loadingState) => { + return () => + initialized({ + type: "READY_TO_RUN_PAGERANK", + loading: loadingState, + graphWithAdapters: {graph: new Graph(), adapters: []}, + }); + }, + pagerankEvaluated: (loadingState) => { + return () => + initialized({ + type: "PAGERANK_EVALUATED", + loading: loadingState, + graphWithAdapters: {graph: new Graph(), adapters: []}, + pagerankNodeDecomposition: new Map(), + }); + }, + }; + + it("getState is wired properly", () => { + const {getState, el} = example(); + expect(el.state().appState).toBe(getState()); }); - it("renders with graph and adapters set", () => { - const app = shallow(); - const {graph, adapters} = example(); - const data = {graph, adapters, pagerankResult: null}; - app.setState({data}); + it("setState is wired properly", () => { + const {setState, el} = example(); + expect(initialState()).not.toBe(initialState()); // sanity check + const newState = initialState(); + setState(newState); + expect(el.state().appState).toBe(newState); }); - it("renders with graph and adapters and pagerankResult", () => { - const app = shallow(); - const {graph, adapters, pagerankResult} = example(); - const data = {graph, adapters, pagerankResult}; - app.setState({data}); + it("localStore is wired properly", () => { + const {el, localStore} = example(); + expect(el.instance().props.localStore).toBe(localStore); + }); + + describe("when in state:", () => { + function testRepositorySelect(stateFn) { + it("creates a working RepositorySelect", () => { + const {el, setRepo, setState, localStore} = example(); + setState(stateFn()); + const rs = el.find(RepositorySelect); + const newRepo = makeRepo("zoo", "zod"); + rs.props().onChange(newRepo); + expect(setRepo).toHaveBeenCalledWith(newRepo); + expect(rs.props().localStore).toBe(localStore); + }); + } + + function testWeightConfig(stateFn) { + it("creates a working WeightConfig", () => { + const {el, setEdgeEvaluator, setState, localStore} = example(); + setState(stateFn()); + const wc = el.find(WeightConfig); + const ee = createEvaluator(); + wc.props().onChange(ee); + expect(setEdgeEvaluator).toHaveBeenCalledWith(ee); + expect(wc.props().localStore).toBe(localStore); + }); + } + + function testGraphButton(stateFn, {disabled}) { + const adjective = disabled ? "disabled" : "working"; + it(`has a ${adjective} load graph button`, () => { + const {el, loadGraph, setState} = example(); + setState(stateFn()); + el.update(); + const button = el.findWhere( + (b) => b.text() === "Load graph" && b.is("button") + ); + if (disabled) { + expect(button.props().disabled).toBe(true); + } else { + expect(button.props().disabled).toBe(false); + button.simulate("click"); + expect(loadGraph).toBeCalledTimes(1); + } + }); + } + + function testPagerankButton(stateFn, {disabled}) { + const adjective = disabled ? "disabled" : "working"; + it(`has a ${adjective} run PageRank button`, () => { + const {el, runPagerank, setState} = example(); + setState(stateFn()); + el.update(); + const button = el.findWhere( + (b) => b.text() === "Run PageRank" && b.is("button") + ); + if (disabled) { + expect(button.props().disabled).toBe(true); + } else { + expect(button.props().disabled).toBe(false); + button.simulate("click"); + expect(runPagerank).toBeCalledTimes(1); + } + }); + } + + function testPagerankTable(stateFn, present: boolean) { + const verb = present ? "has" : "doesn't have"; + it(`${verb} a PagerankTable`, () => { + const {el, setState} = example(); + const state = stateFn(); + setState(state); + el.update(); + const prt = el.find(PagerankTable); + if (present) { + expect(prt).toHaveLength(1); + if ( + state.type !== "INITIALIZED" || + state.substate.type !== "PAGERANK_EVALUATED" + ) { + throw new Error("This test case is impossible to satisfy"); + } + const adapters = state.substate.graphWithAdapters.adapters; + const pnd = state.substate.pagerankNodeDecomposition; + expect(prt.props().adapters).toBe(adapters); + expect(prt.props().pnd).toBe(pnd); + } else { + expect(prt).toHaveLength(0); + } + }); + } + + function testLoadingIndicator(stateFn) { + it("has a LoadingIndicator", () => { + const {el, setState} = example(); + const state = stateFn(); + setState(state); + el.update(); + const li = el.find(LoadingIndicator); + expect(li.props().appState).toEqual(state); + }); + } + + function stateTestSuite( + suiteName, + stateFn, + {loadGraphDisabled, runPagerankDisabled, hasPagerankTable} + ) { + describe(suiteName, () => { + testWeightConfig(stateFn); + testRepositorySelect(stateFn); + testGraphButton(stateFn, {disabled: loadGraphDisabled}); + testPagerankButton(stateFn, {disabled: runPagerankDisabled}); + testPagerankTable(stateFn, hasPagerankTable); + testLoadingIndicator(stateFn); + }); + } + + stateTestSuite("UNINITIALIZED", exampleStates.uninitialized, { + loadGraphDisabled: true, + runPagerankDisabled: true, + hasPagerankTable: false, + }); + describe("READY_TO_LOAD_GRAPH", () => { + for (const loadingState of ["LOADING", "NOT_LOADING", "FAILED"]) { + stateTestSuite( + loadingState, + exampleStates.readyToLoadGraph(loadingState), + { + loadGraphDisabled: false, + runPagerankDisabled: true, + hasPagerankTable: false, + } + ); + } + }); + + describe("READY_TO_RUN_PAGERANK", () => { + for (const loadingState of ["LOADING", "NOT_LOADING", "FAILED"]) { + stateTestSuite( + loadingState, + exampleStates.readyToRunPagerank(loadingState), + { + loadGraphDisabled: true, + runPagerankDisabled: loadingState === "LOADING", + hasPagerankTable: false, + } + ); + } + }); + + describe("PAGERANK_EVALUATED", () => { + for (const loadingState of ["LOADING", "NOT_LOADING", "FAILED"]) { + stateTestSuite( + loadingState, + exampleStates.pagerankEvaluated(loadingState), + { + loadGraphDisabled: true, + runPagerankDisabled: loadingState === "LOADING", + hasPagerankTable: true, + } + ); + } + }); + }); + + describe("LoadingIndicator", () => { + describe("displays the right status text when ", () => { + function testStatusText(stateName, stateFn, expectedText) { + it(stateName, () => { + const el = shallow(); + expect(el.text()).toEqual(expectedText); + }); + } + testStatusText( + "initializing", + exampleStates.uninitialized, + "Initializing..." + ); + testStatusText( + "ready to load graph", + exampleStates.readyToLoadGraph("NOT_LOADING"), + "Ready to load graph" + ); + testStatusText( + "loading graph", + exampleStates.readyToLoadGraph("LOADING"), + "Loading graph..." + ); + testStatusText( + "failed to load graph", + exampleStates.readyToLoadGraph("FAILED"), + "Error while loading graph" + ); + testStatusText( + "ready to run pagerank", + exampleStates.readyToRunPagerank("NOT_LOADING"), + "Ready to run PageRank" + ); + testStatusText( + "running pagerank", + exampleStates.readyToRunPagerank("LOADING"), + "Running PageRank..." + ); + testStatusText( + "pagerank failed", + exampleStates.readyToRunPagerank("FAILED"), + "Error while running PageRank" + ); + testStatusText( + "pagerank succeeded", + exampleStates.pagerankEvaluated("NOT_LOADING"), + "" + ); + testStatusText( + "re-running pagerank", + exampleStates.pagerankEvaluated("LOADING"), + "Re-running PageRank..." + ); + testStatusText( + "re-running pagerank failed", + exampleStates.pagerankEvaluated("FAILED"), + "Error while running PageRank" + ); + }); }); });