From ba5bba677890db49a4b75d9c389909b8c0b0a70f Mon Sep 17 00:00:00 2001 From: David de Kloet <122978264+dskloetd@users.noreply.github.com> Date: Wed, 8 Jan 2025 13:13:59 +0100 Subject: [PATCH] registerCleanupForTesting (#6114) # Motivation Our unit tests have always been quite brittle because we need to remember to clean up all global state between each test. Often tests fail when run in a different order because one test accidentally depends on state left behind by other tests. This has improved a lot now that we always [reset all stores](https://github.com/dfinity/nns-dapp/pull/5724) and [restore all mocks](https://github.com/dfinity/nns-dapp/pull/5788), but there are still other cleanups that need to be performed in tests. This PR proposes a generic way of registering a cleanup in the file that defines the thing that needs to be cleaned up, instead of in every test. As a proof of concept, it is then used to automatically call `resetMockedConstants();` in every test that (transitively) imports `$tests/utils/mockable-constants.test-utils.ts`. # Changes 1. Create an empty `registerCleanupForTesting` function which should be used to register cleanup functions. 2. In `frontend/vitest.setup.ts` mock `frontend/src/lib/utils/test-support.utils.ts` such that the registered cleanup functions are called before every test. # Tests Tests only # Todos - [ ] Add entry to changelog (if necessary). not necessary --- frontend/src/lib/utils/test-support.utils.ts | 5 +++++ .../lib/components/common/MenuItems.spec.ts | 2 -- .../lib/services/sns-neurons.services.spec.ts | 2 -- .../src/tests/lib/utils/ckbtc.utils.spec.ts | 9 +-------- .../utils/mockable-constants.test-utils.ts | 5 ++++- frontend/vitest.setup.ts | 18 +++++++++++++----- 6 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 frontend/src/lib/utils/test-support.utils.ts diff --git a/frontend/src/lib/utils/test-support.utils.ts b/frontend/src/lib/utils/test-support.utils.ts new file mode 100644 index 00000000000..a47a01f5d02 --- /dev/null +++ b/frontend/src/lib/utils/test-support.utils.ts @@ -0,0 +1,5 @@ +// The purpose of this function is to be mocked by tests. +// +// Production code should use this function to register cleanup functions that +// should be run before each test. +export const registerCleanupForTesting = (_cleanup: () => void) => {}; diff --git a/frontend/src/tests/lib/components/common/MenuItems.spec.ts b/frontend/src/tests/lib/components/common/MenuItems.spec.ts index dc789b3f2af..ebd73fa317c 100644 --- a/frontend/src/tests/lib/components/common/MenuItems.spec.ts +++ b/frontend/src/tests/lib/components/common/MenuItems.spec.ts @@ -12,7 +12,6 @@ import { principal } from "$tests/mocks/sns-projects.mock"; import { mockSnsProposal } from "$tests/mocks/sns-proposals.mock"; import { MenuItemsPo } from "$tests/page-objects/MenuItems.page-object"; import { JestPageObjectElement } from "$tests/page-objects/jest.page-object"; -import { resetMockedConstants } from "$tests/utils/mockable-constants.test-utils"; import { advanceTime, runResolvedPromises, @@ -64,7 +63,6 @@ describe("MenuItems", () => { }; beforeEach(() => { - resetMockedConstants(); menuStore.resetForTesting(); layoutMenuOpen.set(false); vi.useFakeTimers(); diff --git a/frontend/src/tests/lib/services/sns-neurons.services.spec.ts b/frontend/src/tests/lib/services/sns-neurons.services.spec.ts index 121fcd89ba3..8320ba809fb 100644 --- a/frontend/src/tests/lib/services/sns-neurons.services.spec.ts +++ b/frontend/src/tests/lib/services/sns-neurons.services.spec.ts @@ -35,7 +35,6 @@ import { mockSnsNeuron, } from "$tests/mocks/sns-neurons.mock"; import { mockSnsToken, mockTokenStore } from "$tests/mocks/sns-projects.mock"; -import { resetMockedConstants } from "$tests/utils/mockable-constants.test-utils"; import { resetSnsProjects, setSnsProjects } from "$tests/utils/sns.test-utils"; import { toastsStore } from "@dfinity/gix-components"; import { decodeIcrcAccount } from "@dfinity/ledger-icrc"; @@ -93,7 +92,6 @@ describe("sns-neurons-services", () => { beforeEach(() => { resetIdentity(); - resetMockedConstants(); resetSnsProjects(); vi.spyOn(console, "error").mockReturnValue(undefined); }); diff --git a/frontend/src/tests/lib/utils/ckbtc.utils.spec.ts b/frontend/src/tests/lib/utils/ckbtc.utils.spec.ts index d9ad379529b..422c75d6835 100644 --- a/frontend/src/tests/lib/utils/ckbtc.utils.spec.ts +++ b/frontend/src/tests/lib/utils/ckbtc.utils.spec.ts @@ -7,10 +7,7 @@ import { mockCkBTCToken } from "$tests/mocks/ckbtc-accounts.mock"; import { mockCkBTCMinterInfo } from "$tests/mocks/ckbtc-minter.mock"; import en from "$tests/mocks/i18n.mock"; import { mockMainAccount } from "$tests/mocks/icp-accounts.store.mock"; -import { - mockedConstants, - resetMockedConstants, -} from "$tests/utils/mockable-constants.test-utils"; +import { mockedConstants } from "$tests/utils/mockable-constants.test-utils"; describe("ckbtc.utils", () => { const RETRIEVE_BTC_MIN_AMOUNT = 100_000n; @@ -29,10 +26,6 @@ describe("ckbtc.utils", () => { }, }; - beforeEach(() => { - resetMockedConstants(); - }); - it("should not throw error", () => { expect(() => assertCkBTCUserInputAmount({ diff --git a/frontend/src/tests/utils/mockable-constants.test-utils.ts b/frontend/src/tests/utils/mockable-constants.test-utils.ts index fd290a1982c..ecc1e96051f 100644 --- a/frontend/src/tests/utils/mockable-constants.test-utils.ts +++ b/frontend/src/tests/utils/mockable-constants.test-utils.ts @@ -2,6 +2,7 @@ // frontend/src/lib/constants/mockable.constants.ts import type * as mockableConstants from "$lib/constants/mockable.constants"; +import { registerCleanupForTesting } from "$lib/utils/test-support.utils"; type MockableConstantsKey = keyof typeof mockableConstants; type MockableConstants = { [K in MockableConstantsKey]: unknown }; @@ -19,7 +20,7 @@ export const mockedConstants: MockableConstants = { notForceCallStrategy: undefined, }; -export const resetMockedConstants = () => { +const resetMockedConstants = () => { Object.keys(mockedConstants).forEach((key) => { mockedConstants[key] = defaultTestConstants[key]; }); @@ -29,3 +30,5 @@ export const setDefaultTestConstants = (constants: MockableConstants) => { defaultTestConstants = constants; resetMockedConstants(); }; + +registerCleanupForTesting(resetMockedConstants); diff --git a/frontend/vitest.setup.ts b/frontend/vitest.setup.ts index 61cf5c53433..60403b642ca 100644 --- a/frontend/vitest.setup.ts +++ b/frontend/vitest.setup.ts @@ -33,26 +33,34 @@ beforeEach(() => { vi.unstubAllGlobals(); }); -// Reset every store before each test. -const resetStoreFunctions = vi.hoisted(() => { +const cleanupFunctions = vi.hoisted(() => { return []; }); +// Reset every store before each test. vi.mock("svelte/store", async (importOriginal) => { const svelteStoreModule = await importOriginal(); return { ...svelteStoreModule, writable: (initialValue, ...otherArgs) => { const store = svelteStoreModule.writable(initialValue, ...otherArgs); - resetStoreFunctions.push(() => store.set(initialValue)); + cleanupFunctions.push(() => store.set(initialValue)); return store; }, }; }); +vi.mock("$lib/utils/test-support.utils", async () => { + return { + registerCleanupForTesting: (cleanup: () => void) => { + cleanupFunctions.push(cleanup); + }, + }; +}); + beforeEach(() => { - for (const reset of resetStoreFunctions) { - reset(); + for (const cleanup of cleanupFunctions) { + cleanup(); } });