Skip to content

Commit 7148559

Browse files
committed
docs: add TESTING-STANDARDS.md
Adds a new standards doc for front end component testing. Update rewritten suites that weren't aligning.
1 parent d07d445 commit 7148559

18 files changed

Lines changed: 355 additions & 43 deletions
Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
# Frontend Test Standards
2+
3+
This document is the standard for how test files in this codebase should
4+
be structured going forward. New and refactored specs should follow it.
5+
It isn't exhaustive -- add to it as new patterns get established.
6+
7+
## Mounting utilities
8+
9+
Two shared entry points live in `tests/`. Pick based on what the
10+
component under test actually needs:
11+
12+
| Utility | Use when the component... |
13+
|---|---|
14+
| `mountComponent` | Establishes its own `<Form>` context internally, or needs no vee-validate form context at all -- just Pinia / provide / stubs. This is the default. |
15+
| `mountWithFormContext` | **Assumes an ancestor** already provides vee-validate's form context (a field/section component that doesn't render its own `<Form>`). |
16+
17+
`mountWithFormContext` mounts the component inside a synthetic harness
18+
that calls `useForm()` on its behalf. Reach for it only when the
19+
component genuinely has no other way to get a form context in the test
20+
-- using it on a component that owns its own `<Form>` creates a nested,
21+
shadowed context instead of an error, which is a quiet bug to track down
22+
later. When in doubt, start with `mountComponent`; only switch to
23+
`mountWithFormContext` if the component throws on a missing form
24+
context (e.g. `useIsFormDirty`/`useFormValues`/`useField` with no
25+
provider).
26+
27+
## Router mocking
28+
29+
`tests/mockRouter.ts` exports a shared `mockRouter` (`{ push, replace }`)
30+
and `resetMockRouter()`. It's deliberately separate from the mounting
31+
utilities above -- routing is an unrelated concern, not something every
32+
mounted component needs. Use it in any spec that needs to assert on
33+
navigation:
34+
35+
```ts
36+
import { mockRouter, resetMockRouter } from '../utils/mockRouter';
37+
38+
// vi.mock is hoisted, so it can't reference `mockRouter` directly from the
39+
// shared util at declaration time -- but it CAN return the same object
40+
// reference at call time, which is all that matters for spies.
41+
vi.mock('vue-router', () => ({
42+
useRouter: () => mockRouter,
43+
useRoute: () => ({ params: {}, query: {} })
44+
}));
45+
46+
beforeEach(() => {
47+
resetMockRouter();
48+
});
49+
50+
// later, in a test:
51+
expect(mockRouter.replace).toHaveBeenCalledWith({ name: RouteName.EXT_GENERAL });
52+
```
53+
54+
If a spec needs `useRoute()` to return different query params per test
55+
(rather than a fixed object), define its own local `vi.fn()` wrapper
56+
around it instead -- `mockRouter` only covers `useRouter()`'s
57+
`push`/`replace`, since that's the part every spec asserting on
58+
navigation needs the same way:
59+
60+
```ts
61+
// `useRoute` is a vi.fn() (rather than returning a fixed object) so
62+
// individual tests can override its return value to exercise the
63+
// query-string-driven pagination logic in the component.
64+
const mockUseRoute = vi.fn(() => ({ query: {} as Record<string, string> }));
65+
66+
vi.mock('vue-router', () => ({
67+
useRoute: () => mockUseRoute(),
68+
useRouter: () => mockRouter
69+
}));
70+
71+
// later, in a test:
72+
it('reads rows/sort/page from the query string while on the projects tab', () => {
73+
mockUseRoute.mockReturnValue({
74+
query: { tab: '0', rows: '20', order: '1', field: 'projectName', page: '2' }
75+
});
76+
77+
const { wrapper } = mountProjectListNavigator();
78+
// ...assert on the component's reaction to that query...
79+
});
80+
```
81+
82+
(from `ProjectListNavigator.spec.ts`.)
83+
84+
## Pinia store state: seed it, don't mock it
85+
86+
Drive a component's store-dependent behavior by seeding real state via
87+
`piniaState`, not by `vi.mock`-ing the store module. Mocking a store
88+
replaces it wholesale, which hides how the component and the store's
89+
getters actually interact -- a getter that derives from two state fields,
90+
or a computed that depends on another getter, can silently drift out of
91+
sync with the real implementation without any test failing, since the
92+
mock just returns whatever you told it to. Seeding the state a getter
93+
derives from exercises the real computed logic and catches that drift:
94+
95+
```ts
96+
mountComponent(Foo, {
97+
piniaState: { form: { formType: FormType.NEW, formState: FormState.UNLOCKED } }
98+
});
99+
```
100+
101+
The only sanctioned exception is `useAuthNStore`, described next -- and
102+
even that one is deliberately narrow: it mocks a single leaf module for a
103+
documented, unavoidable reason, not a general pattern to reach for when
104+
seeding state feels inconvenient.
105+
106+
## Authn store mocking (exception to "seed it, don't mock it")
107+
108+
`useAuthNStore` (`src/store/authnStore.ts`) is a Pinia *setup* store. Even
109+
under `createTestingPinia`, only the store's returned actions get
110+
auto-stubbed -- the setup() body itself still runs for real, which
111+
unconditionally constructs a real `AuthService` / `oidc-client-ts`
112+
`UserManager`. With `monitorSession` on by default, that spins up a
113+
`SessionMonitor` whose constructor fires an unawaited `getUser()`, logging
114+
`"[UserManager] getUser: user not found in storage"` to the console in
115+
every spec that touches the real store -- there's no `piniaState` to seed
116+
around this, since it happens before any state is read.
117+
118+
Because of that, specs depending on `useAuthNStore` are a deliberate
119+
exception to the "seed it, don't mock it" rule above:
120+
use `tests/mockAuthNStore.ts` instead.
121+
122+
```ts
123+
import { mockAuthNStore, resetMockAuthNStore } from '../../../mockAuthNStore';
124+
125+
vi.mock('@/store/authnStore', () => ({
126+
default: () => mockAuthNStore,
127+
useAuthNStore: () => mockAuthNStore
128+
}));
129+
130+
beforeEach(() => {
131+
resetMockAuthNStore();
132+
});
133+
134+
// later, in a test:
135+
mockAuthNStore.getIsAuthenticated.value = true;
136+
mockAuthNStore.getProfile.value = testProfile;
137+
```
138+
139+
Mock the leaf module `@/store/authnStore`, not the `@/store` barrel --
140+
that keeps every other store re-exported from `@/store` real for specs
141+
that need both. This doesn't apply to any other store; every other store
142+
should still be driven via real `piniaState` per the rule above.
143+
144+
## Local mount factory
145+
146+
Every spec defines exactly one local factory that wraps the appropriate
147+
shared utility above with that file's own defaults (props, piniaState,
148+
stubs, provide, etc). Name it **`mount` + the component's name** --
149+
e.g. `mountCancelButton`, `mountNaturalDisasterCard`,
150+
`mountProjectListNavigator`. The pattern is fixed; the name itself
151+
isn't -- every spec should be immediately recognizable as
152+
"`mount` + whatever this file is testing."
153+
154+
```ts
155+
function mountCancelButton(options: { editable?: boolean } = {}) {
156+
const onClicked = vi.fn();
157+
158+
const { wrapper } = mountWithFormContext(CancelButton, {
159+
fields: ['testField'],
160+
componentProps: { ...options, onClicked }
161+
});
162+
163+
return { wrapper, onClicked };
164+
}
165+
```
166+
167+
(from `CancelButton.spec.ts` -- see also `ContactCardIntakeForm.spec.ts` for
168+
the same pattern with real `piniaState` and multiple returned values.)
169+
170+
Rules for the local mount factory:
171+
172+
- **Always takes an options object with defaults**, never positional
173+
arguments -- `mountCancelButton({ editable: false })`, not
174+
`mountCancelButton(false)`.
175+
- **Always returns an object**, even if it only contains `wrapper`. Never
176+
return a bare wrapper. This keeps every call site's destructuring
177+
(`const { wrapper } = mountCancelButton()`) valid regardless of which
178+
spec you copy it from.
179+
- Lives near the top of the file, immediately after fixtures (see layout
180+
below), so it's the first thing a reader sees before the tests
181+
themselves.
182+
183+
## File layout
184+
185+
Every spec follows the same section order, marked with a one-line
186+
comment banner (not a boxed/bordered one -- keep it light):
187+
188+
```ts
189+
// Mocks
190+
191+
vi.mock('vue-router', () => ({ ... }));
192+
193+
// Fixtures
194+
195+
const testFoo = { ... };
196+
197+
// Mount
198+
199+
function mountComponentName(options = {}) { ... }
200+
201+
beforeEach(() => { ... });
202+
203+
// Tests
204+
205+
describe('ComponentName', () => {
206+
describe('rendering', () => { ... });
207+
describe('validation', () => { ... });
208+
describe('user interaction', () => { ... });
209+
});
210+
```
211+
212+
Notes:
213+
214+
- **Mocks always come first, unconditionally.** `vi.mock` calls are
215+
hoisted by Vitest regardless of where they're written in the file, so
216+
placing them anywhere else misrepresents execution order to a reader.
217+
- **Fixtures** are named, reusable, known-good pieces of test data (e.g.
218+
`testProject`, `sampleContact`) -- pulled out of individual `it()`
219+
blocks so multiple tests share the exact same input, one place to
220+
update when a type shape changes, and tests read as behavior rather
221+
than data setup.
222+
- Keep every section present even when it's nearly empty (e.g. a
223+
component with barely any fixtures). Consistency across the suite
224+
matters more than saving a few lines in short files.
225+
- **`// Tests` goes after `beforeEach`/`afterEach`, immediately before
226+
the root `describe` block** -- not before them. The lifecycle hooks
227+
are setup tightly coupled to the mount factory (often resetting mocks
228+
it depends on), so they stay grouped under `// Mount`; `// Tests`
229+
marks where the actual test bodies begin. Every section gets a banner,
230+
including this last one -- consistency across the whole file matters
231+
more than a banner being technically redundant with the `describe`
232+
title beneath it.
233+
- Within the root `describe`, group nested `describe`s by concern, not
234+
by method/prop name. Prefer a small, consistent vocabulary across the
235+
whole codebase: `rendering`, `validation`, `user interaction`, or a
236+
specific named behavior (e.g. `form error reporting`). Don't let each
237+
file invent its own taxonomy.
238+
239+
## Known gotchas
240+
241+
**Never `vi.spyOn` a Pinia getter -- drive the real state instead.**
242+
A Pinia getter is a `computed()` ref stored as a plain property value,
243+
not a real accessor descriptor. `vi.spyOn(store, 'someGetter', 'get')`
244+
does not reliably attach, and direct assignment
245+
(`store.someGetter = value`) silently no-ops (computed refs are
246+
read-only; Vue just logs a dev warning). This applies **even when the
247+
getter looks like a callable method** -- e.g. `authzStore.can(...)` is
248+
actually `computed(() => (initiative, resource, action) => boolean)`,
249+
not a plain action, despite being called like one. The only reliable
250+
fix is to seed the real state the getter derives from, via
251+
`piniaState`, e.g.:
252+
253+
```ts
254+
mountComponent(Foo, {
255+
piniaState: { form: { formType: FormType.NEW, formState: FormState.UNLOCKED } }
256+
});
257+
```
258+
259+
Plain **actions** (`setFormError`, `setAuthorizationContext`, etc.) are
260+
ordinary functions and spy normally -- `createTestingPinia` stubs them
261+
automatically by default, or `vi.spyOn(store, 'someAction')` works fine
262+
if you need a custom implementation. The distinction is getters vs.
263+
actions, not "looks like a function call" vs. "looks like a property."
264+
265+
**`.props('key')` can fail to typecheck on a union of wrapper types.**
266+
`VueWrapper#props()` is overloaded (`props(): Props` and
267+
`props<K>(key: K): Props[K]`). When you call `.props('key')` on a value
268+
whose type is a union of *different* generic instantiations of
269+
`VueWrapper` (e.g. iterating `[...findAllComponents(A), ...findAllComponents(B)]`),
270+
TypeScript can't merge the overload sets across the union and falls back
271+
to the zero-arg overload only. Use `.props().key` in that situation --
272+
it works because the zero-arg overload is common to every member of the
273+
union. A single, non-unioned `findComponent(X).props('key')` is fine as-is.
274+
275+
**A provided/injected value that's read via template auto-unwrap must be
276+
a genuine `ref`/`computed`, not a plain object shaped like one.**
277+
Vue's template auto-unwrap only triggers for real reactive refs (objects
278+
carrying the internal ref marker), not a plain `{ value: x }` object that
279+
merely looks ref-shaped. If the real app provides a value via
280+
`provide(someKey, computed(() => ...))` and the component reads it
281+
*inside its template* with no `.value`, the test's `provide` option must
282+
use an actual `ref()`/`computed()` too:
283+
284+
```ts
285+
import { ref } from 'vue';
286+
287+
provide: {
288+
[someKey]: ref('some-value') // not { value: 'some-value' }
289+
}
290+
```
291+
292+
If the component instead reads the injected value via explicit `.value`
293+
in `<script setup>` logic (not the template), a plain `{ value: x }`
294+
object works fine either way, since that's just manual property access,
295+
not Vue's unwrap magic. Check the actual call site before assuming which
296+
one you need.

frontend/tests/mockAuthNStore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* Shared authn store mock -- a deliberate exception to the "seed real Pinia
3-
* state, don't mock the store" rule in TESTING_STANDARDS.md. `useAuthNStore`
3+
* state, don't mock the store" rule in TESTING-STANDARDS.md. `useAuthNStore`
44
* is a Pinia *setup* store (src/store/authnStore.ts). Even under
55
* `createTestingPinia`, only the store's returned actions get auto-stubbed --
66
* the setup() body itself still runs for real, unconditionally constructing

frontend/tests/mountComponent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export interface MountComponentOptions extends PiniaOptions {
4848
props?: Record<string, unknown>;
4949
/** Component stubs, merged with any caller-specific defaults */
5050
stubs?: Stubs;
51-
/** Injection keys/values, e.g. { [someKey]: { value: 'x' } } */
51+
/** Injection keys/values, e.g. { [someKey]: ref('x') } */
5252
provide?: Record<string | symbol, unknown>;
5353
/** Extra global plugins beyond the always-installed PrimeVue/i18n/etc. */
5454
plugins?: Plugin[];

frontend/tests/unit/components/authorization/AuthorizationForm.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { PermitNeeded } from '@/utils/enums/permit';
1212
import { projectRouteNameKey, projectServiceKey } from '@/utils/keys';
1313

1414
import type { GetPeachSummaryResponse, Permit, PermitTracking, PermitType, User } from '@/types';
15+
import { ref } from 'vue';
1516

1617
// Mock Services
1718
vi.mock('@/services', () => ({
@@ -109,8 +110,8 @@ const wrapperSettings = (props = {}, isPeachEnabled = true) => ({
109110
PrimeVue
110111
],
111112
provide: {
112-
[projectRouteNameKey as symbol]: { value: 'housing-project' },
113-
[projectServiceKey as symbol]: { value: { emailConfirmation: vi.fn() } }
113+
[projectRouteNameKey as symbol]: ref('housing-project'),
114+
[projectServiceKey as symbol]: ref({ foo: vi.fn() })
114115
},
115116
stubs: {
116117
'font-awesome-icon': true,

frontend/tests/unit/components/contact/ContactHistoryList.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type { Enquiry, HousingProject } from '@/types';
1414
import { BasicResponse } from '@/utils/enums/application';
1515
import { NumResidentialUnits } from '@/utils/enums/housing';
1616
import { enquiryRouteNameKey, projectRouteNameKey } from '@/utils/keys';
17+
import { ref } from 'vue';
1718

1819
vi.mock('vue-i18n', () => ({
1920
useI18n: () => ({
@@ -116,8 +117,8 @@ const wrapperSettings = (loading = false, contactsHistory = testHistory, assigne
116117
PrimeVue
117118
],
118119
provide: {
119-
[projectRouteNameKey as symbol]: { value: 'project-route-name' },
120-
[enquiryRouteNameKey as symbol]: { value: 'enquiry-route-name' }
120+
[projectRouteNameKey as symbol]: ref('project-route-name'),
121+
[enquiryRouteNameKey as symbol]: ref('enquiry-route-name')
121122
},
122123
stubs: ['Spinner', 'DataTable', 'Column', 'router-link']
123124
}

frontend/tests/unit/components/contact/ContactPage.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import ContactPage from '@/components/contact/ContactPage.vue';
99
import { contactService, housingProjectService, enquiryService, userService } from '@/services';
1010
import { ContactPreference, ProjectRelationship } from '@/utils/enums/projectCommon';
1111
import { contactRouteNameKey, projectServiceKey } from '@/utils/keys';
12+
import { ref } from 'vue';
1213

1314
vi.mock('vue-i18n', () => ({
1415
useI18n: () => ({
@@ -59,8 +60,8 @@ const wrapperSettings = () => ({
5960
ToastService
6061
],
6162
provide: {
62-
[contactRouteNameKey as symbol]: { value: 'route-name' },
63-
[projectServiceKey as symbol]: { value: { foo: vi.fn() } }
63+
[contactRouteNameKey as symbol]: ref('route-name'),
64+
[projectServiceKey as symbol]: ref({ foo: vi.fn() })
6465
},
6566
stubs: [
6667
'Button',

frontend/tests/unit/components/enquiry/EnquiryCard.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ApplicationStatus, EnquirySubmittedMethod, SubmissionType } from '@/uti
1010

1111
import type { Enquiry, User } from '@/types';
1212
import { projectEnquiryRouteNameKey } from '@/utils/keys';
13+
import { ref } from 'vue';
1314

1415
const listUsersSpy = vi.spyOn(userService, 'listUsers');
1516

@@ -54,7 +55,7 @@ const wrapperSettings = (testEnquiryProp = testEnquiry) => ({
5455
ToastService
5556
],
5657
provide: {
57-
[projectEnquiryRouteNameKey as symbol]: { value: 'route-name' }
58+
[projectEnquiryRouteNameKey as symbol]: ref('route-name')
5859
},
5960
stubs: ['font-awesome-icon', 'router-link']
6061
}

0 commit comments

Comments
 (0)