Skip to content

Commit

Permalink
registerCleanupForTesting (#6114)
Browse files Browse the repository at this point in the history
# 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](#5724) and [restore all
mocks](#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
  • Loading branch information
dskloetd authored Jan 8, 2025
1 parent 47eccac commit ba5bba6
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 18 deletions.
5 changes: 5 additions & 0 deletions frontend/src/lib/utils/test-support.utils.ts
Original file line number Diff line number Diff line change
@@ -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) => {};
2 changes: 0 additions & 2 deletions frontend/src/tests/lib/components/common/MenuItems.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -64,7 +63,6 @@ describe("MenuItems", () => {
};

beforeEach(() => {
resetMockedConstants();
menuStore.resetForTesting();
layoutMenuOpen.set(false);
vi.useFakeTimers();
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/tests/lib/services/sns-neurons.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -93,7 +92,6 @@ describe("sns-neurons-services", () => {

beforeEach(() => {
resetIdentity();
resetMockedConstants();
resetSnsProjects();
vi.spyOn(console, "error").mockReturnValue(undefined);
});
Expand Down
9 changes: 1 addition & 8 deletions frontend/src/tests/lib/utils/ckbtc.utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,10 +26,6 @@ describe("ckbtc.utils", () => {
},
};

beforeEach(() => {
resetMockedConstants();
});

it("should not throw error", () => {
expect(() =>
assertCkBTCUserInputAmount({
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/tests/utils/mockable-constants.test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -19,7 +20,7 @@ export const mockedConstants: MockableConstants = {
notForceCallStrategy: undefined,
};

export const resetMockedConstants = () => {
const resetMockedConstants = () => {
Object.keys(mockedConstants).forEach((key) => {
mockedConstants[key] = defaultTestConstants[key];
});
Expand All @@ -29,3 +30,5 @@ export const setDefaultTestConstants = (constants: MockableConstants) => {
defaultTestConstants = constants;
resetMockedConstants();
};

registerCleanupForTesting(resetMockedConstants);
18 changes: 13 additions & 5 deletions frontend/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: <T>(initialValue, ...otherArgs) => {
const store = svelteStoreModule.writable<T>(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();
}
});

Expand Down

0 comments on commit ba5bba6

Please sign in to comment.