-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Expand file tree
/
Copy pathunit-testing-guidelines.mdc
More file actions
718 lines (547 loc) · 23.7 KB
/
unit-testing-guidelines.mdc
File metadata and controls
718 lines (547 loc) · 23.7 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
---
description: Project Guidelines for Unit Testing
globs: *.test.*
alwaysApply: false
---
Reference: [MetaMask Unit Testing Guidelines](https://github.com/MetaMask/contributor-docs/blob/main/docs/testing/unit-testing.md)
# Unit Testing Guidelines
## Test Naming Rules
- **NEVER use "should" in test names** - this is a hard rule with zero exceptions
- **Use action-oriented descriptions** that describe what the code does
- **Be specific about the behavior being tested**
- **AVOID** weasel words like "handle", "manage", or other non-specific action verbs
- **AVOID** subjective outcome words like "successfully", "correctly", "invalid" - instead indicate what the actual result should be
- **BE SPECIFIC** about conditions: use "email without domain" instead of "invalid email"
```ts
// ❌ WRONG
it('should return fixed timestamp', () => { ... });
it('should ignore events', () => { ... });
it('should display error when input is invalid', () => { ... });
it('handles invalid input correctly', () => { ... });
// ✅ CORRECT
it('returns fixed timestamp for privacy events', () => { ... });
it('ignores events without privacy timestamp property', () => { ... });
it('displays error when email is missing @ symbol', () => { ... });
it('returns false for email without domain', () => { ... });
```
## Test Structure and Organization - MANDATORY
- **EVERY test MUST follow the AAA pattern** (Arrange, Act, Assert) with blank line separation
- **Each test must cover ONE behavior** and be isolated from others
- **Use helper functions** for test data creation
- **Group related tests** in `describe` blocks
```ts
it('returns false for email without domain', () => {
const input = 'user@';
const result = validateEmail(input);
expect(result).toBe(false);
});
```
**Helper Functions**:
```ts
const createTestEvent = (overrides = {}) => ({
type: EventType.TrackEvent,
event: 'Test Event',
timestamp: '2024-01-01T12:00:00.000Z',
...overrides,
});
```
## Element Selection - PREFER DATA TEST IDs
- **ALWAYS prefer `testID` props** for selecting elements in tests
- **Use `getByTestId`** as the primary query method for reliable element selection
- **Add `testID` props** to components when writing new code or updating existing code
- **Avoid selecting by text** when the text might change (i18n, copy updates)
```tsx
// ✅ CORRECT - Use testID for reliable selection
<Button testID="submit-button" onPress={handleSubmit}>
{t('common.submit')}
</Button>;
// In test:
const submitButton = screen.getByTestId('submit-button');
expect(submitButton).toBeOnTheScreen();
// ✅ ALSO GOOD - Locale keys (safe from content updates)
const button = screen.getByText(strings('common.submit'));
// ❌ AVOID - Selecting by hardcoded text content
const button = screen.getByText('Submit'); // Breaks when text changes
const input = screen.getByPlaceholderText('Enter email'); // Fragile
```
### CHILD PROP OBJECTS - ALL COMPONENTS SUPPORT THIS
**ALL `@metamask/design-system-react-native` and `app/component-library` components support child prop objects for passing testIDs to internal elements.** This is a universal design pattern - prefer not to mock these components just to inject testIDs.
Common child prop object patterns:
- `closeButtonProps` - for close/dismiss buttons
- `backButtonProps` - for back navigation buttons
- `startAccessoryProps` / `endAccessoryProps` - for accessory elements
- `iconProps` - for icon elements
- `labelProps` - for label text elements
- `inputProps` - for input elements within compound components
- `*Props` - any prop ending in `Props` is likely a child prop object
```tsx
// ❌ WRONG - Mocking to add testID (141 lines of unnecessary code!)
// The testID capability ALREADY EXISTS via child prop objects!
jest.mock('BottomSheetHeader', () => {
return ({ onClose }) => (
<TouchableOpacity testID="close-button" onPress={onClose}>Close</TouchableOpacity>
);
});
// ✅ CORRECT - Use the component's child prop object API
<HeaderCenter
title="Select a region"
onClose={handleClose}
closeButtonProps={{ testID: 'region-selector-close-button' }}
/>
// ✅ CORRECT - BottomSheetHeader supports child prop objects
<BottomSheetHeader
onClose={handleClose}
closeButtonProps={{ testID: 'modal-close-button' }}
onBack={handleBack}
backButtonProps={{ testID: 'modal-back-button' }}
>
{title}
</BottomSheetHeader>
// In test - no mocking needed!
const closeButton = screen.getByTestId('region-selector-close-button');
fireEvent.press(closeButton);
```
### This Is Universal - No Exceptions
| Library | testID Support |
| -------------------------------------- | -------------------------------------------------------------- |
| `@metamask/design-system-react-native` | ✅ All components support `testID` prop AND child prop objects |
| `app/component-library/*` | ✅ All components support `testID` prop AND child prop objects |
**If you need to add a testID to one of these components, check for child prop objects first.** Most components support this functionality. If not available, suggest adjusting the component to support it in another change set.
### How to Find Child Prop Objects
1. **Check TypeScript types** - Look at the component's props interface for props ending in `Props`
2. **Check component source** - Search for `Props` suffix patterns
3. **Check Storybook** - Component stories demonstrate these props
```tsx
// Example: BottomSheetHeader TypeScript interface shows:
// - closeButtonProps?: ButtonIconProps
// - backButtonProps?: ButtonIconProps
// Example: Design system Button with icon
<Button
testID="submit-button"
iconProps={{ testID: 'submit-icon' }}
labelProps={{ testID: 'submit-label' }}
>
Submit
</Button>
```
### TestID Naming Conventions
```tsx
// Use kebab-case for testIDs
testID="settings-screen"
testID="submit-button"
testID="error-message"
testID="token-list-item"
// Include context for list items
testID={`token-item-${token.symbol}`}
testID={`network-option-${network.chainId}`}
```
## Snapshot Testing Policy - MANDATORY
### The Rule
- ❌ **`toMatchSnapshot()` is BANNED** — Do not use it. Do not add it. Do not approve it.
- ✅ **`toMatchInlineSnapshot()` is ALLOWED** — Use sparingly, only when the serialized output is the meaningful assertion.
### Why `toMatchSnapshot()` Is Banned
External snapshot files (`.snap`) have three critical problems:
1. **Invisible in review** — The diff lives in a separate `.snap` file that reviewers routinely rubber-stamp. Regressions hide there.
2. **No code ownership** — `CODEOWNERS` explicitly assigns `**/*.snap` to *nobody*, meaning no team is accountable for snapshot correctness.
3. **Brittle by design** — Any style tweak, whitespace change, or unrelated refactor regenerates the snapshot and silently passes CI.
### Why `toMatchInlineSnapshot()` Is Allowed
The snapshot string lives directly in the test file, so:
- It appears in the PR diff alongside the code that produces it
- The file's code owner is responsible for reviewing it
- Reviewers can see exactly what changed and why
### When to Use `toMatchInlineSnapshot()`
Only use it when the serialized shape of the output **is** the assertion — for example, verifying a complex object structure, a formatted string, or a serialized data payload. Do **not** use it as a lazy substitute for explicit `expect` calls.
```ts
// ❌ BANNED — writes to an external .snap file
expect(tree).toMatchSnapshot();
expect(component).toMatchSnapshot();
expect(result).toMatchSnapshot();
// ✅ ALLOWED — snapshot is inline and visible in review
expect(result).toMatchInlineSnapshot(`
{
"chainId": "0x1",
"name": "Ethereum Mainnet",
}
`);
// ✅ PREFERRED — explicit assertions are always better than snapshots
expect(result.chainId).toBe('0x1');
expect(result.name).toBe('Ethereum Mainnet');
```
### Migrating Existing `toMatchSnapshot()` Calls
When you encounter an existing `toMatchSnapshot()` call:
1. **Prefer replacing it** with explicit `expect` assertions targeting the specific values that matter.
2. If the full serialized output is genuinely meaningful, convert to `toMatchInlineSnapshot()`.
3. Delete the corresponding `.snap` file entry once migrated.
Do **not** leave `toMatchSnapshot()` in place when modifying a test file — migrate it as part of your change.
---
## Assertions - PREFER toBeOnTheScreen
- **ALWAYS use `toBeOnTheScreen()`** to assert element presence - NOT `toBeTruthy()` or `toBeDefined()`
- **Use specific matchers** that communicate intent clearly
- **Avoid weak matchers** that don't actually verify the expected behavior
```tsx
// ✅ CORRECT - Clear, specific assertions
expect(screen.getByTestId('submit-button')).toBeOnTheScreen();
expect(screen.queryByTestId('error-message')).not.toBeOnTheScreen();
expect(screen.getByTestId('balance-text')).toHaveTextContent('100 ETH');
// ❌ WRONG - Weak matchers that don't verify presence properly
expect(screen.getByTestId('submit-button')).toBeTruthy(); // Misleading
expect(screen.getByTestId('submit-button')).toBeDefined(); // Doesn't verify render
expect(element).not.toBeNull(); // Use toBeOnTheScreen() instead
```
### Recommended Matchers
| Instead of | Use |
| ------------------------------------ | --------------------------------------------------------- |
| `toBeTruthy()` for elements | `toBeOnTheScreen()` |
| `toBeDefined()` for elements | `toBeOnTheScreen()` |
| `not.toBeNull()` | `not.toBeOnTheScreen()` or `queryByTestId` returning null |
| `toHaveLength(1)` for single element | `toBeOnTheScreen()` |
## Mocking Rules - CRITICAL
### Exception: UI Components and TestIDs
**Prefer not to mock `@metamask/design-system-react-native` or `app/component-library` components just to inject testIDs.** All these components support testIDs via:
- Direct `testID` prop on the component
- Child prop objects (`closeButtonProps`, `iconProps`, etc.) for internal elements
```tsx
// ❌ WRONG: Mocking to inject testID
jest.mock('BottomSheetHeader', () => ({ onClose }) => (
<TouchableOpacity testID="close-button" onPress={onClose}>
Close
</TouchableOpacity>
));
// ✅ RIGHT: Use child prop objects
<BottomSheetHeader
onClose={handleClose}
closeButtonProps={{ testID: 'modal-close-button' }}
/>;
```
See [PR #25548](https://github.com/MetaMask/metamask-mobile/pull/25548) for refactoring example.
### General Mocking Rules
- **EVERYTHING not under test MUST be mocked** - no exceptions
- **NO** use of `require` - use ES6 imports only
- **NO** use of `any` type - use proper TypeScript types
- **Mock all external dependencies** including APIs, services, hooks
- **Use realistic mock data** that reflects real usage
### Theme Mocking Rules
- **Prefer shared `mockTheme` from `app/util/theme`** instead of hard-coded color literals in tests.
- **Never hardcode design token hex values** in assertions or theme mocks (enforced by `@metamask/design-tokens/color-no-hex`).
- **Avoid local hex color objects** for `useTheme`, `useStyles`, or tailwind mock color functions.
- **If a test only needs a specific theme field**, derive it from `mockTheme` (or spread `mockTheme` and override minimally).
```ts
// ✅ CORRECT: use shared mockTheme for useTheme mocks
import { mockTheme } from '../../util/theme';
jest.mock('../../util/theme', () => ({
useTheme: () => mockTheme,
}));
```
```ts
// ✅ CORRECT: return { styles, theme } for useStyles mocks
import { mockTheme } from '../../util/theme';
jest.mock('../../../../../component-library/hooks', () => ({
useStyles: jest.fn((styleFn, vars) => ({
styles: styleFn({ theme: mockTheme, vars }),
theme: mockTheme,
})),
}));
```
```ts
// ✅ CORRECT: mock useTailwind with the right shape
jest.mock('@metamask/design-system-twrnc-preset', () => ({
useTailwind: () => ({
// Most components only need tw.style(...)
style: jest.fn(() => ({})),
}),
}));
```
```ts
// ✅ ALSO CORRECT: match real useTailwind() return type (callable function + helpers)
import { mockTheme } from '../../util/theme';
jest.mock('@metamask/design-system-twrnc-preset', () => ({
useTailwind: () => {
const tw = () => ({});
tw.style = jest.fn(() => ({}));
return tw;
},
}));
```
```ts
// ❌ AVOID: local hex color mocks
const mockColors = {
text: { default: '#000000' },
background: { default: '#FFFFFF' },
border: { muted: '#E5E7EB' },
};
```
```ts
// ✅ CORRECT
import { apiService } from '../services/api';
jest.mock('../services/api');
const mockApiService = apiService as jest.Mocked<typeof apiService>;
interface MockEvent {
type: EventType;
event: string;
timestamp: string;
}
// ❌ WRONG
const mockApi = require('../services/api'); // ❌ no require
const mockApi: any = jest.fn(); // ❌ no any type
```
## Test Isolation and Focus - MANDATORY
- **Each test MUST be independent** - no shared state between tests
- **Use `beforeEach` for setup, `afterEach` for cleanup**
- **Reset all mocks between tests**
- **Tests MUST run in any order**
- **Avoid duplicated or polluted tests**
- **Use mocks for all external dependencies**
```ts
// ✅ CORRECT Test Isolation
describe('MetaMetricsCustomTimestampPlugin', () => {
let plugin: MetaMetricsCustomTimestampPlugin;
beforeEach(() => {
plugin = new MetaMetricsCustomTimestampPlugin({
timestampStrategy: 'fixed',
customTimestamp: '1970-01-01T00:00:00.000Z',
});
jest.clearAllMocks();
});
afterEach(() => {
jest.resetAllMocks();
});
it('returns fixed timestamp for privacy events', () => {
const event = createTestEvent({ privacyTimestamp: true });
const result = plugin.execute(event);
expect(result.timestamp).toBe('1970-01-01T00:00:00.000Z');
});
});
```
## Test Coverage (MANDATORY)
**EVERY component MUST test:**
- ✅ **Happy path** - normal expected behavior
- ✅ **Edge cases** - null, undefined, empty values, boundary conditions
- ✅ **Error conditions** - invalid inputs, failure scenarios
- ✅ **Different code paths** - all if/else branches, switch cases
- ✅ **Method chaining** - for builder patterns
- ✅ **Side effects** - property changes, state updates, cleanup
```ts
// ✅ CORRECT Coverage Example
describe('MetaMetricsCustomTimestampPlugin', () => {
describe('execute', () => {
it('returns fixed timestamp for privacy events', () => {
// Happy path
});
it('ignores events without privacy timestamp property', () => {
// Edge case
});
it('throws error when strategy is null', () => {
// Error condition
});
it('uses event-specific timestamp strategy when provided', () => {
// Different code path
});
it('removes privacy properties from event', () => {
// Side effect
});
});
});
```
## Parameterized Tests
- Parameterize tests to cover all values (e.g., enums) with type-safe iteration.
```ts
it.each(['small', 'medium', 'large'] as const)('renders %s size', (size) => {
expect(renderComponent(size)).toBeOnTheScreen();
});
```
## Test Determinism
- **EVERYTHING** not under test must be mocked - no exceptions.
- Avoid brittle tests: do not test internal state or UI snapshots for logic. **`toMatchSnapshot()` is banned** — see Snapshot Testing Policy above.
- Only test public behavior, not implementation details.
- Mock time, randomness, and external systems to ensure consistent results.
```ts
// Mock all external dependencies
jest.mock('../services/api');
jest.mock('../utils/date');
jest.mock('../hooks/useAuth');
jest.useFakeTimers();
jest.setSystemTime(new Date('2024-01-01'));
```
- Avoid relying on global state or hardcoded values (e.g., dates) or mock it.
## Async Testing and act() - CRITICAL
**ALWAYS use `act()` when testing async operations that trigger React state updates.**
### When to Use act()
Use `act()` when you:
- Call async functions via component props (e.g., `onRefresh`, `onPress` with async handlers)
- Invoke functions that perform state updates asynchronously
- Test pull-to-refresh or other async interactions
### Symptoms of Missing act()
Tests fail intermittently with:
- `TypeError: terminated`
- `SocketError: other side closed`
- Warnings about state updates not being wrapped in act()
### Examples
```ts
// ❌ WRONG - Causes flaky tests
it('calls Logger.error when handleOnRefresh fails', async () => {
const mockLoggerError = jest.spyOn(Logger, 'error');
render(BankDetails);
// Async function called without act() - causes race condition
screen
.getByTestId('refresh-control-scrollview')
.props.refreshControl.props.onRefresh();
await waitFor(() => {
expect(mockLoggerError).toHaveBeenCalled();
});
});
// ✅ CORRECT - Properly handles async state updates
it('calls Logger.error when handleOnRefresh fails', async () => {
const mockLoggerError = jest.spyOn(Logger, 'error');
render(BankDetails);
// Wrap async operation in act()
await act(async () => {
await screen
.getByTestId('refresh-control-scrollview')
.props.refreshControl.props.onRefresh();
});
await waitFor(() => {
expect(mockLoggerError).toHaveBeenCalled();
});
});
```
### Common Patterns Requiring act()
```ts
// RefreshControl callbacks
await act(async () => {
await refreshControl.props.onRefresh();
});
// Async button press handlers
await act(async () => {
await button.props.onPress();
});
// Any async callback that updates state
await act(async () => {
await component.props.onSomeAsyncAction();
});
```
### Why This Matters
Without `act()`:
1. Async function starts executing
2. Test continues and waits only for specific assertion
3. Jest cleanup/termination happens while promises are still pending
4. Results in "terminated" or "other side closed" errors
With `act()`:
1. React Testing Library waits for all state updates
2. All promises resolve before test proceeds
3. Clean, deterministic test execution
# Reviewer Responsibilities
- Validate that tests fail when the code is broken (test the test).
```ts
// Break the SuT and make sure this test fails
expect(result).toBe(false);
```
- Ensure tests use proper matchers (`toBeOnTheScreen` vs `toBeDefined`).
- **Reject any new `.snap` files or new `toMatchSnapshot()` calls** — these are banned; require the author to use explicit assertions or `toMatchInlineSnapshot()` instead.
- Reject tests with complex names combining multiple logical conditions (AND/OR).
# Refactoring Support
- Ensure tests provide safety nets during refactors and logic changes. Run the tests before pushing commits!
- Encourage small, testable components.
- Unit tests must act as documentation for feature expectations.
# Quality Checklist - MANDATORY
Before submitting any test file, verify:
- [ ] **No `toMatchSnapshot()` calls** — BANNED; use explicit assertions or `toMatchInlineSnapshot()` instead
- [ ] **No mocking to inject testIDs** - Use component's built-in testID support
- [ ] **testIDs via child prop objects** - Use `closeButtonProps={{ testID }}` not mocks
- [ ] **No "should" in any test name**
- [ ] **All tests follow AAA pattern**
- [ ] **Each test has one clear purpose**
- [ ] **All code paths are tested**
- [ ] **Edge cases are covered**
- [ ] **Test data is realistic**
- [ ] **Tests are independent**
- [ ] **Assertions use `toBeOnTheScreen()`** - NOT `toBeTruthy()` or `toBeDefined()`
- [ ] **Assertions are specific**
- [ ] **Elements selected by `testID`** - NOT fragile text queries
- [ ] **Test names are descriptive**
- [ ] **No test duplication**
- [ ] **Async operations wrapped in act()** when they trigger state updates
# Common Mistakes to AVOID - CRITICAL
- ❌ **Using `toMatchSnapshot()`** — BANNED; it writes opaque `.snap` files with no code owner; use explicit assertions or `toMatchInlineSnapshot()` instead
- ❌ **Mocking to inject testIDs** - Components already support testID (see guidelines above)
- ❌ **Using "should" in test names** - This is the #1 mistake, use action-oriented descriptions
- ❌ **Testing multiple behaviors in one test** - One test, one behavior
- ❌ **Sharing state between tests** - Each test must be independent
- ❌ **Not testing error conditions** - Test both success and failure paths
- ❌ **Using unrealistic test data** - Use data that reflects real usage
- ❌ **Not following AAA pattern** - Always Arrange, Act, Assert
- ❌ **Not testing edge cases** - Test null, undefined, empty values
- ❌ **Using weak matchers** - Use `toBeOnTheScreen()` instead of `toBeTruthy()`/`toBeDefined()`
- ❌ **Selecting elements by text** - Use `testID` props for reliable selection
- ❌ **Not wrapping async state updates in act()** - Causes flaky "terminated" errors
# Unit tests developement workflow
- Always run unit tests after making code changes.
- **NEVER** use npm, npx, or other package managers - ONLY use yarn
## Testing Commands
### Single File Testing
```shell
# Use this command for testing a specific file
yarn jest <filename>
# Use this command for testing specific test cases
yarn jest <filename> -t "<test-name-pattern>"
# Use this command for running all unit tests
yarn test:unit
# Run a specific test file
yarn jest MyComponent.test.tsx
yarn jest utils/helpers.test.ts
```
### Coverage Reports
```shell
# Use this command for coverage reports
yarn test:unit:coverage
```
## Workflow Requirements
- Confirm all tests are passing before commit.
- **Do not add new `toMatchSnapshot()` calls** — this is banned. See Snapshot Testing Policy.
- When modifying a test file that contains `toMatchSnapshot()`, opportunistically migrate those calls to explicit assertions or `toMatchInlineSnapshot()` — do not block a PR solely to force migration, but do not add new ones.
- `toMatchInlineSnapshot()` updates are acceptable but must be reviewed — confirm the new inline snapshot reflects an intentional, expected change.
# Reference Code Examples
**Proper AAA**:
```ts
it('indicates expired milk when past due date', () => {
const today = new Date('2025-06-01');
const milk = { expiration: new Date('2025-05-30') };
const result = isMilkGood(today, milk);
expect(result).toBe(false);
});
```
## ❌ Banned: `toMatchSnapshot()`
```ts
// ❌ BANNED — toMatchSnapshot() writes to an external .snap file.
// It has no code owner, hides regressions, and breaks on trivial changes.
it('renders the button', () => {
const { container } = render(<MyButton />);
expect(container).toMatchSnapshot(); // 🚫 BANNED — do not use
});
```
## ✅ Robust UI Assertion
```ts
it('displays error message when API fails', async () => {
mockApi.failOnce();
const { findByText } = render(<MyComponent />);
expect(await findByText('Something went wrong')).toBeOnTheScreen();
});
```
**Test the Test**:
```ts
it('hides selector when disabled', () => {
const { queryByTestId } = render(<Selector enabled={false} />);
expect(queryByTestId('IPFS_GATEWAY_SELECTED')).toBeNull();
// Break test: change enabled={false} to enabled={true} and verify test fails
});
```
## Reviewer Responsibilities
Validate tests fail when code breaks • Ensure proper matchers • **Reject new `toMatchSnapshot()` calls and new `.snap` files** • Reject complex names with AND/OR
```ts
// OK
it('renders button when enabled');
// NOT OK
it('renders and disables button when input is empty or missing required field');
```
## Workflow
Always run tests after changes • Confirm all pass before commit • **Do not add new `toMatchSnapshot()` calls** — migrate existing ones to explicit assertions or `toMatchInlineSnapshot()` when touching a test file
**Resources**: [Contributor docs](https://github.com/MetaMask/contributor-docs/blob/main/docs/testing/unit-testing.md) • [Jest Matchers](https://jestjs.io/docs/using-matchers) • [React Native Testing Library](https://testing-library.com/docs/react-native-testing-library/intro/)