Skip to content

Commit 6c9e7b7

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 6c9e7b7

4 files changed

Lines changed: 290 additions & 14 deletions

File tree

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
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+
## Pinia store state: seed it, don't mock it
61+
62+
Drive a component's store-dependent behavior by seeding real state via
63+
`piniaState`, not by `vi.mock`-ing the store module. Mocking a store
64+
replaces it wholesale, which hides how the component and the store's
65+
getters actually interact -- a getter that derives from two state fields,
66+
or a computed that depends on another getter, can silently drift out of
67+
sync with the real implementation without any test failing, since the
68+
mock just returns whatever you told it to. Seeding the state a getter
69+
derives from exercises the real computed logic and catches that drift:
70+
71+
```ts
72+
mountComponent(Foo, {
73+
piniaState: { form: { formType: FormType.NEW, formState: FormState.UNLOCKED } }
74+
});
75+
```
76+
77+
The only sanctioned exception is `useAuthNStore`, described next -- and
78+
even that one is deliberately narrow: it mocks a single leaf module for a
79+
documented, unavoidable reason, not a general pattern to reach for when
80+
seeding state feels inconvenient.
81+
82+
## Authn store mocking (exception to "seed real Pinia state")
83+
84+
`useAuthNStore` (`src/store/authnStore.ts`) is a Pinia *setup* store. Even
85+
under `createTestingPinia`, only the store's returned actions get
86+
auto-stubbed -- the setup() body itself still runs for real, which
87+
unconditionally constructs a real `AuthService` / `oidc-client-ts`
88+
`UserManager`. With `monitorSession` on by default, that spins up a
89+
`SessionMonitor` whose constructor fires an unawaited `getUser()`, logging
90+
`"[UserManager] getUser: user not found in storage"` to the console in
91+
every spec that touches the real store -- there's no `piniaState` to seed
92+
around this, since it happens before any state is read.
93+
94+
Because of that, specs depending on `useAuthNStore` are a deliberate
95+
exception to the "seed real Pinia state, don't mock the store" rule above:
96+
use `tests/mockAuthNStore.ts` instead.
97+
98+
```ts
99+
import { mockAuthNStore, resetMockAuthNStore } from '../../../mockAuthNStore';
100+
101+
vi.mock('@/store/authnStore', () => ({
102+
default: () => mockAuthNStore,
103+
useAuthNStore: () => mockAuthNStore
104+
}));
105+
106+
beforeEach(() => {
107+
resetMockAuthNStore();
108+
});
109+
110+
// later, in a test:
111+
mockAuthNStore.getIsAuthenticated.value = true;
112+
mockAuthNStore.getProfile.value = testProfile;
113+
```
114+
115+
Mock the leaf module `@/store/authnStore`, not the `@/store` barrel --
116+
that keeps every other store re-exported from `@/store` real for specs
117+
that need both. This doesn't apply to any other store; every other store
118+
should still be driven via real `piniaState` per the rule above.
119+
120+
## Local mount factory
121+
122+
Every spec defines exactly one local factory that wraps the appropriate
123+
shared utility above with that file's own defaults (props, piniaState,
124+
stubs, provide, etc). Name it **`mount` + the component's name** --
125+
e.g. `mountCancelButton`, `mountNaturalDisasterCard`,
126+
`mountProjectListNavigator`. The pattern is fixed; the name itself
127+
isn't -- every spec should be immediately recognizable as
128+
"`mount` + whatever this file is testing."
129+
130+
```ts
131+
function mountCancelButton(options: { editable?: boolean } = {}) {
132+
const onClicked = vi.fn();
133+
134+
const { wrapper } = mountWithFormContext(CancelButton, {
135+
fields: ['testField'],
136+
componentProps: { ...options, onClicked }
137+
});
138+
139+
return { wrapper, onClicked };
140+
}
141+
```
142+
143+
(from `CancelButton.spec.ts` -- see also `ContactCardIntakeForm.spec.ts` for
144+
the same pattern with real `piniaState` and multiple returned values.)
145+
146+
Rules for the local mount factory:
147+
148+
- **Always takes an options object with defaults**, never positional
149+
arguments -- `mountCancelButton({ editable: false })`, not
150+
`mountCancelButton(false)`.
151+
- **Always returns an object**, even if it only contains `wrapper`. Never
152+
return a bare wrapper. This keeps every call site's destructuring
153+
(`const { wrapper } = mountCancelButton()`) valid regardless of which
154+
spec you copy it from.
155+
- Lives near the top of the file, immediately after fixtures (see layout
156+
below), so it's the first thing a reader sees before the tests
157+
themselves.
158+
159+
## File layout
160+
161+
Every spec follows the same section order, marked with a one-line
162+
comment banner (not a boxed/bordered one -- keep it light):
163+
164+
```ts
165+
// Mocks
166+
167+
vi.mock('vue-router', () => ({ ... }));
168+
169+
// Fixtures
170+
171+
const testFoo = { ... };
172+
173+
// Mount
174+
175+
function mountComponentName(options = {}) { ... }
176+
177+
beforeEach(() => { ... });
178+
179+
// Tests
180+
181+
describe('ComponentName', () => {
182+
describe('rendering', () => { ... });
183+
describe('validation', () => { ... });
184+
describe('user interaction', () => { ... });
185+
});
186+
```
187+
188+
Notes:
189+
190+
- **Mocks always come first, unconditionally.** `vi.mock` calls are
191+
hoisted by Vitest regardless of where they're written in the file, so
192+
placing them anywhere else misrepresents execution order to a reader.
193+
- **Fixtures** are named, reusable, known-good pieces of test data (e.g.
194+
`testProject`, `sampleContact`) -- pulled out of individual `it()`
195+
blocks so multiple tests share the exact same input, one place to
196+
update when a type shape changes, and tests read as behavior rather
197+
than data setup.
198+
- Keep every section present even when it's nearly empty (e.g. a
199+
component with barely any fixtures). Consistency across the suite
200+
matters more than saving a few lines in short files.
201+
- **`// Tests` goes after `beforeEach`/`afterEach`, immediately before
202+
the root `describe` block** -- not before them. The lifecycle hooks
203+
are setup tightly coupled to the mount factory (often resetting mocks
204+
it depends on), so they stay grouped under `// Mount`; `// Tests`
205+
marks where the actual test bodies begin. Every section gets a banner,
206+
including this last one -- consistency across the whole file matters
207+
more than a banner being technically redundant with the `describe`
208+
title beneath it.
209+
- Within the root `describe`, group nested `describe`s by concern, not
210+
by method/prop name. Prefer a small, consistent vocabulary across the
211+
whole codebase: `rendering`, `validation`, `user interaction`, or a
212+
specific named behavior (e.g. `form error reporting`). Don't let each
213+
file invent its own taxonomy.
214+
215+
## Known gotchas
216+
217+
**Never `vi.spyOn` a Pinia getter -- drive the real state instead.**
218+
A Pinia getter is a `computed()` ref stored as a plain property value,
219+
not a real accessor descriptor. `vi.spyOn(store, 'someGetter', 'get')`
220+
does not reliably attach, and direct assignment
221+
(`store.someGetter = value`) silently no-ops (computed refs are
222+
read-only; Vue just logs a dev warning). This applies **even when the
223+
getter looks like a callable method** -- e.g. `authzStore.can(...)` is
224+
actually `computed(() => (initiative, resource, action) => boolean)`,
225+
not a plain action, despite being called like one. The only reliable
226+
fix is to seed the real state the getter derives from, via
227+
`piniaState`, e.g.:
228+
229+
```ts
230+
mountComponent(Foo, {
231+
piniaState: { form: { formType: FormType.NEW, formState: FormState.UNLOCKED } }
232+
});
233+
```
234+
235+
Plain **actions** (`setFormError`, `setAuthorizationContext`, etc.) are
236+
ordinary functions and spy normally -- `createTestingPinia` stubs them
237+
automatically by default, or `vi.spyOn(store, 'someAction')` works fine
238+
if you need a custom implementation. The distinction is getters vs.
239+
actions, not "looks like a function call" vs. "looks like a property."
240+
241+
**`.props('key')` can fail to typecheck on a union of wrapper types.**
242+
`VueWrapper#props()` is overloaded (`props(): Props` and
243+
`props<K>(key: K): Props[K]`). When you call `.props('key')` on a value
244+
whose type is a union of *different* generic instantiations of
245+
`VueWrapper` (e.g. iterating `[...findAllComponents(A), ...findAllComponents(B)]`),
246+
TypeScript can't merge the overload sets across the union and falls back
247+
to the zero-arg overload only. Use `.props().key` in that situation --
248+
it works because the zero-arg overload is common to every member of the
249+
union. A single, non-unioned `findComponent(X).props('key')` is fine as-is.
250+
251+
**A provided/injected value that's read via template auto-unwrap must be
252+
a genuine `ref`/`computed`, not a plain object shaped like one.**
253+
Vue's template auto-unwrap only triggers for real reactive refs (objects
254+
carrying the internal ref marker), not a plain `{ value: x }` object that
255+
merely looks ref-shaped. If the real app provides a value via
256+
`provide(someKey, computed(() => ...))` and the component reads it
257+
*inside its template* with no `.value`, the test's `provide` option must
258+
use an actual `ref()`/`computed()` too:
259+
260+
```ts
261+
import { ref } from 'vue';
262+
263+
provide: {
264+
[someKey]: ref('some-value') // not { value: 'some-value' }
265+
}
266+
```
267+
268+
If the component instead reads the injected value via explicit `.value`
269+
in `<script setup>` logic (not the template), a plain `{ value: x }`
270+
object works fine either way, since that's just manual property access,
271+
not Vue's unwrap magic. Check the actual call site before assuming which
272+
one you need.

frontend/tests/unit/components/form/FormAutosave.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ const DELAY = 500;
1010

1111
// Mount
1212

13-
function mountFormAutoSave(callback = vi.fn().mockResolvedValue(undefined), delay: number = DELAY) {
13+
function mountFormAutoSave(options: { callback?: ReturnType<typeof vi.fn>; delay?: number } = {}) {
14+
const { callback = vi.fn().mockResolvedValue(undefined), delay = DELAY } = options;
15+
1416
const { wrapper } = mountWithFormContext(FormAutoSave, {
1517
fields: ['testField'],
1618
componentProps: { callback, delay }

frontend/tests/unit/components/form/common/ContactCardNavForm.spec.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ const contactFieldNames = [
1919

2020
// Mount
2121

22-
function mountContactCardNavForm(formValues: DeepPartial<HousingFormSchemaType> = {}) {
22+
function mountContactCardNavForm(options: { formValues?: DeepPartial<HousingFormSchemaType> } = {}) {
23+
const { formValues = {} } = options;
24+
2325
const { wrapper } = mountWithFormContext(ContactCardNavForm, {
2426
componentProps: { formValues }
2527
});
@@ -38,7 +40,7 @@ describe('ContactCardNavForm', () => {
3840

3941
describe('when there is no selected contact', () => {
4042
it('renders neither the manual-contact warning nor the contact fields', () => {
41-
const { wrapper } = mountContactCardNavForm({ contact: undefined });
43+
const { wrapper } = mountContactCardNavForm({ formValues: { contact: undefined } });
4244

4345
expect(wrapper.find('.p-message').exists()).toBe(false);
4446
expect(wrapper.findComponent(InputText).exists()).toBe(false);
@@ -57,7 +59,7 @@ describe('ContactCardNavForm', () => {
5759
} as unknown as DeepPartial<HousingFormSchemaType>;
5860

5961
it('renders all six contact fields, each disabled', () => {
60-
const { wrapper } = mountContactCardNavForm(selectedContact);
62+
const { wrapper } = mountContactCardNavForm({ formValues: selectedContact });
6163

6264
const textFields = wrapper.findAllComponents(InputText);
6365
const selectFields = wrapper.findAllComponents(Select);
@@ -77,15 +79,17 @@ describe('ContactCardNavForm', () => {
7779
});
7880

7981
it('does not show the manual-contact warning when the contact has a linked userId', () => {
80-
const { wrapper } = mountContactCardNavForm(selectedContact);
82+
const { wrapper } = mountContactCardNavForm({ formValues: selectedContact });
8183

8284
expect(wrapper.find('.p-message').exists()).toBe(false);
8385
});
8486

8587
it('shows the manual-contact warning when the contact has no linked userId (a manually-entered contact)', () => {
8688
const { wrapper } = mountContactCardNavForm({
87-
contact: { ...selectedContact.contact, userId: undefined }
88-
} as unknown as DeepPartial<HousingFormSchemaType>);
89+
formValues: {
90+
contact: { ...selectedContact.contact, userId: undefined }
91+
} as unknown as DeepPartial<HousingFormSchemaType>
92+
});
8993

9094
expect(wrapper.find('.p-message').exists()).toBe(true);
9195
});
@@ -98,7 +102,7 @@ describe('ContactCardNavForm', () => {
98102
// actually intentional, since it looks more like unfinished wiring than
99103
// a deliberate default.
100104
it('renders the hidden contact.userId input as empty regardless of the contact prop', () => {
101-
const { wrapper } = mountContactCardNavForm(selectedContact);
105+
const { wrapper } = mountContactCardNavForm({ formValues: selectedContact });
102106

103107
const hiddenInput = wrapper.find('input[name="contact.userId"]');
104108
expect(hiddenInput.exists()).toBe(true);

frontend/tests/unit/components/housing/project/ProjectIntakeForm.spec.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,10 @@ const minimalProject = { projectId: 'project-1' } as unknown as HousingProject;
5555

5656
const searchContactsSpy = vi.spyOn(contactService, 'searchContacts');
5757

58-
function mountProjectIntakeForm(props: Record<string, unknown> = {}) {
58+
function mountProjectIntakeForm(options: { props?: Record<string, unknown> } = {}) {
5959
const { wrapper } = mountComponent(ProjectIntakeForm, {
60-
props,
61-
piniaState: {
62-
auth: { user: {} }
63-
},
60+
props: options.props,
61+
piniaState: {},
6462
stubs: {
6563
Map: true,
6664
ProjectIntakeAssistance: true,
@@ -128,7 +126,7 @@ describe('ProjectIntakeForm', () => {
128126

129127
describe('onBeforeMount', () => {
130128
it('redirects to the general error route if both `draft` and `project` are supplied', async () => {
131-
mountProjectIntakeForm({ draft: minimalDraft, project: minimalProject });
129+
mountProjectIntakeForm({ props: { draft: minimalDraft, project: minimalProject } });
132130
await flushPromises();
133131

134132
expect(mockRouter.replace).toHaveBeenCalledWith({ name: RouteName.EXT_GENERAL });

0 commit comments

Comments
 (0)