From 06551f4909d94f5b6f5bffa99fd594000dbd14ff Mon Sep 17 00:00:00 2001 From: Ian MacFarland Date: Wed, 9 Dec 2020 16:42:46 -0800 Subject: [PATCH] make tests reactive --- spotlight-client/package.json | 1 + .../src/DataStore/DataStore.test.ts | 20 +++++- .../src/contentModels/Metric.test.ts | 65 ++++++++++++++----- spotlight-client/src/contentModels/Metric.ts | 4 ++ spotlight-client/src/index.tsx | 11 ++++ spotlight-client/src/setupTests.ts | 11 +++- spotlight-client/tsconfig.json | 3 +- yarn.lock | 5 ++ 8 files changed, 99 insertions(+), 21 deletions(-) diff --git a/spotlight-client/package.json b/spotlight-client/package.json index 4b512442..e44cd6f8 100644 --- a/spotlight-client/package.json +++ b/spotlight-client/package.json @@ -20,6 +20,7 @@ "assert-never": "^1.2.1", "jest-fetch-mock": "^3.0.3", "mobx": "^6.0.4", + "mobx-utils": "^6.0.1", "react": "^16.13.1", "react-app-polyfill": "^1.0.6", "react-dom": "^16.13.1", diff --git a/spotlight-client/src/DataStore/DataStore.test.ts b/spotlight-client/src/DataStore/DataStore.test.ts index 05effe6d..b3d542d1 100644 --- a/spotlight-client/src/DataStore/DataStore.test.ts +++ b/spotlight-client/src/DataStore/DataStore.test.ts @@ -15,11 +15,21 @@ // along with this program. If not, see . // ============================================================================= +import { autorun } from "mobx"; import Tenant from "../contentModels/Tenant"; import RootStore from "./RootStore"; let DataStore: RootStore; +/** + * Convenience method to run an immediate, one-time reactive effect + */ +function reactImmediately(effect: () => void) { + // this will call the effect function immediately, + // and then immediately call the disposer to tear down the reaction + autorun(effect)(); +} + beforeEach(() => { DataStore = new RootStore(); }); @@ -36,11 +46,17 @@ describe("tenant store", () => { }); test("has no default tenant", () => { - expect(tenantStore.currentTenant).toBeUndefined(); + reactImmediately(() => { + expect(tenantStore.currentTenant).toBeUndefined(); + }); + expect.hasAssertions(); }); test("can set current tenant", () => { tenantStore.setCurrentTenant({ tenantId: "US_ND" }); - expect(tenantStore.currentTenant).toBeInstanceOf(Tenant); + reactImmediately(() => { + expect(tenantStore.currentTenant).toBeInstanceOf(Tenant); + }); + expect.hasAssertions(); }); }); diff --git a/spotlight-client/src/contentModels/Metric.test.ts b/spotlight-client/src/contentModels/Metric.test.ts index a7bc5543..acb90abb 100644 --- a/spotlight-client/src/contentModels/Metric.test.ts +++ b/spotlight-client/src/contentModels/Metric.test.ts @@ -16,6 +16,8 @@ // ============================================================================= import fetchMock from "jest-fetch-mock"; +import { when } from "mobx"; +import { fromPromise } from "mobx-utils"; import { createMetricMapping } from "./Metric"; import retrieveContent from "../contentApi/retrieveContent"; import { MetricTypeId, MetricTypeIdList } from "../contentApi/types"; @@ -65,34 +67,63 @@ describe("data fetching", () => { testMetricMapping = getTestMapping(); }); - test.each(MetricTypeIdList)("for metric %s", async (metricId) => { + test.each(MetricTypeIdList)("for metric %s", (metricId, done) => { + expect.hasAssertions(); const metric = getTestMetric(metricId); - await metric.fetch(); - - // Be advised, these snapshots are huge! However, the only expected failure cases here are: - // 1. you intentionally changed the contents of the fixture in spotlight-api - // 2. you intentionally changed the record format for this Metric type - // Be especially careful inspecting snapshots for Metrics that filter their sources, - // e.g. Parole/Probation metrics. Verify that they use the right rows! - expect(metric.records).toMatchSnapshot(); + metric.fetch(); + + when( + () => metric.records !== undefined, + () => { + // Be advised, these snapshots are huge! However, the only expected failure cases here are: + // 1. you intentionally changed the contents of the fixture in spotlight-api + // 2. you intentionally changed the record format for this Metric type + // Be especially careful inspecting snapshots for Metrics that filter their sources, + // e.g. Parole/Probation metrics. Verify that they use the right rows! + expect(metric.records).toMatchSnapshot(); + // @ts-expect-error typedefs for `test.each` are wrong, `done` will be a function + done(); + } + ); }); }); -test("file loading state", async () => { +test("file loading state", (done) => { testMetricMapping = getTestMapping(); // not really necessary to test this once per type; we just pick one arbitrarily const metric = getTestMetric("ParoleSuccessHistorical"); - expect(metric.isLoading).toBeUndefined(); + let dataPromise: ReturnType; - const dataPromise = metric.fetch(); - expect(metric.isLoading).toBe(true); - expect(metric.records).toBeUndefined(); + // this should be the initial state of the metric instance + when( + () => metric.isLoading === undefined, + () => { + expect(metric.records).toBeUndefined(); + // the fetch is initiated here; this will trigger the reactions below + dataPromise = fromPromise(metric.fetch()); + } + ); + + when( + () => dataPromise.state === "pending", + () => { + expect(metric.isLoading).toBe(true); + expect(metric.records).toBeUndefined(); + } + ); + + when( + () => dataPromise.state === "fulfilled", + () => { + expect(metric.isLoading).toBe(false); + expect(metric.records).toBeDefined(); + done(); + } + ); - await dataPromise; - expect(metric.isLoading).toBe(false); - expect(metric.records).toBeDefined(); + expect.assertions(5); }); test("fetch error state", async () => { diff --git a/spotlight-client/src/contentModels/Metric.ts b/spotlight-client/src/contentModels/Metric.ts index ff774ef8..5bed75cd 100644 --- a/spotlight-client/src/contentModels/Metric.ts +++ b/spotlight-client/src/contentModels/Metric.ts @@ -109,6 +109,10 @@ export default class Metric { methodology: false, name: false, tenantId: false, + // in practice, collections should not change once we are done + // boostrapping them (which is done right after instantiation); + // no need to make them observable + collections: false, }); // initialize metadata diff --git a/spotlight-client/src/index.tsx b/spotlight-client/src/index.tsx index fab86209..c5c29098 100644 --- a/spotlight-client/src/index.tsx +++ b/spotlight-client/src/index.tsx @@ -17,10 +17,21 @@ import "react-app-polyfill/ie11"; import "react-app-polyfill/stable"; +import { configure } from "mobx"; + import React from "react"; import ReactDOM from "react-dom"; import App from "./App"; +configure({ + // make proxies optional for IE 11 support + useProxies: "ifavailable", + // activate runtime linting + computedRequiresReaction: true, + reactionRequiresObservable: true, + observableRequiresReaction: true, +}); + ReactDOM.render( diff --git a/spotlight-client/src/setupTests.ts b/spotlight-client/src/setupTests.ts index 1eebbc07..efc869e7 100644 --- a/spotlight-client/src/setupTests.ts +++ b/spotlight-client/src/setupTests.ts @@ -21,11 +21,20 @@ // learn more: https://github.com/testing-library/jest-dom import "@testing-library/jest-dom/extend-expect"; - import fetchMock from "jest-fetch-mock"; +import { configure } from "mobx"; // we want this mock to be available but disabled by default; // tests should default to doing real fetches against a /spotlight-api test server // but can mock it per test to simulate errors, etc fetchMock.enableMocks(); fetchMock.dontMock(); + +configure({ + // activate runtime linting + computedRequiresReaction: true, + reactionRequiresObservable: true, + observableRequiresReaction: true, + // debug setting to avoid silent failures in reactive code + disableErrorBoundaries: true, +}); diff --git a/spotlight-client/tsconfig.json b/spotlight-client/tsconfig.json index ca3e4c93..a04fff01 100644 --- a/spotlight-client/tsconfig.json +++ b/spotlight-client/tsconfig.json @@ -21,7 +21,8 @@ "isolatedModules": true, "lib": ["dom", "dom.iterable", "esnext"], "moduleResolution": "node", - "noEmit": true + "noEmit": true, + "useDefineForClassFields": true }, "include": ["src"] } diff --git a/yarn.lock b/yarn.lock index 4ae17250..5e86a460 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8977,6 +8977,11 @@ mkdirp@^0.5.1, mkdirp@^0.5.3, mkdirp@^0.5.5, mkdirp@~0.5.1: dependencies: minimist "^1.2.5" +mobx-utils@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/mobx-utils/-/mobx-utils-6.0.1.tgz#bc16c2c62794a48498685f3663e6162d42fb6106" + integrity sha512-0p5xqHDt/arz3cswVcEHYKY/BwL+fifsNroT3Gjkz9lNC7ZDsJm3IuGsmYlvFcVRJRr1rwlFZGuZXrt/MhHEJQ== + mobx@^6.0.4: version "6.0.4" resolved "https://registry.yarnpkg.com/mobx/-/mobx-6.0.4.tgz#8fc3e3629a3346f8afddf5bd954411974744dad1"