docs(05): research phase tech-debt
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,431 @@
|
||||
# Phase 5: Tech Debt - Research
|
||||
|
||||
**Researched:** 2026-03-30
|
||||
**Domain:** React wizard refactoring — conditional rendering, registry-driven UI, test quality
|
||||
**Confidence:** HIGH
|
||||
|
||||
<user_constraints>
|
||||
## User Constraints (from CONTEXT.md)
|
||||
|
||||
### Locked Decisions
|
||||
|
||||
**scriptTargets filtering (TECH-01)**
|
||||
- ReviewStep conditionally renders output blocks based on `state.deployment.scriptTargets` — blocks are unmounted entirely when their target is deselected (not CSS-hidden)
|
||||
- Intune maps to: intuneInstall + intuneDetection blocks
|
||||
- RMM maps to: rmmScript block
|
||||
- rclone.conf OutputBlock is always shown regardless of scriptTargets
|
||||
- ZIP bundle also respects scriptTargets — only files for selected targets are included (not always 4)
|
||||
- Edge case: if both targets are deselected, only rclone.conf is shown and ZIP contains only rclone.conf (no blocking or warning needed)
|
||||
|
||||
**Back button (TECH-02)**
|
||||
- Must preserve all form data (wizard state is in the store — navigation does not clear it)
|
||||
- Back dispatches `SET_STEP(2)` to return to DeploymentStep
|
||||
|
||||
**Registry structure (TECH-03)**
|
||||
- Enrich `BACKEND_REGISTRY` to `Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }>`
|
||||
- All consumers of `BACKEND_REGISTRY[type]` that access field arrays must update to `BACKEND_REGISTRY[type].fields`
|
||||
- `BackendSelectionStep` derives the card list from `Object.entries(BACKEND_REGISTRY)` — no hardcoded `BACKENDS` array
|
||||
- Display metadata stays in the registry entry so Phase 6 additions auto-surface with zero additional code
|
||||
- Existing test assertions on card text ('Azure Blob Storage', etc.) remain valid — source of truth just moves
|
||||
|
||||
**Dead export removal (TECH-04)**
|
||||
- Remove `BackendFormValues<T>` export from `src/schemas/index.ts` (lines 27-28)
|
||||
- Verify no TypeScript errors arise after removal (no consumers expected)
|
||||
|
||||
**act() warnings (TECH-05)**
|
||||
- Claude's Discretion — fix the act() warnings in `BackendSelectionStep.test.tsx` using appropriate testing-library patterns
|
||||
|
||||
### Claude's Discretion
|
||||
|
||||
- Back button (TECH-02): placement and styling left to planner
|
||||
- act() warnings (TECH-05): fix approach left to planner
|
||||
|
||||
### Deferred Ideas (OUT OF SCOPE)
|
||||
|
||||
None — discussion stayed within phase scope.
|
||||
</user_constraints>
|
||||
|
||||
<phase_requirements>
|
||||
## Phase Requirements
|
||||
|
||||
| ID | Description | Research Support |
|
||||
|----|-------------|-----------------|
|
||||
| TECH-01 | User sees only the script output blocks matching their selected deployment targets (Intune and/or RMM) in ReviewStep | Conditional render pattern; ZIP filter array construction; existing `state.deployment.scriptTargets` already tracks selections |
|
||||
| TECH-02 | User can navigate back from ReviewStep using an explicit Back button | `SET_STEP(2)` action exists; DeploymentStep Back button is the established style pattern |
|
||||
| TECH-03 | Backend list in BackendSelectionStep is automatically derived from BACKEND_REGISTRY keys (no hardcoded list) | Registry shape change; three consumers need `.fields` access update; BackendSelectionStep replaces `BACKENDS` constant |
|
||||
| TECH-04 | Dead `BackendFormValues<T>` export is removed from `src/schemas/index.ts` | Lines 27-28 confirmed; no consumers found via code audit |
|
||||
| TECH-05 | BackendSelectionStep test suite runs without `act()` warnings | Warnings confirmed in live test run; root cause is `fireEvent` triggering async state updates without `act()` wrapping |
|
||||
</phase_requirements>
|
||||
|
||||
## Summary
|
||||
|
||||
This phase is purely internal code quality work — no new user-facing features except the Back button on ReviewStep and the scriptTargets filtering. All changes are surgical: modify four files (`registry.ts`, `schemas/index.ts`, `BackendSelectionStep.tsx`, `ReviewStep.tsx`) and update three test files to keep assertions aligned.
|
||||
|
||||
The codebase is in clean, idiomatic React/TypeScript with Vitest + Testing Library. Patterns are already established: conditional render (not CSS-hide) for content that has no preserved state, store dispatch for navigation, registry-driven loops for field rendering. Phase 5 applies these patterns to the remaining places that weren't wired up in v1.0.
|
||||
|
||||
The act() warnings are real and confirmed to be in both `BackendSelectionStep.test.tsx` (warnings about `BackendSelectionStep` and `WizardProvider` components) and `ReviewStep.test.tsx` (warnings about `OutputBlock`). The root cause is `fireEvent` calling React Hook Form's `handleSubmit`, which triggers async state updates and `setTimeout` (in `OutputBlock.handleCopy`) without `act()` wrapping. The fix is `userEvent` (which wraps in `act()`) or explicit `act()` calls around async sequences.
|
||||
|
||||
**Primary recommendation:** Address each requirement as a focused, independent task. TECH-03 (registry enrichment) has the most ripple effect — do it first so TECH-01 and the BackendSelectionStep work have the final registry shape available.
|
||||
|
||||
## Standard Stack
|
||||
|
||||
### Core (already in use — no new installs)
|
||||
|
||||
| Library | Version | Purpose | Why Standard |
|
||||
|---------|---------|---------|--------------|
|
||||
| React | 18.3.1 | UI rendering, conditional render, hooks | Project baseline |
|
||||
| TypeScript | 5.5.3 | Type safety for registry shape change | Project baseline |
|
||||
| Vitest | 4.1.1 | Test runner | Project baseline |
|
||||
| @testing-library/react | 16.3.2 | Component testing | Project baseline |
|
||||
| @testing-library/user-event | 14.6.1 | User interaction simulation (wraps in `act()`) | Already installed; fixes act() warnings |
|
||||
|
||||
**Installation:** No new packages required. `@testing-library/user-event` is already in `devDependencies`.
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### Recommended File Touch Map
|
||||
|
||||
```
|
||||
src/
|
||||
├── schemas/
|
||||
│ ├── registry.ts # TECH-03: enrich shape + add displayName/description to each entry
|
||||
│ └── index.ts # TECH-03: update .fields access in buildZodSchema; TECH-04: remove BackendFormValues<T>
|
||||
├── components/wizard/
|
||||
│ ├── BackendSelectionStep.tsx # TECH-03: replace BACKENDS const with Object.entries(BACKEND_REGISTRY)
|
||||
│ ├── BackendSelectionStep.test.tsx # TECH-05: replace fireEvent with userEvent; TECH-03: verify assertions still pass
|
||||
│ ├── RemoteConfigStep.tsx # TECH-03: update BACKEND_REGISTRY[backendType] → BACKEND_REGISTRY[backendType].fields
|
||||
│ └── ReviewStep.tsx # TECH-01: conditional renders; TECH-02: Back button
|
||||
└── (no new files needed)
|
||||
```
|
||||
|
||||
### Pattern 1: Conditional Render by scriptTargets (TECH-01)
|
||||
|
||||
**What:** Unmount OutputBlocks whose target is not in `state.deployment.scriptTargets`
|
||||
**When to use:** When the content has no local state worth preserving between show/hide
|
||||
**Why not CSS-hide:** OutputBlock has no internal state that must survive deselection (unlike AzureAuthToggle's field values)
|
||||
|
||||
```typescript
|
||||
// In ReviewStep — derive booleans from state
|
||||
const showIntune = state.deployment.scriptTargets.includes('intune');
|
||||
const showRmm = state.deployment.scriptTargets.includes('rmm');
|
||||
|
||||
// Conditional render (NOT CSS hidden)
|
||||
{showIntune && (
|
||||
<OutputBlock label="Intune Install Script" content={intuneInstall} filename="intune-install.ps1" disabled={!acknowledged} />
|
||||
)}
|
||||
{showIntune && (
|
||||
<OutputBlock label="Intune Detection Script" content={intuneDetection} filename="intune-detection.ps1" disabled={!acknowledged} />
|
||||
)}
|
||||
{showRmm && (
|
||||
<OutputBlock label="RMM Script" content={rmmScript} filename="rmm-script.ps1" disabled={!acknowledged} />
|
||||
)}
|
||||
```
|
||||
|
||||
### Pattern 2: ZIP Filter by scriptTargets (TECH-01)
|
||||
|
||||
**What:** Build the files array dynamically before passing to `downloadZip()`
|
||||
|
||||
```typescript
|
||||
async function handleDownloadZip() {
|
||||
const files: { name: string; content: string }[] = [
|
||||
{ name: 'rclone.conf', content: rcloneConf }, // always included
|
||||
];
|
||||
if (showIntune) {
|
||||
files.push({ name: 'intune-install.ps1', content: intuneInstall });
|
||||
files.push({ name: 'intune-detection.ps1', content: intuneDetection });
|
||||
}
|
||||
if (showRmm) {
|
||||
files.push({ name: 'rmm-script.ps1', content: rmmScript });
|
||||
}
|
||||
await downloadZip(files, 'rclone-deployment.zip');
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 3: Back Button in ReviewStep (TECH-02)
|
||||
|
||||
**What:** Follow DeploymentStep's exact button layout — Back + action button side-by-side at the bottom
|
||||
**Established reference:** `DeploymentStep.tsx` lines 90-105 — `flex gap-3 mt-6` container, `px-4 py-2 text-sm border border-gray-300 rounded-md` for Back, blue variant for primary
|
||||
|
||||
```typescript
|
||||
// Bottom of ReviewStep JSX, before closing </div>
|
||||
<div className="flex gap-3 mt-6">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => dispatch({ type: 'SET_STEP', payload: 2 })}
|
||||
className="px-4 py-2 text-sm border border-gray-300 rounded-md hover:bg-gray-50"
|
||||
>
|
||||
Back
|
||||
</button>
|
||||
</div>
|
||||
```
|
||||
|
||||
Note: `dispatch` is not currently destructured from `useWizard()` in ReviewStep — must add it alongside `state`.
|
||||
|
||||
### Pattern 4: Registry Shape Enrichment (TECH-03)
|
||||
|
||||
**What:** Change `BACKEND_REGISTRY` value type from `FieldDef[]` to `{ displayName: string; description: string; fields: FieldDef[] }`
|
||||
|
||||
```typescript
|
||||
// registry.ts — new shape
|
||||
export const BACKEND_REGISTRY: Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }> = {
|
||||
azureblob: {
|
||||
displayName: 'Azure Blob Storage',
|
||||
description: 'Microsoft Azure cloud storage',
|
||||
fields: [ /* existing FieldDef array */ ],
|
||||
},
|
||||
s3: {
|
||||
displayName: 'Amazon S3',
|
||||
description: 'AWS Simple Storage Service',
|
||||
fields: [ /* existing FieldDef array */ ],
|
||||
},
|
||||
's3-compatible': {
|
||||
displayName: 'S3-Compatible',
|
||||
description: 'Wasabi, MinIO, Cloudflare R2, and others',
|
||||
fields: [ /* existing FieldDef array */ ],
|
||||
},
|
||||
};
|
||||
```
|
||||
|
||||
**Consumers that break and must be updated:**
|
||||
|
||||
1. `src/schemas/index.ts` line 10: `const fields = BACKEND_REGISTRY[backendType];` → `const fields = BACKEND_REGISTRY[backendType].fields;`
|
||||
2. `src/components/wizard/RemoteConfigStep.tsx` line 58: `BACKEND_REGISTRY.azureblob.find(...)` → `BACKEND_REGISTRY.azureblob.fields.find(...)`
|
||||
3. `src/components/wizard/RemoteConfigStep.tsx` line 73: `BACKEND_REGISTRY[backendType].map(...)` → `BACKEND_REGISTRY[backendType].fields.map(...)`
|
||||
|
||||
**BackendSelectionStep replacement:**
|
||||
|
||||
```typescript
|
||||
// Replace hardcoded BACKENDS const entirely
|
||||
// Object.entries preserves insertion order in V8 (azureblob first, consistent with WIZD-01 test)
|
||||
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
|
||||
<BackendCard
|
||||
key={type}
|
||||
name={entry.displayName}
|
||||
description={entry.description}
|
||||
selected={state.remote.backendType === type}
|
||||
onClick={() => handleCardClick(type as BackendType)}
|
||||
/>
|
||||
))}
|
||||
```
|
||||
|
||||
### Pattern 5: Fixing act() Warnings (TECH-05)
|
||||
|
||||
**Root cause confirmed in live test run:**
|
||||
- `BackendSelectionStep.test.tsx`: `fireEvent.click(azureButton)` triggers `handleSubmit` → async form validation → `dispatch` in store → state update in `WizardProvider` / `BackendSelectionStep` without act() wrapping
|
||||
- `ReviewStep.test.tsx`: `fireEvent.click(copyButtons[0])` triggers `handleCopy` → `setCopied(true)` → `setTimeout(() => setCopied(false), 2000)` — the timeout fires after test assertion, causing the `OutputBlock` warning
|
||||
|
||||
**Fix strategy for BackendSelectionStep.test.tsx:**
|
||||
Replace `fireEvent` with `userEvent` from `@testing-library/user-event`. `userEvent.setup()` + `await user.click()` wraps all state updates in `act()` automatically.
|
||||
|
||||
```typescript
|
||||
// Before (produces act() warnings)
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
fireEvent.click(azureButton);
|
||||
|
||||
// After (no act() warnings)
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
|
||||
const user = userEvent.setup();
|
||||
// ...
|
||||
await user.click(azureButton);
|
||||
```
|
||||
|
||||
**Fix strategy for ReviewStep act() warnings (OutputBlock setTimeout):**
|
||||
Two options (both valid, Claude's Discretion):
|
||||
1. Use `vi.useFakeTimers()` in `beforeEach` of the clipboard copy tests so `setTimeout` never fires unexpectedly
|
||||
2. Use `userEvent` for click interactions (same as BackendSelectionStep fix)
|
||||
|
||||
Option 1 (fake timers) is more surgical for the OutputBlock case since the setTimeout is a 2-second "Copied!" display reset — it's not a test concern.
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
|
||||
- **CSS-hiding OutputBlocks:** Tempting to add `hidden` class based on scriptTargets, but decision is locked to unmount — conditional render is the correct approach.
|
||||
- **Hardcoding the BACKENDS array after registry enrichment:** The whole point of TECH-03 is `Object.entries(BACKEND_REGISTRY)` — do not leave a parallel BACKENDS constant.
|
||||
- **Wrapping every fireEvent in act():** The correct fix is userEvent, not wrapping `fireEvent` calls manually — manual `act()` wrapping produces verbose, harder-to-maintain tests.
|
||||
- **Removing useMemo calls for conditionally rendered content:** `intuneInstall`, `intuneDetection`, `rmmScript` useMemo calls can stay even if the block is hidden — the computation is cheap and the memoized value is used in the ZIP handler too.
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| Act()-safe user interactions | Manual `act()` wrappers around fireEvent | `userEvent` from @testing-library/user-event | userEvent simulates real browser events and automatically wraps in act(); already installed |
|
||||
| Fake timers for setTimeout | Custom timer management | `vi.useFakeTimers()` / `vi.useRealTimers()` | Vitest built-in; handles all setTimeout/setInterval cleanly |
|
||||
| Registry-driven UI ordering | Custom sort/filter logic | `Object.entries()` (V8 insertion order) | Object.entries() on a `Record<BackendType, ...>` preserves declaration order — Azure first matches WIZD-01 requirement |
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1: Forgetting dispatch in ReviewStep
|
||||
**What goes wrong:** `ReviewStep` currently only destructures `state` from `useWizard()`. Adding a Back button requires `dispatch` too. Easy to miss.
|
||||
**Why it happens:** ReviewStep was read-only in v1.0 — no navigation actions needed.
|
||||
**How to avoid:** Change `const { state } = useWizard();` to `const { state, dispatch } = useWizard();` at the top of ReviewStep.
|
||||
|
||||
### Pitfall 2: registry.test.ts breaks after TECH-03
|
||||
**What goes wrong:** `registry.test.ts` accesses `BACKEND_REGISTRY[backend]` and calls `.length` and `.find()` directly on the registry entry — these will fail after the shape change to `{ displayName, description, fields }`.
|
||||
**Lines affected:** Lines 14 (`BACKEND_REGISTRY[backend].length`), 19 (`for (const field of BACKEND_REGISTRY[backend])`), 37/43/50 (`BACKEND_REGISTRY.azureblob.find(...)`, etc.)
|
||||
**How to avoid:** Update registry.test.ts to access `.fields` — e.g., `BACKEND_REGISTRY[backend].fields.length`, `BACKEND_REGISTRY[backend].fields.find(...)`.
|
||||
|
||||
### Pitfall 3: ReviewStep.test.tsx asserts exactly 4 OutputBlocks / 4 downloads
|
||||
**What goes wrong:** Tests DOWN-02, DOWN-03, DOWN-04, DOWN-05 use positional indexing (`downloadButtons[1]`, `downloadButtons[2]`, `downloadButtons[3]`) and `expect(files).toHaveLength(4)`. After TECH-01, when scriptTargets is both (default), 4 blocks still render — but tests for individual downloads will break if called with scriptTargets that hide blocks.
|
||||
**Current status:** Default `INITIAL_STATE` has `scriptTargets: ['intune', 'rmm']` so existing tests still pass with all 4 blocks. Tests specifically for filtered rendering are new (Wave 0 stubs needed).
|
||||
**How to avoid:** Existing ReviewStep tests can remain as-is (they use default state = both targets). Add new test cases for filtered scenarios (intune-only, rmm-only, neither) — these are new Wave 0 stubs.
|
||||
|
||||
### Pitfall 4: BackendSelectionStep.test.tsx assertions on card text remain valid
|
||||
**What goes wrong:** After TECH-03, tests check for 'Azure Blob Storage', 'Amazon S3', 'S3-Compatible' text. These strings move from the hardcoded `BACKENDS` array to `BACKEND_REGISTRY[type].displayName`. As long as displayName values match, tests pass without any change.
|
||||
**How to avoid:** Use the exact same display strings when adding `displayName` to registry entries.
|
||||
|
||||
### Pitfall 5: TypeScript error after removing BackendFormValues<T>
|
||||
**What goes wrong:** If any file imports `BackendFormValues` from `src/schemas/index.ts`, removing it causes a TS compile error.
|
||||
**Evidence:** Code audit found no consumers — `BackendFormValues` only appears in `src/schemas/index.ts` lines 27-28. But TypeScript compile check is the definitive verification step.
|
||||
**How to avoid:** After removal, run `tsc --noEmit` or `npm run build` to confirm zero errors.
|
||||
|
||||
### Pitfall 6: userEvent requires setup() call
|
||||
**What goes wrong:** `userEvent.click()` was called directly in older versions of @testing-library/user-event. In v14 (installed: 14.6.1), the correct API is `const user = userEvent.setup()` then `await user.click()`.
|
||||
**How to avoid:** Always use `userEvent.setup()` pattern — not the legacy `userEvent.click()` shorthand.
|
||||
|
||||
## Code Examples
|
||||
|
||||
### Verified pattern: userEvent v14 API
|
||||
```typescript
|
||||
// Source: @testing-library/user-event v14 official API
|
||||
import userEvent from '@testing-library/user-event';
|
||||
|
||||
describe('BackendSelectionStep', () => {
|
||||
it('clicking a backend card dispatches SET_BACKEND_TYPE and SET_STEP', async () => {
|
||||
const user = userEvent.setup();
|
||||
renderStep();
|
||||
const nameInput = screen.getByRole('textbox');
|
||||
await user.type(nameInput, 'my-remote');
|
||||
const azureButton = screen.getAllByRole('button').find(b => b.textContent?.includes('Azure Blob Storage'))!;
|
||||
await user.click(azureButton);
|
||||
expect(screen.queryByRole('alert')).toBeNull();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Verified pattern: vi.useFakeTimers() for setTimeout-based act() warnings
|
||||
```typescript
|
||||
// Source: Vitest official docs — timer mocking
|
||||
import { beforeEach, afterEach, vi } from 'vitest';
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('clicking Copy calls clipboard.writeText', async () => {
|
||||
renderStep();
|
||||
const checkbox = screen.getByRole('checkbox');
|
||||
fireEvent.click(checkbox);
|
||||
const copyButtons = screen.getAllByText('Copy');
|
||||
fireEvent.click(copyButtons[0]);
|
||||
expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1);
|
||||
// setTimeout(() => setCopied(false)) never fires during test — no act() warning
|
||||
});
|
||||
```
|
||||
|
||||
### Verified pattern: Object.entries() on enriched registry
|
||||
```typescript
|
||||
// Source: MDN Object.entries() — insertion order preserved for string keys
|
||||
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
|
||||
<BackendCard
|
||||
key={type}
|
||||
name={entry.displayName}
|
||||
description={entry.description}
|
||||
selected={state.remote.backendType === type}
|
||||
onClick={() => handleCardClick(type as BackendType)}
|
||||
/>
|
||||
))}
|
||||
```
|
||||
|
||||
## State of the Art
|
||||
|
||||
| Old Approach | Current Approach | When Changed | Impact |
|
||||
|--------------|------------------|--------------|--------|
|
||||
| `fireEvent` for all test interactions | `userEvent.setup()` + `await user.click()` | @testing-library/user-event v14 | Eliminates act() warnings from async state updates triggered by simulated events |
|
||||
| `BACKEND_REGISTRY: Record<BackendType, FieldDef[]>` | `Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }>` | Phase 5 (TECH-03) | BackendSelectionStep no longer needs a parallel BACKENDS array; Phase 6 additions auto-surface |
|
||||
|
||||
**Deprecated/outdated:**
|
||||
- `fireEvent` for click interactions that trigger state updates: still works but causes act() warnings; replace with userEvent where state updates occur
|
||||
- Hardcoded `BACKENDS` array in `BackendSelectionStep.tsx`: dead after TECH-03; replaced by `Object.entries(BACKEND_REGISTRY)`
|
||||
- `BackendFormValues<T>` type export: dead code — no consumers exist; removed in TECH-04
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **ReviewStep.test.tsx: DOWN-05 asserts `files.toHaveLength(4)` — does this test need updating?**
|
||||
- What we know: Default INITIAL_STATE has scriptTargets = ['intune', 'rmm'], so with default state the ZIP still gets 4 files.
|
||||
- What's unclear: Should the TECH-01 implementation also add new test cases covering filtered scenarios (2 files for intune-only, etc.)?
|
||||
- Recommendation: Keep the existing DOWN-05 test intact (it documents correct behavior when both targets are selected). Add new test cases in Wave 0 for filtered scenarios (intune-only, rmm-only, neither).
|
||||
|
||||
2. **act() warnings in ReviewStep.test.tsx — fix in this phase or leave?**
|
||||
- What we know: TECH-05 requirement text says "BackendSelectionStep test suite" specifically. ReviewStep warnings are from OutputBlock's setTimeout.
|
||||
- What's unclear: Whether the phase goal implicitly includes ReviewStep warnings given CONTEXT.md says "eliminate act() warnings from the BackendSelectionStep test suite."
|
||||
- Recommendation: TECH-05 scope is BackendSelectionStep. Fix ReviewStep act() warnings as a bonus in the same task (low effort — add `vi.useFakeTimers()` to the relevant ReviewStep tests). If time-constrained, BackendSelectionStep warnings are the acceptance criterion.
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | Vitest 4.1.1 |
|
||||
| Config file | `vitest.config.ts` (root) |
|
||||
| Quick run command | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx src/components/wizard/ReviewStep.test.tsx src/schemas/registry.test.ts` |
|
||||
| Full suite command | `npx vitest run` |
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||
|--------|----------|-----------|-------------------|-------------|
|
||||
| TECH-01 | Intune blocks absent when scriptTargets excludes 'intune' | unit | `npx vitest run src/components/wizard/ReviewStep.test.tsx` | ❌ Wave 0 (new cases needed) |
|
||||
| TECH-01 | RMM block absent when scriptTargets excludes 'rmm' | unit | `npx vitest run src/components/wizard/ReviewStep.test.tsx` | ❌ Wave 0 (new cases needed) |
|
||||
| TECH-01 | ZIP contains only rclone.conf when both targets deselected | unit | `npx vitest run src/components/wizard/ReviewStep.test.tsx` | ❌ Wave 0 (new cases needed) |
|
||||
| TECH-02 | Back button dispatches SET_STEP(2) | unit | `npx vitest run src/components/wizard/ReviewStep.test.tsx` | ❌ Wave 0 (new case needed) |
|
||||
| TECH-03 | BackendSelectionStep renders cards from BACKEND_REGISTRY | unit | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx` | ✅ (existing assertions still valid) |
|
||||
| TECH-03 | registry.test.ts field access via .fields | unit | `npx vitest run src/schemas/registry.test.ts` | ❌ Wave 0 (update existing) |
|
||||
| TECH-04 | No TypeScript error after BackendFormValues removal | type-check | `npx tsc --noEmit` | ✅ (compile check) |
|
||||
| TECH-05 | BackendSelectionStep test suite zero act() warnings | unit | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx 2>&1 \| grep -c "act("` | ✅ (existing tests, modified) |
|
||||
|
||||
### Sampling Rate
|
||||
- **Per task commit:** `npx vitest run` (full suite — only 3.4s, fast enough for every commit)
|
||||
- **Per wave merge:** `npx vitest run` + `npx tsc --noEmit`
|
||||
- **Phase gate:** Full suite green + zero TypeScript errors before `/gsd:verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
|
||||
- [ ] `src/components/wizard/ReviewStep.test.tsx` — add cases for TECH-01: intune-only, rmm-only, neither (requires WizardProvider accepting initial state, or dispatching SET_DEPLOYMENT in test setup)
|
||||
- [ ] `src/components/wizard/ReviewStep.test.tsx` — add case for TECH-02: Back button renders and dispatches SET_STEP(2)
|
||||
- [ ] `src/schemas/registry.test.ts` — update field access to `.fields` after TECH-03 shape change (existing tests break without this update)
|
||||
|
||||
**Note on ReviewStep test state injection:** Current `renderStep()` wraps with bare `WizardProvider` which uses `INITIAL_STATE`. To test filtered rendering, tests need a way to render with custom deployment state. Options: (a) export a `WizardProvider` variant accepting `initialState` prop, or (b) render and dispatch `SET_DEPLOYMENT` in test setup before assertions. Option (b) requires no code changes to the provider — preferred.
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence)
|
||||
- Direct code audit of `src/components/wizard/ReviewStep.tsx` — confirmed no dispatch, no Back button, always 4 OutputBlocks
|
||||
- Direct code audit of `src/components/wizard/BackendSelectionStep.tsx` — confirmed hardcoded BACKENDS array
|
||||
- Direct code audit of `src/schemas/registry.ts` — confirmed current shape is `Record<BackendType, FieldDef[]>`
|
||||
- Direct code audit of `src/schemas/index.ts` — confirmed BackendFormValues<T> at lines 27-28 with no consumers
|
||||
- Live test run (`npx vitest run`) — confirmed act() warnings present in both BackendSelectionStep and ReviewStep test files, all 98 tests passing
|
||||
- `src/store/types.ts` — confirmed SET_STEP action exists, scriptTargets in WizardState.deployment
|
||||
- `package.json` — confirmed @testing-library/user-event 14.6.1 already installed
|
||||
|
||||
### Secondary (MEDIUM confidence)
|
||||
- @testing-library/user-event v14 API: `userEvent.setup()` pattern verified against installed version and established community usage
|
||||
|
||||
### Tertiary (LOW confidence)
|
||||
- None
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Standard stack: HIGH — all libraries already in use, confirmed from package.json
|
||||
- Architecture: HIGH — all patterns derived from direct code audit of existing files
|
||||
- Pitfalls: HIGH — most pitfalls discovered by cross-referencing actual test files with planned changes
|
||||
- act() fix: HIGH — userEvent.setup() pattern confirmed against installed v14.6.1
|
||||
|
||||
**Research date:** 2026-03-30
|
||||
**Valid until:** 2026-05-30 (stable libraries — React 18, Vitest 4, userEvent 14)
|
||||
Reference in New Issue
Block a user