From a133a7252f0c1cfa982a3f8d7e300a01bca89c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Tue, 4 Sep 2018 19:41:37 -0700 Subject: [PATCH] Fix a bug that can lock the cred explorer UI (#764) Currently, it's possible to lock the cred explorer UI via the following sequence of actions: 1. Press the analyze cred button 2. While it is running, modify the weights After following this repro, the UI is stuck: it will say "loading" forever, and the analyze cred button is disabled. The issue is that loadGraph and runPagerank do not apply their success (or failure) state transitions if they are pre-empted by another state change. If a repo change occurs, that's the right behavior: the repo change puts the state back to `"READY_TO_LOAD_GRAPH"`, which means the UI is ready to re-load, and showing the PageRank results for the wrong repo would be very confusing. However, if an edge evaluator change occurs while loadGraph or runPagerank is happening, the state is left in the "LOADING" status (which means the analyze cred button is disabled). This commit fixes the issue via a refactor: per @wchargin's suggestion, responsibility for the edge evaluator moves from the state module out to `credExplorer/App.js`. This dramatically simplifies the state module, as we no longer need a `Substate` concept: we can simplify the state into a single sequence of states. As of the refactor, the bug is impossible. Test plan: Unit tests have been updated to maintain coverage on the refactored code. I manually tested that the bug no longer repro's, and that the cred explorer UI continues to function. I did not add a new test to protect against regression, because in the new codepath, the bug is basically impossible. --- src/app/credExplorer/App.js | 72 +++++---- src/app/credExplorer/App.test.js | 90 +++++------ src/app/credExplorer/state.js | 174 ++++++++------------- src/app/credExplorer/state.test.js | 233 +++++++++++------------------ 4 files changed, 227 insertions(+), 342 deletions(-) diff --git a/src/app/credExplorer/App.js b/src/app/credExplorer/App.js index 6075baa..1420a7a 100644 --- a/src/app/credExplorer/App.js +++ b/src/app/credExplorer/App.js @@ -2,11 +2,13 @@ import React from "react"; +import * as NullUtil from "../../util/null"; import type {Assets} from "../assets"; import type {LocalStore} from "../localStore"; import CheckedLocalStore from "../checkedLocalStore"; import BrowserLocalStore from "../browserLocalStore"; +import {type EdgeEvaluator} from "../../core/attribution/pagerank"; import {PagerankTable} from "./pagerankTable/Table"; import {WeightConfig} from "./WeightConfig"; import RepositorySelect from "./RepositorySelect"; @@ -15,7 +17,7 @@ import { createStateTransitionMachine, type AppState, type StateTransitionMachineInterface, - initialState, + uninitializedState, } from "./state"; export default class AppPage extends React.Component<{|+assets: Assets|}> { @@ -35,6 +37,7 @@ export default class AppPage extends React.Component<{|+assets: Assets|}> { type Props = {|+assets: Assets, +localStore: LocalStore|}; type State = {| appState: AppState, + edgeEvaluator: ?EdgeEvaluator, |}; export function createApp( @@ -49,7 +52,8 @@ export function createApp( constructor(props: Props) { super(props); this.state = { - appState: initialState(), + appState: uninitializedState(), + edgeEvaluator: null, }; this.stateTransitionMachine = createSTM( () => this.state.appState, @@ -60,15 +64,10 @@ export function createApp( render() { const {localStore} = this.props; const {appState} = this.state; - 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; + if (appState.type === "PAGERANK_EVALUATED") { + const adapters = appState.graphWithAdapters.adapters; + const pnd = appState.pagerankNodeDecomposition; pagerankTable = ( this.stateTransitionMachine.setEdgeEvaluator(ee)} + onChange={(edgeEvaluator) => this.setState({edgeEvaluator})} /> {pagerankTable} @@ -140,32 +142,26 @@ export function loadingText(state: AppState) { case "UNINITIALIZED": { return "Initializing..."; } - 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)); - } + case "READY_TO_LOAD_GRAPH": { + return { + LOADING: "Loading graph...", + NOT_LOADING: "Ready to load graph", + FAILED: "Error while loading graph", + }[state.loading]; + } + case "READY_TO_RUN_PAGERANK": { + return { + LOADING: "Running PageRank...", + NOT_LOADING: "Ready to run PageRank", + FAILED: "Error while running PageRank", + }[state.loading]; + } + case "PAGERANK_EVALUATED": { + return { + LOADING: "Re-running PageRank...", + NOT_LOADING: "", + FAILED: "Error while running PageRank", + }[state.loading]; } default: throw new Error((state.type: empty)); diff --git a/src/app/credExplorer/App.test.js b/src/app/credExplorer/App.test.js index a7b84ca..2337c09 100644 --- a/src/app/credExplorer/App.test.js +++ b/src/app/credExplorer/App.test.js @@ -13,7 +13,8 @@ import RepositorySelect from "./RepositorySelect"; import {PagerankTable} from "./pagerankTable/Table"; import {WeightConfig} from "./WeightConfig"; import {createApp, LoadingIndicator} from "./App"; -import {initialState} from "./state"; +import {uninitializedState} from "./state"; +import {_Prefix as GithubPrefix} from "../../plugins/github/nodes"; require("../testUtil").configureEnzyme(); @@ -21,7 +22,6 @@ describe("app/credExplorer/App", () => { function example() { let setState, getState; const setRepo = jest.fn(); - const setEdgeEvaluator = jest.fn(); const loadGraph = jest.fn(); const runPagerank = jest.fn(); const loadGraphAndRunPagerank = jest.fn(); @@ -31,7 +31,6 @@ describe("app/credExplorer/App", () => { getState = _getState; return { setRepo, - setEdgeEvaluator, loadGraph, runPagerank, loadGraphAndRunPagerank, @@ -39,7 +38,7 @@ describe("app/credExplorer/App", () => { } const App = createApp(createMockSTM); const el = shallow( - + ); if (setState == null || getState == null) { throw new Error("Initialization problems"); @@ -49,7 +48,6 @@ describe("app/credExplorer/App", () => { setState, getState, setRepo, - setEdgeEvaluator, loadGraph, runPagerank, loadGraphAndRunPagerank, @@ -63,41 +61,32 @@ describe("app/credExplorer/App", () => { }; } - function initialized(substate) { - return { - type: "INITIALIZED", - repo: makeRepo("foo", "bar"), - edgeEvaluator: createEvaluator(), - substate, - }; - } - const emptyAdapters = new DynamicAdapterSet(new StaticAdapterSet([]), []); const exampleStates = { - uninitialized: initialState, + uninitialized: uninitializedState, readyToLoadGraph: (loadingState) => { - return () => - initialized({ - type: "READY_TO_LOAD_GRAPH", - loading: loadingState, - }); + return () => ({ + type: "READY_TO_LOAD_GRAPH", + repo: makeRepo("foo", "bar"), + loading: loadingState, + }); }, readyToRunPagerank: (loadingState) => { - return () => - initialized({ - type: "READY_TO_RUN_PAGERANK", - loading: loadingState, - graphWithAdapters: {graph: new Graph(), adapters: emptyAdapters}, - }); + return () => ({ + type: "READY_TO_RUN_PAGERANK", + repo: makeRepo("foo", "bar"), + loading: loadingState, + graphWithAdapters: {graph: new Graph(), adapters: emptyAdapters}, + }); }, pagerankEvaluated: (loadingState) => { - return () => - initialized({ - type: "PAGERANK_EVALUATED", - loading: loadingState, - graphWithAdapters: {graph: new Graph(), adapters: emptyAdapters}, - pagerankNodeDecomposition: new Map(), - }); + return () => ({ + type: "PAGERANK_EVALUATED", + repo: makeRepo("foo", "bar"), + loading: loadingState, + graphWithAdapters: {graph: new Graph(), adapters: emptyAdapters}, + pagerankNodeDecomposition: new Map(), + }); }, }; @@ -107,8 +96,8 @@ describe("app/credExplorer/App", () => { }); it("setState is wired properly", () => { const {setState, el} = example(); - expect(initialState()).not.toBe(initialState()); // sanity check - const newState = initialState(); + expect(uninitializedState()).not.toBe(uninitializedState()); // sanity check + const newState = uninitializedState(); setState(newState); expect(el.state().appState).toBe(newState); }); @@ -139,12 +128,12 @@ describe("app/credExplorer/App", () => { function testWeightConfig(stateFn) { it("creates a working WeightConfig", () => { - const {el, setEdgeEvaluator, setState} = example(); + const {el, setState} = example(); setState(stateFn()); const wc = el.find(WeightConfig); const ee = createEvaluator(); wc.props().onChange(ee); - expect(setEdgeEvaluator).toHaveBeenCalledWith(ee); + expect(el.state().edgeEvaluator).toBe(ee); }); } @@ -153,6 +142,8 @@ describe("app/credExplorer/App", () => { it(`has a ${adjective} analyze cred button`, () => { const {el, loadGraphAndRunPagerank, setState} = example(); setState(stateFn()); + const edgeEvaluator = createEvaluator(); + el.setState({edgeEvaluator}); el.update(); const button = el.findWhere( (b) => b.text() === "Analyze cred" && b.is("button") @@ -163,8 +154,24 @@ describe("app/credExplorer/App", () => { expect(button.props().disabled).toBe(false); button.simulate("click"); expect(loadGraphAndRunPagerank).toBeCalledTimes(1); + expect(loadGraphAndRunPagerank).toBeCalledWith( + el.instance().props.assets, + edgeEvaluator, + GithubPrefix.user + ); } }); + if (!disabled) { + it("...unless the EdgeEvaluator is not available", () => { + const {el, setState} = example(); + setState(stateFn()); + el.update(); + const button = el.findWhere( + (b) => b.text() === "Analyze cred" && b.is("button") + ); + expect(button.props().disabled).toBe(true); + }); + } } function testPagerankTable(stateFn, present: boolean) { @@ -177,14 +184,11 @@ describe("app/credExplorer/App", () => { const prt = el.find(PagerankTable); if (present) { expect(prt).toHaveLength(1); - if ( - state.type !== "INITIALIZED" || - state.substate.type !== "PAGERANK_EVALUATED" - ) { + if (state.type !== "PAGERANK_EVALUATED") { throw new Error("This test case is impossible to satisfy"); } - const adapters = state.substate.graphWithAdapters.adapters; - const pnd = state.substate.pagerankNodeDecomposition; + const adapters = state.graphWithAdapters.adapters; + const pnd = state.pagerankNodeDecomposition; expect(prt.props().adapters).toBe(adapters); expect(prt.props().pnd).toBe(pnd); } else { diff --git a/src/app/credExplorer/state.js b/src/app/credExplorer/state.js index 1c0d99a..e6ba6c2 100644 --- a/src/app/credExplorer/state.js +++ b/src/app/credExplorer/state.js @@ -2,7 +2,6 @@ import deepEqual from "lodash.isequal"; -import * as NullUtil from "../../util/null"; import {Graph, type NodeAddressT} from "../../core/graph"; import type {Assets} from "../../app/assets"; import type {Repo} from "../../core/repo"; @@ -25,37 +24,32 @@ import {defaultStaticAdapters} from "../adapters/defaultPlugins"; transitions, including error cases, are thoroughly tested. */ -export type AppState = Uninitialized | Initialized; -export type Uninitialized = {| - +type: "UNINITIALIZED", - edgeEvaluator: ?EdgeEvaluator, - repo: ?Repo, -|}; -export type Initialized = {| - +type: "INITIALIZED", - +edgeEvaluator: EdgeEvaluator, - +repo: Repo, - +substate: AppSubstate, -|}; - export type LoadingState = "NOT_LOADING" | "LOADING" | "FAILED"; -export type AppSubstate = +export type AppState = + | Uninitialized | ReadyToLoadGraph | ReadyToRunPagerank | PagerankEvaluated; + +export type Uninitialized = {| + +type: "UNINITIALIZED", +|}; export type ReadyToLoadGraph = {| +type: "READY_TO_LOAD_GRAPH", + +repo: Repo, +loading: LoadingState, |}; export type ReadyToRunPagerank = {| +type: "READY_TO_RUN_PAGERANK", + +repo: Repo, +graphWithAdapters: GraphWithAdapters, +loading: LoadingState, |}; export type PagerankEvaluated = {| +type: "PAGERANK_EVALUATED", +graphWithAdapters: GraphWithAdapters, - pagerankNodeDecomposition: PagerankNodeDecomposition, + +repo: Repo, + +pagerankNodeDecomposition: PagerankNodeDecomposition, +loading: LoadingState, |}; @@ -71,17 +65,20 @@ export function createStateTransitionMachine( ); } -export function initialState(): AppState { - return {type: "UNINITIALIZED", repo: null, edgeEvaluator: null}; +export function uninitializedState(): AppState { + return {type: "UNINITIALIZED"}; } // Exported for testing purposes. export interface StateTransitionMachineInterface { +setRepo: (Repo) => void; - +setEdgeEvaluator: (EdgeEvaluator) => void; +loadGraph: (Assets) => Promise; - +runPagerank: (NodeAddressT) => Promise; - +loadGraphAndRunPagerank: (Assets, NodeAddressT) => Promise; + +runPagerank: (EdgeEvaluator, NodeAddressT) => Promise; + +loadGraphAndRunPagerank: ( + Assets, + EdgeEvaluator, + NodeAddressT + ) => Promise; } /* In production, instantiate via createStateTransitionMachine; the constructor * implementation allows specification of the loadGraphWithAdapters and @@ -119,85 +116,37 @@ export class StateTransitionMachine implements StateTransitionMachineInterface { this.pagerank = pagerank; } - _maybeInitialize(state: Uninitialized): AppState { - const {repo, edgeEvaluator} = state; - if (repo != null && edgeEvaluator != null) { - const substate = {type: "READY_TO_LOAD_GRAPH", loading: "NOT_LOADING"}; - return {type: "INITIALIZED", repo, edgeEvaluator, substate}; - } else { - return state; - } - } - setRepo(repo: Repo) { - const state = this.getState(); - switch (state.type) { - case "UNINITIALIZED": { - const newState = this._maybeInitialize({...state, repo}); - this.setState(newState); - break; - } - case "INITIALIZED": { - const substate = {type: "READY_TO_LOAD_GRAPH", loading: "NOT_LOADING"}; - const newState = {...state, repo, substate}; - this.setState(newState); - break; - } - default: { - throw new Error((state.type: empty)); - } - } - } - - setEdgeEvaluator(edgeEvaluator: EdgeEvaluator) { - const state = this.getState(); - switch (state.type) { - case "UNINITIALIZED": { - const newState = this._maybeInitialize({...state, edgeEvaluator}); - this.setState(newState); - break; - } - case "INITIALIZED": { - const newState = {...state, edgeEvaluator}; - this.setState(newState); - break; - } - default: { - throw new Error((state.type: empty)); - } - } + const newState: AppState = { + type: "READY_TO_LOAD_GRAPH", + repo: repo, + loading: "NOT_LOADING", + }; + this.setState(newState); } /** Loads the graph, reports whether it was successful */ async loadGraph(assets: Assets): Promise { const state = this.getState(); - if ( - state.type !== "INITIALIZED" || - state.substate.type !== "READY_TO_LOAD_GRAPH" - ) { + if (state.type !== "READY_TO_LOAD_GRAPH") { throw new Error("Tried to loadGraph in incorrect state"); } - const {repo, substate} = state; - const loadingState = { - ...state, - substate: {...substate, loading: "LOADING"}, - }; + const {repo} = state; + const loadingState = {...state, loading: "LOADING"}; this.setState(loadingState); let newState: ?AppState; let success = true; try { const graphWithAdapters = await this.loadGraphWithAdapters(assets, repo); newState = { - ...state, - substate: { - type: "READY_TO_RUN_PAGERANK", - graphWithAdapters, - loading: "NOT_LOADING", - }, + type: "READY_TO_RUN_PAGERANK", + graphWithAdapters, + repo, + loading: "NOT_LOADING", }; } catch (e) { console.error(e); - newState = {...state, substate: {...substate, loading: "FAILED"}}; + newState = {...loadingState, loading: "FAILED"}; success = false; } if (deepEqual(this.getState(), loadingState)) { @@ -207,26 +156,24 @@ export class StateTransitionMachine implements StateTransitionMachineInterface { return false; } - async runPagerank(totalScoreNodePrefix: NodeAddressT) { + async runPagerank( + edgeEvaluator: EdgeEvaluator, + totalScoreNodePrefix: NodeAddressT + ) { const state = this.getState(); if ( - state.type !== "INITIALIZED" || - state.substate.type === "READY_TO_LOAD_GRAPH" + state.type !== "READY_TO_RUN_PAGERANK" && + state.type !== "PAGERANK_EVALUATED" ) { throw new Error("Tried to runPagerank in incorrect state"); } - const {edgeEvaluator, substate} = state; - // Oh, the things we must do to appease flow - const loadingSubstate = - substate.type === "PAGERANK_EVALUATED" - ? {...substate, loading: "LOADING"} - : {...substate, loading: "LOADING"}; - const loadingState = { - ...state, - substate: loadingSubstate, - }; + // Flow hack :/ + const loadingState = + state.type === "READY_TO_RUN_PAGERANK" + ? {...state, loading: "LOADING"} + : {...state, loading: "LOADING"}; this.setState(loadingState); - const graph = substate.graphWithAdapters.graph; + const graph = state.graphWithAdapters.graph; let newState: ?AppState; try { const pagerankNodeDecomposition = await this.pagerank( @@ -237,50 +184,49 @@ export class StateTransitionMachine implements StateTransitionMachineInterface { totalScoreNodePrefix: totalScoreNodePrefix, } ); - const newSubstate = { + newState = { type: "PAGERANK_EVALUATED", - graphWithAdapters: substate.graphWithAdapters, pagerankNodeDecomposition, + graphWithAdapters: state.graphWithAdapters, + repo: state.repo, loading: "NOT_LOADING", }; - newState = {...state, substate: newSubstate}; } catch (e) { console.error(e); - const failedSubstate = - // More flow appeasement - substate.type === "PAGERANK_EVALUATED" - ? {...substate, loading: "FAILED"} - : {...substate, loading: "FAILED"}; - newState = {...state, substate: failedSubstate}; + // Flow hack :/ + newState = + state.type === "READY_TO_RUN_PAGERANK" + ? {...state, loading: "FAILED"} + : {...state, loading: "FAILED"}; } if (deepEqual(this.getState(), loadingState)) { - this.setState(NullUtil.get(newState)); + this.setState(newState); } } async loadGraphAndRunPagerank( assets: Assets, + edgeEvaluator: EdgeEvaluator, totalScoreNodePrefix: NodeAddressT ) { const state = this.getState(); - if (state.type === "UNINITIALIZED") { + const type = state.type; + if (type === "UNINITIALIZED") { throw new Error("Tried to load and run from incorrect state"); } - switch (state.substate.type) { + switch (type) { case "READY_TO_LOAD_GRAPH": const loadedGraph = await this.loadGraph(assets); if (loadedGraph) { - await this.runPagerank(totalScoreNodePrefix); + await this.runPagerank(edgeEvaluator, totalScoreNodePrefix); } break; case "READY_TO_RUN_PAGERANK": - await this.runPagerank(totalScoreNodePrefix); - break; case "PAGERANK_EVALUATED": - await this.runPagerank(totalScoreNodePrefix); + await this.runPagerank(edgeEvaluator, totalScoreNodePrefix); break; default: - throw new Error((state.substate.type: empty)); + throw new Error((type: empty)); } } } diff --git a/src/app/credExplorer/state.test.js b/src/app/credExplorer/state.test.js index 3c83d1e..fe2e21b 100644 --- a/src/app/credExplorer/state.test.js +++ b/src/app/credExplorer/state.test.js @@ -2,7 +2,7 @@ import { StateTransitionMachine, - initialState, + uninitializedState, type AppState, type GraphWithAdapters, } from "./state"; @@ -41,31 +41,29 @@ describe("app/credExplorer/state", () => { ); return {getState, stm, loadGraphMock, pagerankMock}; } - function initialized(substate): AppState { + function readyToLoadGraph(): AppState { return { - type: "INITIALIZED", + type: "READY_TO_LOAD_GRAPH", repo: makeRepo("foo", "bar"), - edgeEvaluator: edgeEvaluator(), - substate, + loading: "NOT_LOADING", }; } - function readyToLoadGraph(): AppState { - return initialized({type: "READY_TO_LOAD_GRAPH", loading: "NOT_LOADING"}); - } function readyToRunPagerank(): AppState { - return initialized({ + return { type: "READY_TO_RUN_PAGERANK", + repo: makeRepo("foo", "bar"), loading: "NOT_LOADING", graphWithAdapters: graphWithAdapters(), - }); + }; } function pagerankEvaluated(): AppState { - return initialized({ + return { type: "PAGERANK_EVALUATED", + repo: makeRepo("foo", "bar"), graphWithAdapters: graphWithAdapters(), pagerankNodeDecomposition: pagerankNodeDecomposition(), loading: "NOT_LOADING", - }); + }; } function edgeEvaluator(): EdgeEvaluator { return (_unused_Edge) => ({toWeight: 3, froWeight: 4}); @@ -79,122 +77,60 @@ describe("app/credExplorer/state", () => { function pagerankNodeDecomposition() { return new Map(); } - function getSubstate(state: AppState) { - if (state.type !== "INITIALIZED") { - throw new Error("Tried to get invalid substate"); - } - return state.substate; - } function loading(state: AppState) { - if ( - state.type !== "INITIALIZED" || - state.substate.type === "PAGERANK_EVALUATED" - ) { + if (state.type === "UNINITIALIZED") { throw new Error("Tried to get invalid loading"); } - return state.substate.loading; + return state.loading; + } + function getRepo(state: AppState) { + if (state.type === "UNINITIALIZED") { + throw new Error("Tried to get invalid repo"); + } + return state.repo; } describe("setRepo", () => { describe("in UNINITIALIZED", () => { - it("stays UNINITIALIZED if edge evaluator not set", () => { - const {getState, stm} = example(initialState()); + it("transitions to READY_TO_LOAD_GRAPH", () => { + const {getState, stm} = example(uninitializedState()); const repo = makeRepo("foo", "bar"); stm.setRepo(repo); const state = getState(); - expect(state.type).toBe("UNINITIALIZED"); - expect(state.repo).toEqual(repo); - }); - it("transitions to INITIALIZED if an edge evaluator was set", () => { - const {getState, stm} = example(initialState()); - stm.setEdgeEvaluator(edgeEvaluator()); - const repo = makeRepo("foo", "bar"); - stm.setRepo(repo); - const state = getState(); - expect(state.type).toBe("INITIALIZED"); - expect(state.repo).toEqual(repo); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toEqual(repo); }); }); - describe("in INITIALIZED", () => { - it("stays in READY_TO_LOAD_GRAPH with new repo", () => { - const {getState, stm} = example(readyToLoadGraph()); - const repo = makeRepo("zoink", "zod"); - stm.setRepo(repo); - const state = getState(); - expect(getSubstate(state).type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.repo).toEqual(repo); - }); - it("transitions READY_TO_RUN_PAGERANK to READY_TO_LOAD_GRAPH with new repo", () => { - const {getState, stm} = example(readyToRunPagerank()); - const repo = makeRepo("zoink", "zod"); - stm.setRepo(repo); - const state = getState(); - expect(getSubstate(state).type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.repo).toEqual(repo); - }); - it("transitions PAGERANK_EVALUATED to READY_TO_LOAD_GRAPH with new repo", () => { - const {getState, stm} = example(pagerankEvaluated()); - const repo = makeRepo("zoink", "zod"); - stm.setRepo(repo); - const state = getState(); - expect(getSubstate(state).type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.repo).toEqual(repo); - }); + it("stays in READY_TO_LOAD_GRAPH with new repo", () => { + const {getState, stm} = example(readyToLoadGraph()); + const repo = makeRepo("zoink", "zod"); + stm.setRepo(repo); + const state = getState(); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toEqual(repo); }); - }); - - describe("setEdgeEvaluator", () => { - describe("in UNINITIALIZED", () => { - it("sets ee without transitioning to INITIALIZE if repo not set", () => { - const {getState, stm} = example(initialState()); - const ee = edgeEvaluator(); - stm.setEdgeEvaluator(ee); - const state = getState(); - expect(state.type).toBe("UNINITIALIZED"); - expect(state.edgeEvaluator).toBe(ee); - }); - it("triggers transition to INITIALIZED if repo was set", () => { - const {getState, stm} = example(initialState()); - stm.setRepo(makeRepo("foo", "zod")); - const ee = edgeEvaluator(); - stm.setEdgeEvaluator(ee); - const state = getState(); - expect(state.type).toBe("INITIALIZED"); - expect(state.edgeEvaluator).toBe(ee); - }); + it("transitions READY_TO_RUN_PAGERANK to READY_TO_LOAD_GRAPH with new repo", () => { + const {getState, stm} = example(readyToRunPagerank()); + const repo = makeRepo("zoink", "zod"); + stm.setRepo(repo); + const state = getState(); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toEqual(repo); }); - describe("in INITIALIZED", () => { - it("does not transition READY_TO_LOAD_GRAPH", () => { - const {getState, stm} = example(readyToLoadGraph()); - const ee = edgeEvaluator(); - stm.setEdgeEvaluator(ee); - const state = getState(); - expect(getSubstate(state).type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.edgeEvaluator).toBe(ee); - }); - it("does not transition READY_TO_RUN_PAGERANK", () => { - const {getState, stm} = example(readyToRunPagerank()); - const ee = edgeEvaluator(); - stm.setEdgeEvaluator(ee); - const state = getState(); - expect(getSubstate(state).type).toBe("READY_TO_RUN_PAGERANK"); - expect(state.edgeEvaluator).toBe(ee); - }); - it("does not transition PAGERANK_EVALUATED", () => { - const {getState, stm} = example(pagerankEvaluated()); - const ee = edgeEvaluator(); - stm.setEdgeEvaluator(ee); - const state = getState(); - expect(getSubstate(state).type).toBe("PAGERANK_EVALUATED"); - expect(state.edgeEvaluator).toBe(ee); - }); + it("transitions PAGERANK_EVALUATED to READY_TO_LOAD_GRAPH with new repo", () => { + const {getState, stm} = example(pagerankEvaluated()); + const repo = makeRepo("zoink", "zod"); + stm.setRepo(repo); + const state = getState(); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toEqual(repo); }); }); describe("loadGraph", () => { it("can only be called when READY_TO_LOAD_GRAPH", async () => { const badStates = [ - initialState(), + uninitializedState(), readyToRunPagerank(), pagerankEvaluated(), ]; @@ -220,7 +156,7 @@ describe("app/credExplorer/state", () => { expect(loading(getState())).toBe("NOT_LOADING"); stm.loadGraph(new Assets("/my/gateway/")); expect(loading(getState())).toBe("LOADING"); - expect(getSubstate(getState()).type).toBe("READY_TO_LOAD_GRAPH"); + expect(getState().type).toBe("READY_TO_LOAD_GRAPH"); }); it("transitions to READY_TO_RUN_PAGERANK on success", async () => { const {getState, stm, loadGraphMock} = example(readyToLoadGraph()); @@ -229,15 +165,14 @@ describe("app/credExplorer/state", () => { const succeeded = await stm.loadGraph(new Assets("/my/gateway/")); expect(succeeded).toBe(true); const state = getState(); - const substate = getSubstate(state); expect(loading(state)).toBe("NOT_LOADING"); - expect(substate.type).toBe("READY_TO_RUN_PAGERANK"); - if (substate.type !== "READY_TO_RUN_PAGERANK") { + expect(state.type).toBe("READY_TO_RUN_PAGERANK"); + if (state.type !== "READY_TO_RUN_PAGERANK") { throw new Error("Impossible"); } - expect(substate.graphWithAdapters).toBe(gwa); + expect(state.graphWithAdapters).toBe(gwa); }); - it("does not transition if another transition happens first", async () => { + it("does not transition if repo transition happens first", async () => { const {getState, stm, loadGraphMock} = example(readyToLoadGraph()); const swappedRepo = makeRepo("too", "fast"); loadGraphMock.mockImplementation( @@ -250,10 +185,9 @@ describe("app/credExplorer/state", () => { const succeeded = await stm.loadGraph(new Assets("/my/gateway/")); expect(succeeded).toBe(false); const state = getState(); - const substate = getSubstate(state); expect(loading(state)).toBe("NOT_LOADING"); - expect(substate.type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.repo).toBe(swappedRepo); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toEqual(swappedRepo); }); it("sets loading state FAILED on reject", async () => { const {getState, stm, loadGraphMock} = example(readyToLoadGraph()); @@ -264,9 +198,8 @@ describe("app/credExplorer/state", () => { const succeeded = await stm.loadGraph(new Assets("/my/gateway/")); expect(succeeded).toBe(false); const state = getState(); - const substate = getSubstate(state); expect(loading(state)).toBe("FAILED"); - expect(substate.type).toBe("READY_TO_LOAD_GRAPH"); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); expect(console.error).toHaveBeenCalledTimes(1); expect(console.error).toHaveBeenCalledWith(error); }); @@ -274,12 +207,12 @@ describe("app/credExplorer/state", () => { describe("runPagerank", () => { it("can only be called when READY_TO_RUN_PAGERANK or PAGERANK_EVALUATED", async () => { - const badStates = [initialState(), readyToLoadGraph()]; + const badStates = [uninitializedState(), readyToLoadGraph()]; for (const b of badStates) { const {stm} = example(b); - await expect(stm.runPagerank(NodeAddress.empty)).rejects.toThrow( - "incorrect state" - ); + await expect( + stm.runPagerank(edgeEvaluator(), NodeAddress.empty) + ).rejects.toThrow("incorrect state"); } }); it("can be run when READY_TO_RUN_PAGERANK or PAGERANK_EVALUATED", async () => { @@ -288,30 +221,31 @@ describe("app/credExplorer/state", () => { const {stm, getState, pagerankMock} = example(g); const pnd = pagerankNodeDecomposition(); pagerankMock.mockResolvedValue(pnd); - await stm.runPagerank(NodeAddress.empty); + await stm.runPagerank(edgeEvaluator(), NodeAddress.empty); const state = getState(); - const substate = getSubstate(state); - if (substate.type !== "PAGERANK_EVALUATED") { + if (state.type !== "PAGERANK_EVALUATED") { throw new Error("Impossible"); } - expect(substate.type).toBe("PAGERANK_EVALUATED"); - expect(substate.pagerankNodeDecomposition).toBe(pnd); + expect(state.type).toBe("PAGERANK_EVALUATED"); + expect(state.pagerankNodeDecomposition).toBe(pnd); } }); it("immediately sets loading status", () => { const {getState, stm} = example(readyToRunPagerank()); expect(loading(getState())).toBe("NOT_LOADING"); - stm.runPagerank(NodeAddress.empty); + stm.runPagerank(edgeEvaluator(), NodeAddress.empty); expect(loading(getState())).toBe("LOADING"); }); it("calls pagerank with the totalScoreNodePrefix option", async () => { const {pagerankMock, stm} = example(readyToRunPagerank()); const foo = NodeAddress.fromParts(["foo"]); - await stm.runPagerank(foo); + const ee = edgeEvaluator(); + await stm.runPagerank(ee, foo); const args = pagerankMock.mock.calls[0]; + expect(args[1]).toBe(ee); expect(args[2].totalScoreNodePrefix).toBe(foo); }); - it("does not transition if another transition happens first", async () => { + it("does not transition if a repo change happens first", async () => { const {getState, stm, pagerankMock} = example(readyToRunPagerank()); const swappedRepo = makeRepo("too", "fast"); pagerankMock.mockImplementation( @@ -321,12 +255,11 @@ describe("app/credExplorer/state", () => { resolve(graphWithAdapters()); }) ); - await stm.runPagerank(NodeAddress.empty); + await stm.runPagerank(edgeEvaluator(), NodeAddress.empty); const state = getState(); - const substate = getSubstate(state); expect(loading(state)).toBe("NOT_LOADING"); - expect(substate.type).toBe("READY_TO_LOAD_GRAPH"); - expect(state.repo).toBe(swappedRepo); + expect(state.type).toBe("READY_TO_LOAD_GRAPH"); + expect(getRepo(state)).toBe(swappedRepo); }); it("sets loading state FAILED on reject", async () => { const {getState, stm, pagerankMock} = example(readyToRunPagerank()); @@ -334,11 +267,10 @@ describe("app/credExplorer/state", () => { // $ExpectFlowError console.error = jest.fn(); pagerankMock.mockRejectedValue(error); - await stm.runPagerank(NodeAddress.empty); + await stm.runPagerank(edgeEvaluator(), NodeAddress.empty); const state = getState(); - const substate = getSubstate(state); expect(loading(state)).toBe("FAILED"); - expect(substate.type).toBe("READY_TO_RUN_PAGERANK"); + expect(state.type).toBe("READY_TO_RUN_PAGERANK"); expect(console.error).toHaveBeenCalledTimes(1); expect(console.error).toHaveBeenCalledWith(error); }); @@ -346,9 +278,13 @@ describe("app/credExplorer/state", () => { describe("loadGraphAndRunPagerank", () => { it("errors if called with uninitialized state", async () => { - const {stm} = example(initialState()); + const {stm} = example(uninitializedState()); await expect( - stm.loadGraphAndRunPagerank(new Assets("gateway"), NodeAddress.empty) + stm.loadGraphAndRunPagerank( + new Assets("gateway"), + edgeEvaluator(), + NodeAddress.empty + ) ).rejects.toThrow("incorrect state"); }); it("when READY_TO_LOAD_GRAPH, loads graph then runs pagerank", async () => { @@ -358,11 +294,12 @@ describe("app/credExplorer/state", () => { stm.loadGraph.mockResolvedValue(true); const assets = new Assets("/gateway/"); const prefix = NodeAddress.fromParts(["bar"]); - await stm.loadGraphAndRunPagerank(assets, prefix); + const ee = edgeEvaluator(); + await stm.loadGraphAndRunPagerank(assets, ee, prefix); expect(stm.loadGraph).toHaveBeenCalledTimes(1); expect(stm.loadGraph).toHaveBeenCalledWith(assets); expect(stm.runPagerank).toHaveBeenCalledTimes(1); - expect(stm.runPagerank).toHaveBeenCalledWith(prefix); + expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix); }); it("does not run pagerank if loadGraph did not succeed", async () => { const {stm} = example(readyToLoadGraph()); @@ -371,7 +308,7 @@ describe("app/credExplorer/state", () => { stm.loadGraph.mockResolvedValue(false); const assets = new Assets("/gateway/"); const prefix = NodeAddress.fromParts(["bar"]); - await stm.loadGraphAndRunPagerank(assets, prefix); + await stm.loadGraphAndRunPagerank(assets, edgeEvaluator(), prefix); expect(stm.loadGraph).toHaveBeenCalledTimes(1); expect(stm.runPagerank).toHaveBeenCalledTimes(0); }); @@ -380,20 +317,22 @@ describe("app/credExplorer/state", () => { (stm: any).loadGraph = jest.fn(); (stm: any).runPagerank = jest.fn(); const prefix = NodeAddress.fromParts(["bar"]); - await stm.loadGraphAndRunPagerank(new Assets("/gateway/"), prefix); + const ee = edgeEvaluator(); + await stm.loadGraphAndRunPagerank(new Assets("/gateway/"), ee, prefix); expect(stm.loadGraph).toHaveBeenCalledTimes(0); expect(stm.runPagerank).toHaveBeenCalledTimes(1); - expect(stm.runPagerank).toHaveBeenCalledWith(prefix); + expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix); }); it("when PAGERANK_EVALUATED, runs pagerank", async () => { const {stm} = example(pagerankEvaluated()); (stm: any).loadGraph = jest.fn(); (stm: any).runPagerank = jest.fn(); const prefix = NodeAddress.fromParts(["bar"]); - await stm.loadGraphAndRunPagerank(new Assets("/gateway/"), prefix); + const ee = edgeEvaluator(); + await stm.loadGraphAndRunPagerank(new Assets("/gateway/"), ee, prefix); expect(stm.loadGraph).toHaveBeenCalledTimes(0); expect(stm.runPagerank).toHaveBeenCalledTimes(1); - expect(stm.runPagerank).toHaveBeenCalledWith(prefix); + expect(stm.runPagerank).toHaveBeenCalledWith(ee, prefix); }); }); });