From f73abb5adcf96ee8a16c226435a3f98317dbbe37 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 22 Jan 2025 16:02:19 -0800 Subject: [PATCH 1/4] move fetch mock helpers to separate file --- .../ApiBasedRepositoryLoaderTests.test.ts | 44 +++---------------- ...TimedApiBasedRepositoryLoaderTests.test.ts | 13 ++++++ test/mockHelpers/fetchMockHelpers.ts | 41 +++++++++++++++++ 3 files changed, 59 insertions(+), 39 deletions(-) create mode 100644 test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts create mode 100644 test/mockHelpers/fetchMockHelpers.ts diff --git a/test/loaders/ApiBasedRepositoryLoaderTests.test.ts b/test/loaders/ApiBasedRepositoryLoaderTests.test.ts index ed4b92d..7bb4e7f 100644 --- a/test/loaders/ApiBasedRepositoryLoaderTests.test.ts +++ b/test/loaders/ApiBasedRepositoryLoaderTests.test.ts @@ -13,50 +13,16 @@ import { fetchShuttleDataSuccessfulResponse } from "../jsonSnapshots/fetchShuttleData/fetchShuttleDataSuccessfulResponse"; import { fetchEtaDataSuccessfulResponse } from "../jsonSnapshots/fetchEtaData/fetchEtaDataSuccessfulResponse"; - -/** - * Function to update behavior of the global `fetch` function. - * Note that the Passio GO API returns status code 200 for failed responses. - * @param obj - * @param status - */ -function updateGlobalFetchMockJson( - obj: any, - status: number = 200 -) { - // @ts-ignore - global.fetch = jest.fn(() => { - return Promise.resolve({ - json: () => Promise.resolve(obj), - status, - ok: status.toString().startsWith("2"), // 200-level codes are OK - }) - }) as jest.Mock; -} - -/** - * Reset the global fetch function mock's JSON to return an empty object. - * @param obj - */ -function resetGlobalFetchMockJson() { - updateGlobalFetchMockJson({}); -} +import { + resetGlobalFetchMockJson, + updateGlobalFetchMockJson, + updateGlobalFetchMockJsonToThrowSyntaxError +} from "../mockHelpers/fetchMockHelpers"; async function assertAsyncCallbackThrowsApiResponseError(callback: () => Promise) { await expect(callback).rejects.toThrow(ApiResponseError); } -function updateGlobalFetchMockJsonToThrowSyntaxError() { - // @ts-ignore - global.fetch = jest.fn(() => { - return Promise.resolve({ - json: () => Promise.reject(new SyntaxError("Unable to parse JSON")), - status: 200, - ok: true, - }) - }) as jest.Mock; -} - describe("ApiBasedRepositoryLoader", () => { let loader: ApiBasedRepositoryLoader; diff --git a/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts new file mode 100644 index 0000000..41d0011 --- /dev/null +++ b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts @@ -0,0 +1,13 @@ +import { beforeEach, describe } from "@jest/globals"; +import { TimedApiBasedRepositoryLoader } from "../../src/loaders/TimedApiBasedRepositoryLoader"; +import { UnoptimizedInMemoryRepository } from "../../src/repositories/UnoptimizedInMemoryRepository"; +import { resetGlobalFetchMockJson } from "../mockHelpers/fetchMockHelpers"; + +describe("TimedApiBasedRepositoryLoader", () => { + let loader: TimedApiBasedRepositoryLoader; + + beforeEach(() => { + loader = new TimedApiBasedRepositoryLoader(new UnoptimizedInMemoryRepository()); + resetGlobalFetchMockJson(); + }); +}); \ No newline at end of file diff --git a/test/mockHelpers/fetchMockHelpers.ts b/test/mockHelpers/fetchMockHelpers.ts new file mode 100644 index 0000000..f1994cb --- /dev/null +++ b/test/mockHelpers/fetchMockHelpers.ts @@ -0,0 +1,41 @@ +import { jest } from "@jest/globals"; + +/** + * Function to update behavior of the global `fetch` function. + * Note that the Passio GO API returns status code 200 for failed responses. + * @param obj + * @param status + */ +export function updateGlobalFetchMockJson( + obj: any, + status: number = 200 +) { + // @ts-ignore + global.fetch = jest.fn(() => { + return Promise.resolve({ + json: () => Promise.resolve(obj), + status, + ok: status.toString().startsWith("2"), // 200-level codes are OK + }) + }); +} + +/** + * Reset the global fetch function mock's JSON to return an empty object. + * @param obj + */ +export function resetGlobalFetchMockJson() { + updateGlobalFetchMockJson({}); +} + + +export function updateGlobalFetchMockJsonToThrowSyntaxError() { + // @ts-ignore + global.fetch = jest.fn(() => { + return Promise.resolve({ + json: () => Promise.reject(new SyntaxError("Unable to parse JSON")), + status: 200, + ok: true, + }) + }) as jest.Mock; +} From 200c5328b6e70d4050dc775564632e17c40fa0a1 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 22 Jan 2025 16:20:20 -0800 Subject: [PATCH 2/4] make timeout constant a class property --- src/loaders/TimedApiBasedRepositoryLoader.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/loaders/TimedApiBasedRepositoryLoader.ts b/src/loaders/TimedApiBasedRepositoryLoader.ts index 763c9fa..89fe34b 100644 --- a/src/loaders/TimedApiBasedRepositoryLoader.ts +++ b/src/loaders/TimedApiBasedRepositoryLoader.ts @@ -2,8 +2,6 @@ import { GetterSetterRepository } from "../repositories/GetterSetterRepository"; import { IEta, IRoute, IShuttle, IStop, ISystem } from "../entities/entities"; import { ApiBasedRepositoryLoader } from "./ApiBasedRepositoryLoader"; -const timeout = 10000; - // Ideas to break this into smaller pieces in the future: // Have one repository data loader running for each supported system // Each data loader independently updates data based on frequency of usage @@ -22,6 +20,8 @@ export class TimedApiBasedRepositoryLoader extends ApiBasedRepositoryLoader { private shouldBeRunning: boolean = false; private timer: any; + readonly timeout = 10000; + constructor( repository: GetterSetterRepository, ) { @@ -61,7 +61,6 @@ export class TimedApiBasedRepositoryLoader extends ApiBasedRepositoryLoader { console.error(e); } - this.timer = setTimeout(this.startFetchDataAndUpdate, timeout); + this.timer = setTimeout(this.startFetchDataAndUpdate, this.timeout); } - } \ No newline at end of file From 7007073602432067116f6609e979e8dfcd863550 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 22 Jan 2025 16:20:32 -0800 Subject: [PATCH 3/4] add test implementations for timed api based repository loader --- ...TimedApiBasedRepositoryLoaderTests.test.ts | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts index 41d0011..236af2d 100644 --- a/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts +++ b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts @@ -1,13 +1,70 @@ -import { beforeEach, describe } from "@jest/globals"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, jest } from "@jest/globals"; import { TimedApiBasedRepositoryLoader } from "../../src/loaders/TimedApiBasedRepositoryLoader"; -import { UnoptimizedInMemoryRepository } from "../../src/repositories/UnoptimizedInMemoryRepository"; import { resetGlobalFetchMockJson } from "../mockHelpers/fetchMockHelpers"; +import { GetterSetterRepository } from "../../src/repositories/GetterSetterRepository"; describe("TimedApiBasedRepositoryLoader", () => { + let repositoryMock: GetterSetterRepository; let loader: TimedApiBasedRepositoryLoader; + let spies: any; + + beforeAll(() => { + jest.useFakeTimers(); + jest.spyOn(global, "setTimeout"); + }); beforeEach(() => { - loader = new TimedApiBasedRepositoryLoader(new UnoptimizedInMemoryRepository()); resetGlobalFetchMockJson(); + + repositoryMock = { + clearSystemData: jest.fn(), + clearRouteData: jest.fn(), + clearStopData: jest.fn(), + clearShuttleData: jest.fn(), + clearEtaData: jest.fn(), + } as unknown as GetterSetterRepository; + + loader = new TimedApiBasedRepositoryLoader(repositoryMock); + + spies = { + fetchAndUpdateSystemData: jest.spyOn(loader, 'fetchAndUpdateSystemData'), + fetchAndUpdateRouteDataForExistingSystemsInRepository: jest.spyOn(loader, 'fetchAndUpdateRouteDataForExistingSystemsInRepository'), + fetchAndUpdateStopAndPolylineDataForRoutesInExistingSystemsInRepository: jest.spyOn(loader, 'fetchAndUpdateStopAndPolylineDataForRoutesInExistingSystemsInRepository'), + fetchAndUpdateShuttleDataForExistingSystemsInRepository: jest.spyOn(loader, 'fetchAndUpdateShuttleDataForExistingSystemsInRepository'), + fetchAndUpdateEtaDataForExistingStopsForSystemsInRepository: jest.spyOn(loader, 'fetchAndUpdateEtaDataForExistingStopsForSystemsInRepository') + }; + + Object.values(spies).forEach((spy: any) => { + spy.mockResolvedValue(undefined); + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + jest.clearAllTimers(); + }) + + describe("start", () => { + it("should update internal state, call data fetching methods, and start a timer", async () => { + await loader.start(); + expect(loader["shouldBeRunning"]).toBe(true); + + Object.values(repositoryMock).forEach((mockFn) => { + expect(mockFn).toHaveBeenCalled(); + }); + Object.values(spies).forEach((spy: any) => { + expect(spy).toHaveBeenCalled(); + }); + + expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), loader.timeout); + expect(loader.timeout).not.toBeUndefined(); + }); + }); + + describe("stop", () => { + it("should update internal state", async () => { + loader.stop(); + expect(loader['shouldBeRunning']).toBe(false); + }); }); }); \ No newline at end of file From 122dfd2f1b8ad5511f041f0e8bec97b4e5076646 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 22 Jan 2025 16:22:46 -0800 Subject: [PATCH 4/4] add additional test case for already running timer --- .../TimedApiBasedRepositoryLoaderTests.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts index 236af2d..ec37052 100644 --- a/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts +++ b/test/loaders/TimedApiBasedRepositoryLoaderTests.test.ts @@ -59,6 +59,18 @@ describe("TimedApiBasedRepositoryLoader", () => { expect(setTimeout).toHaveBeenCalledWith(expect.any(Function), loader.timeout); expect(loader.timeout).not.toBeUndefined(); }); + + it("does nothing if timer is already running", async () => { + await loader.start(); + await loader.start(); + + Object.values(repositoryMock).forEach((mockFn) => { + expect(mockFn).toHaveBeenCalledTimes(1); + }); + Object.values(spies).forEach((spy: any) => { + expect(spy).toHaveBeenCalledTimes(1); + }); + }); }); describe("stop", () => {