docs(05-tech-debt): create phase 5 plan — 4 plans across 2 waves
Wave 0 TDD stubs, Wave 1 parallel (registry + ReviewStep), Wave 2 act() fix. Covers TECH-01 through TECH-05. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 4.6
@@ -0,0 +1,210 @@
|
||||
---
|
||||
phase: 05-tech-debt
|
||||
plan: "03"
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "05-01"
|
||||
- "05-02"
|
||||
files_modified:
|
||||
- src/components/wizard/BackendSelectionStep.test.tsx
|
||||
- src/components/wizard/ReviewStep.test.tsx
|
||||
autonomous: true
|
||||
requirements:
|
||||
- TECH-05
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "BackendSelectionStep test suite runs with zero act() warnings in Vitest output"
|
||||
- "ReviewStep test suite runs with zero act() warnings (bonus — low-effort fix)"
|
||||
- "All previously passing tests remain green after the fireEvent-to-userEvent migration"
|
||||
artifacts:
|
||||
- path: "src/components/wizard/BackendSelectionStep.test.tsx"
|
||||
provides: "userEvent-based interactions replacing fireEvent"
|
||||
contains: "userEvent.setup()"
|
||||
- path: "src/components/wizard/ReviewStep.test.tsx"
|
||||
provides: "vi.useFakeTimers() for OutputBlock setTimeout warnings"
|
||||
contains: "vi.useFakeTimers"
|
||||
key_links:
|
||||
- from: "BackendSelectionStep.test.tsx"
|
||||
to: "userEvent v14 API"
|
||||
via: "const user = userEvent.setup(); await user.click()"
|
||||
pattern: "userEvent\\.setup\\(\\)"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Fix act() warnings in BackendSelectionStep.test.tsx by migrating from fireEvent to userEvent v14, and suppress ReviewStep act() warnings with vi.useFakeTimers() for the OutputBlock setTimeout.
|
||||
|
||||
Purpose: TECH-05 — clean test output with zero act() warnings makes CI signal trustworthy and prevents false positives.
|
||||
Output: Both test files use act()-safe patterns; no production code changes.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@C:/Users/SebastienQUEROL/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@C:/Users/SebastienQUEROL/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/phases/05-tech-debt/05-CONTEXT.md
|
||||
@.planning/phases/05-tech-debt/05-RESEARCH.md
|
||||
@.planning/phases/05-tech-debt/05-01-SUMMARY.md
|
||||
@.planning/phases/05-tech-debt/05-02-SUMMARY.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Key patterns — extracted from RESEARCH.md -->
|
||||
|
||||
userEvent v14 API (already installed: @testing-library/user-event 14.6.1):
|
||||
```typescript
|
||||
import userEvent from '@testing-library/user-event';
|
||||
|
||||
// Per test or per describe block
|
||||
const user = userEvent.setup();
|
||||
// ...
|
||||
await user.click(element); // replaces fireEvent.click(element)
|
||||
await user.type(input, text); // replaces fireEvent.change(input, { target: { value: text } })
|
||||
```
|
||||
|
||||
The user setup() instance must be created INSIDE describe() or it() (not at module level) to avoid state leakage between tests.
|
||||
|
||||
BackendSelectionStep.test.tsx — which fireEvent calls to migrate:
|
||||
- fireEvent.change(nameInput, { target: { value: '...' } }) → await user.type(nameInput, '...')
|
||||
NOTE: user.type() appends to existing value. If input has a defaultValue, use user.clear() first or set up user.type with full value.
|
||||
Alternative: fireEvent.change is safe for setting values (no state updates) — only fireEvent.click triggers async state. Can keep fireEvent.change and only replace fireEvent.click.
|
||||
- fireEvent.click(azureButton) → await user.click(azureButton)
|
||||
- All it() callbacks that contain await user.click() MUST be async
|
||||
|
||||
vi.useFakeTimers pattern for ReviewStep (for OutputBlock's setCopied setTimeout):
|
||||
```typescript
|
||||
import { beforeEach, afterEach, vi } from 'vitest';
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
// Note: existing beforeEach in ReviewStep.test.tsx runs vi.clearAllMocks() and sets up stubs
|
||||
// Add vi.useFakeTimers() call to the EXISTING beforeEach block (not a new one)
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
```
|
||||
|
||||
Root cause reference:
|
||||
- BackendSelectionStep: fireEvent.click on a backend card → handleSubmit → async React Hook Form validation → dispatch → WizardProvider state update — all without act() wrapping
|
||||
- ReviewStep: fireEvent.click on Copy button → setCopied(true) → setTimeout(() => setCopied(false), 2000) — timer fires after test assertion, producing act() warning from OutputBlock
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Migrate BackendSelectionStep.test.tsx from fireEvent to userEvent</name>
|
||||
<files>src/components/wizard/BackendSelectionStep.test.tsx</files>
|
||||
<behavior>
|
||||
- All WIZD-01 and WIZD-04 tests continue to pass (same assertions, just act()-safe interactions)
|
||||
- Vitest output for BackendSelectionStep.test.tsx contains zero occurrences of "act(" in warnings
|
||||
- Tests that previously used await waitFor() still use it (async validation is still async)
|
||||
- Tests that clicked backend cards use await user.click() and are async
|
||||
</behavior>
|
||||
<action>
|
||||
Edit src/components/wizard/BackendSelectionStep.test.tsx:
|
||||
|
||||
1. Remove fireEvent from the @testing-library/react import (keep render, screen, waitFor)
|
||||
2. Add import: `import userEvent from '@testing-library/user-event';`
|
||||
3. For each it() that calls fireEvent.click on a backend card button, convert to userEvent pattern:
|
||||
- Add `const user = userEvent.setup();` at the top of the it() body
|
||||
- Make the it() callback async
|
||||
- Replace `fireEvent.click(azureButton)` with `await user.click(azureButton)`
|
||||
- Replace `fireEvent.change(nameInput, { target: { value: 'my-remote' } })` with `await user.clear(nameInput); await user.type(nameInput, 'my-remote');`
|
||||
OR keep fireEvent.change for input value setting and only replace fireEvent.click (fireEvent.change does not trigger act()-needing state updates in this component — either approach is valid)
|
||||
|
||||
Specific tests to migrate (all 5 tests in BackendSelectionStep.test.tsx that use fireEvent):
|
||||
- WIZD-01: 'clicking a backend card dispatches SET_BACKEND_TYPE and SET_STEP'
|
||||
- WIZD-04: 'shows inline error after first Next attempt with invalid name'
|
||||
- WIZD-04: 'accepts alphanumeric, dashes, and underscores'
|
||||
- WIZD-04: 'rejects names with spaces or special characters'
|
||||
- WIZD-04: 'renders remote name input at the top of the step' (if it uses fireEvent — check)
|
||||
|
||||
Tests that only use screen queries and no interactions (e.g., 'renders Azure Blob Storage card') do NOT need changes.
|
||||
|
||||
Anti-patterns to avoid:
|
||||
- Do NOT wrap fireEvent.click in act() manually — replace with userEvent instead
|
||||
- Do NOT use legacy `userEvent.click()` shorthand — always use `userEvent.setup()` + `await user.click()`
|
||||
- userEvent.setup() must be inside the test body, not at module level
|
||||
|
||||
After changes, verify no fireEvent.click calls remain in the file.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /c/Users/SebastienQUEROL/Documents/projets/Ready2Blob && npx vitest run src/components/wizard/BackendSelectionStep.test.tsx 2>&1 | grep -E "act\(|PASS|FAIL|Tests" | head -20</automated>
|
||||
</verify>
|
||||
<done>
|
||||
All BackendSelectionStep tests pass.
|
||||
Zero lines containing "act(" appear in the warning output.
|
||||
Grep for "act(" returns 0 matches in test output.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Fix ReviewStep act() warnings with vi.useFakeTimers</name>
|
||||
<files>src/components/wizard/ReviewStep.test.tsx</files>
|
||||
<action>
|
||||
Edit src/components/wizard/ReviewStep.test.tsx:
|
||||
|
||||
1. Add `afterEach` to the existing imports from vitest: `import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';`
|
||||
|
||||
2. Add `vi.useFakeTimers()` at the start of the existing `beforeEach` block (before vi.clearAllMocks()):
|
||||
```typescript
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers(); // ADD THIS LINE FIRST
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(downloadZip).mockResolvedValue(undefined);
|
||||
vi.stubGlobal('navigator', { clipboard: { writeText: vi.fn().mockResolvedValue(undefined) } });
|
||||
vi.stubGlobal('URL', { createObjectURL: vi.fn(() => 'blob:mock'), revokeObjectURL: vi.fn() });
|
||||
});
|
||||
```
|
||||
|
||||
3. Add a new `afterEach` block after beforeEach:
|
||||
```typescript
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
```
|
||||
|
||||
This suppresses the act() warning from OutputBlock's `setTimeout(() => setCopied(false), 2000)` — the timer is frozen during tests and never fires unexpectedly.
|
||||
|
||||
No other changes needed — all existing assertions remain valid with fake timers active.
|
||||
The new TECH-01 and TECH-02 tests from Plan 00 also benefit from this change (no warnings from their interactions either).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /c/Users/SebastienQUEROL/Documents/projets/Ready2Blob && npx vitest run src/components/wizard/ReviewStep.test.tsx 2>&1 | grep -E "act\(|Tests" | head -10</automated>
|
||||
</verify>
|
||||
<done>
|
||||
All ReviewStep tests pass (including TECH-01 and TECH-02 new cases from Plan 00).
|
||||
Zero lines containing "act(" appear in the test output.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
Full suite must be entirely green with zero act() warnings:
|
||||
```
|
||||
cd /c/Users/SebastienQUEROL/Documents/projets/Ready2Blob && npx vitest run 2>&1 | grep -E "act\(|Test Files|Tests"
|
||||
```
|
||||
Expected:
|
||||
- "Tests X passed (X)" with no failures
|
||||
- Zero lines containing "act("
|
||||
- npx tsc --noEmit: zero errors
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- BackendSelectionStep.test.tsx uses userEvent.setup() + await user.click() for all card interactions
|
||||
- ReviewStep.test.tsx uses vi.useFakeTimers() in beforeEach and vi.useRealTimers() in afterEach
|
||||
- Full Vitest run produces zero "act(" warning lines
|
||||
- All 5 requirements (TECH-01 through TECH-05) verified green in the full test suite
|
||||
- npx tsc --noEmit: zero errors (phase gate)
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/05-tech-debt/05-03-SUMMARY.md`
|
||||
</output>
|
||||
Reference in New Issue
Block a user