Skip to content

Commit ced7252

Browse files
committed
docs(mfa): simplify and deduplicate the graph-traversal comments
Trim the long debugging guides to their essentials, tell each repeated story (payload context split, hand-written journey endpoints, navigation mock) in one place only, and reword the dense graph jargon in plain language. Move misplaced notes next to the code they describe, promote comments that document exported declarations to JSDoc, and fix the comment that wrongly called the shared Navigation mock an implementation.
1 parent 6061b0c commit ced7252

8 files changed

Lines changed: 105 additions & 110 deletions

File tree

tests/unit/components/MultifactorAuthentication/machine/graphTraversal/coverageStaysComplete.test.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ import mfaMachine from '@components/MultifactorAuthentication/machine/mfaMachine
77
// The suites in this file guard the generated coverage itself: they analyze the walked paths that
88
// `viewMatchesMachine.test.tsx` drives through the real UI, without rendering anything themselves. A
99
// failure here usually means the walk stopped covering a state or transition, not that the modal broke.
10-
// Each failure names the missing piece.
1110

1211
const walkedPaths = getWalkedPaths();
1312

14-
// Every settleable leaf must end a path that the UI walk drives, not merely appear mid-path.
13+
// Every settleable leaf must end a path that the UI walk drives, not only appear in the middle of one.
1514
describe('every settleable MFA state is reachable through the real UI', () => {
1615
const walkedLeafValues = walkedPaths.map((path) => path.state.value);
1716

@@ -20,12 +19,10 @@ describe('every settleable MFA state is reachable through the real UI', () => {
2019
});
2120
});
2221

23-
// Shortest paths keep a single incoming route per distinct state-and-context vertex, so a transition can
24-
// silently drop out of the walk once another route to its target is shorter. A failure here has two
25-
// possible causes. When a transition was added to the machine, the fix is an explicit journey in
26-
// `DRIVING_JOURNEYS` that drives it. When the machine stopped resetting part of its context, the same
27-
// state value splits into a second vertex whose transitions have no route, so compare the failing
28-
// vertex's context with the intended one and fix the machine instead of adding a journey.
22+
// Shortest paths keep only one route into each distinct state and context, so a transition can
23+
// silently drop out of the walk when another route to its target is shorter. If this fails after
24+
// adding a transition, add a journey to `DRIVING_JOURNEYS` that drives it. If the failing entry has an
25+
// unexpected context, fix the machine so it resets that context again instead of adding a journey.
2926
describe('every UI-drivable state-changing transition is exercised', () => {
3027
const exercisedTransitionKeys = getExercisedTransitionKeys(walkedPaths);
3128

@@ -34,9 +31,9 @@ describe('every UI-drivable state-changing transition is exercised', () => {
3431
});
3532
});
3633

37-
// The payload INIT fixture exists to give the payload flow its own context vertices. When the machine
38-
// stops copying the payload into its context, every INIT fixture lands in the same vertex, the split
39-
// disappears, and the explicit journeys keep the walk green. This guard pins the split itself.
34+
// The payload INIT fixture exists so that the flow with a payload gets its own entries in the graph.
35+
// When the machine stops copying the payload into its context, the graph no longer tells the two
36+
// flows apart and the other suites still pass. Only this suite fails in that case.
4037
describe('INIT fixtures produce distinct context vertices', () => {
4138
it('keeps at least one landing vertex per distinct INIT fixture', () => {
4239
const landings = getInitEdgeLandings();

tests/unit/components/MultifactorAuthentication/machine/graphTraversal/everyStateReachable.test.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,13 @@ import getSettleableLeafStates from 'tests/utils/mfa/settleableLeafStates';
33
import {matchesState} from 'xstate';
44
import mfaMachine from '@components/MultifactorAuthentication/machine/mfaMachine';
55

6-
// Both suites in this file are generated by traversing the mfaMachine graph automatically instead of
7-
// enumerating cases by hand, and both analyze the chart alone; nothing renders. `getShortestPaths`
8-
// derives the coverage from the chart, so a state added to the machine gains a spec here without edits.
9-
// `settleableLeafStates.ts` defines which states count as settleable. The sibling suites under
10-
// `graphTraversal/` walk the same generated paths through the real modal UI and guard that coverage.
6+
// Both suites in this file are generated by traversing the mfaMachine graph, so a state added to the
7+
// machine gets a test here automatically. Nothing renders in this file. `settleableLeafStates.ts`
8+
// defines which states count as settleable.
119

1210
// Reachability is asserted on the unfiltered shortest paths, so it also covers routes the UI walk in
1311
// `viewMatchesMachine.test.tsx` cannot drive (a delayed transition, for example). A failure here means
14-
// the chart itself lost the state, not that the view harness broke.
12+
// the machine itself lost the state, not that the UI harness broke.
1513
describe('every settleable MFA state is reachable in the machine chart', () => {
1614
const reachableSnapshots = getMfaShortestPaths().map((path) => path.state);
1715

@@ -20,9 +18,9 @@ describe('every settleable MFA state is reachable in the machine chart', () => {
2018
});
2119
});
2220

23-
// Every generated expectation in these suites follows the machine under test, so a retargeted transition
24-
// that keeps every state reachable regenerates matching expectations and passes. Each journey's pinned
25-
// endpoint is hand-written in `DRIVING_JOURNEYS`, so it fails in that case instead.
21+
// The journey endpoints are the only hand-written expectations, so a transition that points at the
22+
// wrong target can only surface here, and only when a journey drives it. The `DRIVING_JOURNEYS` doc
23+
// explains why.
2624
describe('every driving journey ends in its pinned state', () => {
2725
it.each(getDrivingJourneyPaths())('$description', ({paths, endState}) => {
2826
expect(paths.length).toBeGreaterThan(0);

tests/unit/components/MultifactorAuthentication/machine/graphTraversal/viewMatchesMachine.test.tsx

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ import {handleInitialScreenLayout, mfaNavigationRef} from '@components/Multifact
1313
import CONST from '@src/CONST';
1414
import SCREENS from '@src/SCREENS';
1515

16-
// This file pins the contract between the machine and the rendered modal: it mounts the production
17-
// providers and navigator, drives each machine event as a real gesture, and asserts the UI markers of
18-
// the reached state at every step of every generated path. The paths come from `getWalkedPaths`, so a
19-
// state or event added to the machine gains steps here without edits. The chart-only suites live in
20-
// `everyStateReachable.test.ts`, and the guards on the generated coverage in `coverageStaysComplete.test.ts`.
16+
// This file tests that the rendered modal matches the machine: it mounts the production providers and
17+
// navigator, drives each machine event as a real gesture, and asserts the UI markers of the reached
18+
// state at every step of every generated path. A state or event added to the machine appears in the
19+
// walked paths automatically, and the type checks and guard suites then demand the hand-written
20+
// pieces it needs, such as an executor, a payload fixture, or a UI assertion. The machine-only suites
21+
// live in `everyStateReachable.test.ts` and the coverage guards in `coverageStaysComplete.test.ts`.
2122

2223
// This mock forces a wide layout so the navigator renders the backdrop used as the mounted marker.
2324
jest.mock('@hooks/useResponsiveLayout');
@@ -29,13 +30,11 @@ jest.mock('@libs/XStateInspector', () => ({__esModule: true, default: {inspect:
2930
jest.mock('@components/MultifactorAuthentication/biometrics/useBiometrics', () => jest.requireActual<typeof MfaRealUiMocks>('tests/utils/mfa/realUi/mocks').biometricsHookMock());
3031
// Browser and Android history synchronization is outside the contract between the machine and UI.
3132
jest.mock('@components/MultifactorAuthentication/useSyncMfaModalNavigatorWithHistory', () => jest.requireActual<typeof MfaRealUiMocks>('tests/utils/mfa/realUi/mocks').syncHistoryMock());
32-
// This mock reuses the shared Navigation implementation and overrides the transition methods used by the MFA flow.
33+
// jsdom runs no real navigation transitions, so the mock controls when the transition callbacks fire.
3334
jest.mock('@libs/Navigation/Navigation', () => jest.requireActual<typeof MfaRealUiMocks>('tests/utils/mfa/realUi/mocks').navigationMock());
3435

35-
// These UI markers distinguish the closed, closing, and outcome states. `OutcomeScreenBase` identifies the
36-
// outcome screen, while the backdrop exists only when the MFA navigator is mounted. Every outcome screen
37-
// renders the same `OutcomeScreenBase`, so the success assertion also checks the route name to pin which
38-
// outcome screen is on top.
36+
// These UI markers distinguish the closed, closing, and outcome states. The backdrop exists only while
37+
// the MFA navigator is mounted.
3938
const OUTCOME_SCREEN_TEST_ID = 'OutcomeScreenBase';
4039
const MODAL_BACKDROP_TEST_ID = 'MultifactorAuthenticationModalBackdrop';
4140

@@ -45,14 +44,17 @@ const CONFIRM_BUTTON_TEST_ID = 'MultifactorAuthenticationOutcomeConfirmButton';
4544
const MFA_STATE = CONST.MULTIFACTOR_AUTHENTICATION.MFA_STATE;
4645

4746
type MfaEventType = MfaEvent['type'];
48-
// The event carries only its `type` here because `xstate/graph` erases the payload from the executor's
49-
// step type. The INIT executor recovers the payload through `isTestScenarioInitEvent`.
47+
48+
/**
49+
* The event carries only its `type` here because `xstate/graph` erases the payload from the executor's
50+
* step type. The INIT executor recovers the payload through `isTestScenarioInitEvent`.
51+
*/
5052
type MfaEventExecutor = (step: {event: {type: MfaEventType}}) => Promise<void>;
5153

5254
/**
53-
* `INIT` enters through the public API with the scenario and payload of the step's own event, so paths
54-
* built from different INIT fixtures drive different flows. `MODAL_CLOSED` runs the navigator's teardown
55-
* callback. `satisfies Record<MfaEventType, ...>` requires an explicit executor for every machine event.
55+
* `INIT` starts a flow through the public API, using the scenario and payload from the step's own
56+
* event. `MODAL_CLOSED` runs the navigator's teardown callback. `satisfies Record<MfaEventType, ...>`
57+
* requires an explicit executor for every machine event.
5658
*/
5759
/* eslint-disable @typescript-eslint/naming-convention -- keys mirror the machine's event type union. */
5860
const mfaEventExecutors = {
@@ -91,6 +93,7 @@ const testConfig = {
9193
[`${MFA_STATE.OPEN}.${MFA_STATE.OUTCOME}.${MFA_STATE.SUCCESS}`]: () => {
9294
expect(screen.queryAllByTestId(MODAL_BACKDROP_TEST_ID)).not.toHaveLength(0);
9395
expect(screen.queryAllByTestId(OUTCOME_SCREEN_TEST_ID)).not.toHaveLength(0);
96+
// Every outcome screen renders the same `OutcomeScreenBase`, so the route name identifies which one is on top.
9497
expect(mfaNavigationRef.getCurrentRoute()?.name).toBe(SCREENS.MULTIFACTOR_AUTHENTICATION.OUTCOME_SUCCESS);
9598
},
9699
[MFA_STATE.CLOSING]: () => {
@@ -102,10 +105,13 @@ const testConfig = {
102105

103106
const walkedPaths = getWalkedPaths();
104107

105-
// `path.description` serializes the complete event payload, so test names use the short labels from
106-
// `describeTraversalEvent` instead. The synthetic `xstate.init` event is excluded because it is not part
107-
// of `MfaEvent`.
108108
const INIT_STEP_EVENT_TYPE = 'xstate.init';
109+
110+
/**
111+
* Builds the event part of a test name. `path.description` would serialize the complete event payload,
112+
* so the name uses the short labels from `describeTraversalEvent` instead. The synthetic `xstate.init`
113+
* event is excluded because it is not part of `MfaEvent`.
114+
*/
109115
function describeDrivenEvents(steps: ReadonlyArray<{event: {type: string}}>): string {
110116
const drivenEventLabels = steps
111117
.map((step) => step.event)
@@ -115,9 +121,9 @@ function describeDrivenEvents(steps: ReadonlyArray<{event: {type: string}}>): st
115121
}
116122

117123
describe('the real MFA modal matches the machine at every step of every generated path', () => {
118-
// The navigation buffer is deliberately not reset here. The machine owns that cleanup on `closed`
119-
// entry, which also runs when each test's fresh actor starts, so a reset here would hide a machine
120-
// that stopped performing it.
124+
// The navigation buffer is deliberately not reset here. The machine resets it when it enters
125+
// `closed`, which also runs when each test's fresh actor starts, so a reset here would hide a
126+
// machine that stopped doing that cleanup.
121127
beforeEach(() => {
122128
resetMfaUiMocks();
123129
});
@@ -138,10 +144,9 @@ describe('the real MFA modal matches the machine at every step of every generate
138144
// TestModel runs only the state assertions whose keys match the reached state, so if no key matches a
139145
// state, the test passes without checking it. These guards fail in that case.
140146
//
141-
// Types cannot do this. TypeScript's inferred type does not record whether a state has an `always`
142-
// transition, so it cannot tell a real leaf from a pass-through state such as `{open: "preparing"}`. A
143-
// `Record<leaf, ...>` would then need an empty assertion for every pass-through state, and that empty
144-
// entry would make the check pass on its own once the state later loses its `always` and becomes settleable.
147+
// A type cannot enforce this, because TypeScript does not know which states auto-advance. A
148+
// `Record` over all leaf states would then need empty assertions for the pass-through states, and an
149+
// empty assertion keeps passing silently.
145150
describe('testConfig defines a UI assertion for every settleable state and for nothing else', () => {
146151
const settleableLeafStates = getSettleableLeafStates(mfaMachine.root);
147152
const configuredStateKeys = Object.keys(testConfig.states);

tests/utils/mfa/flowFixtures.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@ const MFA_TEST_SCENARIO_NAME = CONST.MULTIFACTOR_AUTHENTICATION.SCENARIO.BIOMETR
88
type MfaTestScenarioParams = MultifactorAuthenticationScenarioParams<typeof MFA_TEST_SCENARIO_NAME>;
99

1010
/**
11-
* A non-empty payload for the test scenario. The traversal pairs it with the bare fixture so flows with
12-
* and without a payload occupy distinct context vertices in the graph.
11+
* A non-empty payload for the test scenario. The traversal pairs it with the bare fixture so the flow
12+
* with a payload is covered separately from the flow without one.
1313
*/
1414
const MFA_TEST_PAYLOAD: MfaTestScenarioParams = {validateCode: '123456'};
1515

16-
// The narrowed generic ties `scenarioName`, `scenario`, and `payload` to the same scenario, so pairing
17-
// the name with another scenario's config here is a compile error.
16+
/**
17+
* Builds the INIT event fixture for the test scenario. The narrowed generic ties `scenarioName`,
18+
* `scenario`, and `payload` to the same scenario, so pairing the name with another scenario's config
19+
* here is a compile error.
20+
*/
1821
function createInitEvent(payload?: MfaTestScenarioParams): MultifactorAuthenticationInitEvent<typeof MFA_TEST_SCENARIO_NAME> {
1922
return {
2023
type: 'INIT',
@@ -26,18 +29,17 @@ function createInitEvent(payload?: MfaTestScenarioParams): MultifactorAuthentica
2629

2730
/**
2831
* Narrows a traversal event to the test-scenario INIT fixture shape. The scenario-name check is enough,
29-
* because {@link createInitEvent} is the only source of INIT events in the traversal and it correlates
30-
* the payload with the scenario name by construction. The parameter accepts any typed event, because
31-
* `xstate/graph` erases everything but `type` from the event an executor receives.
32+
* because {@link createInitEvent} is the only place that builds INIT events for the traversal. The
33+
* parameter accepts any typed event, because `xstate/graph` erases everything but `type` from the
34+
* event an executor receives.
3235
*/
3336
function isTestScenarioInitEvent(event: {type: string}): event is MultifactorAuthenticationInitEvent<typeof MFA_TEST_SCENARIO_NAME> {
3437
return event.type === 'INIT' && 'scenarioName' in event && event.scenarioName === MFA_TEST_SCENARIO_NAME;
3538
}
3639

3740
/**
38-
* Names a traversal event in path titles and transition descriptions. The INIT fixtures differ only in
39-
* their payload, which the titles otherwise erase, so a failure would not tell which fixture broke. The
40-
* label lists the payload keys instead of the serialized payload to stay short and stable.
41+
* Returns a short label for a traversal event. The INIT fixtures differ only in their payload, so the
42+
* label lists the payload keys to tell them apart without serializing the whole payload.
4143
*/
4244
function describeTraversalEvent(event: {type: string}): string {
4345
if (!isTestScenarioInitEvent(event)) {

0 commit comments

Comments
 (0)