|
| 1 | +# Sketch Test Fix Plan |
| 2 | + |
| 3 | +## Problem Identified ✅ |
| 4 | + |
| 5 | +The Designer page dynamically creates a canvas via JavaScript (`designer.js`), not in the Razor markup. Tests timeout because they expect immediate canvas availability. |
| 6 | + |
| 7 | +**Root Cause:** |
| 8 | +- Designer.razor loads JavaScript module in `OnAfterRenderAsync` |
| 9 | +- Canvas is created inside `<div id="designer-stage">` by JavaScript |
| 10 | +- Tests looking for `<canvas>` immediately will fail |
| 11 | +- Need to wait for: page load → JS module → canvas creation |
| 12 | + |
| 13 | +## Solution Implemented |
| 14 | + |
| 15 | +### 1. Helper Function Created ✅ |
| 16 | +```typescript |
| 17 | +async function waitForDesignerReady(page: any) { |
| 18 | + await page.waitForLoadState('networkidle'); |
| 19 | + await page.locator('#designer-stage').waitFor({ state: 'visible', timeout: 30000 }); |
| 20 | + await page.waitForTimeout(3000); // JS initialization |
| 21 | + await page.locator('#designer-stage canvas').first().waitFor({ state: 'attached', timeout: 15000 }).catch(() => {}); |
| 22 | +} |
| 23 | +``` |
| 24 | + |
| 25 | +### 2. Tests Fixed (3/49) ✅ |
| 26 | +- ✅ `draw line on canvas with mouse` |
| 27 | +- ✅ `draw rectangle on canvas` |
| 28 | +- ✅ `draw circle on canvas` |
| 29 | + |
| 30 | +### 3. Tests Partially Fixed (2/49) ✅ |
| 31 | +- ✅ `designer toolbar loads correctly` - added designer-stage wait |
| 32 | +- ✅ `canvas loads and is interactive` - updated canvas selector |
| 33 | + |
| 34 | +## Remaining Work |
| 35 | + |
| 36 | +### 45 Tests Need Update |
| 37 | + |
| 38 | +All tests that navigate to `/designer/{jobId}` need two changes: |
| 39 | + |
| 40 | +**Before:** |
| 41 | +```typescript |
| 42 | +await page.goto(`/designer/${jobId}`); |
| 43 | +await page.waitForLoadState('networkidle'); |
| 44 | +const canvas = page.locator('canvas').first(); |
| 45 | +``` |
| 46 | + |
| 47 | +**After:** |
| 48 | +```typescript |
| 49 | +await page.goto(`/designer/${jobId}`); |
| 50 | +await waitForDesignerReady(page); |
| 51 | +const canvas = page.locator('#designer-stage canvas').first(); |
| 52 | +``` |
| 53 | + |
| 54 | +### Tests to Update |
| 55 | + |
| 56 | +**Keyboard Shortcuts (8 tests):** |
| 57 | +- keyboard shortcut - V for select tool |
| 58 | +- keyboard shortcut - L for line tool |
| 59 | +- keyboard shortcut - R for rectangle tool |
| 60 | +- keyboard shortcut - C for circle tool |
| 61 | +- keyboard shortcut - G for grid toggle |
| 62 | +- keyboard shortcut - Ctrl/Cmd+Z for undo |
| 63 | +- keyboard shortcut - Ctrl/Cmd+Y for redo |
| 64 | + |
| 65 | +**Export Tests (4 tests):** |
| 66 | +- export PNG and verify download |
| 67 | +- export JSON and verify structure |
| 68 | +- export SVG and verify format |
| 69 | +- export DXF and verify download |
| 70 | + |
| 71 | +**Advanced Features (20+ tests):** |
| 72 | +- change sketch units |
| 73 | +- set sketch name |
| 74 | +- save sketch with name |
| 75 | +- view saved sketches list |
| 76 | +- open existing sketch |
| 77 | +- toggle grid button |
| 78 | +- toggle angle snap |
| 79 | +- autosave dropdown functionality |
| 80 | +- export buttons are present |
| 81 | +- undo/redo buttons functionality |
| 82 | +- reset view button |
| 83 | +- hotkeys help toggle |
| 84 | +- metrics calculation - length and area |
| 85 | +- autosave functionality |
| 86 | +- grid snap behavior |
| 87 | +- angle snap at 15 degree intervals |
| 88 | +- dimension tool adds measurements |
| 89 | +- toggle dimensions visibility |
| 90 | +- toggle shapes visibility |
| 91 | +- draft recovery on page reload |
| 92 | +- discard draft |
| 93 | +- scale input affects drawing units |
| 94 | + |
| 95 | +## Automated Fix Script |
| 96 | + |
| 97 | +Create `scripts/fix-sketch-tests.sh`: |
| 98 | + |
| 99 | +```bash |
| 100 | +#!/bin/bash |
| 101 | +# Replace waitForLoadState with waitForDesignerReady for designer pages |
| 102 | + |
| 103 | +sed -i '' 's/await page.goto(`\/designer\/${jobId}`);\\n await page.waitForLoadState('\''networkidle'\'');/await page.goto(`\/designer\/${jobId}`);\\n await waitForDesignerReady(page);/g' tests/ui.sketches.spec.ts |
| 104 | + |
| 105 | +# Fix canvas selectors |
| 106 | +sed -i '' 's/page.locator('\''canvas'\'').first()/page.locator('\''#designer-stage canvas'\'').first()/g' tests/ui.sketches.spec.ts |
| 107 | + |
| 108 | +echo "✅ Fixed sketch tests" |
| 109 | +``` |
| 110 | + |
| 111 | +## Manual Fix Template |
| 112 | + |
| 113 | +For each test navigating to designer, apply this pattern: |
| 114 | + |
| 115 | +```typescript |
| 116 | +test('test name', async ({ page, request }) => { |
| 117 | + const jobTitle = `Test ${Date.now()}`; |
| 118 | + const jobId = await createTestJob(request, jobTitle); |
| 119 | + |
| 120 | + // OLD: |
| 121 | + // await page.goto(`/designer/${jobId}`); |
| 122 | + // await page.waitForLoadState('networkidle'); |
| 123 | + |
| 124 | + // NEW: |
| 125 | + await page.goto(`/designer/${jobId}`); |
| 126 | + await waitForDesignerReady(page); |
| 127 | + |
| 128 | + // If test uses canvas: |
| 129 | + // OLD: const canvas = page.locator('canvas').first(); |
| 130 | + // NEW: |
| 131 | + const canvas = page.locator('#designer-stage canvas').first(); |
| 132 | + |
| 133 | + // Rest of test... |
| 134 | +}); |
| 135 | +``` |
| 136 | + |
| 137 | +## Expected Outcome |
| 138 | + |
| 139 | +**Current State:** |
| 140 | +- 190/342 tests passing (55.6%) |
| 141 | +- 44/49 sketch tests failing (89.8% failure) |
| 142 | + |
| 143 | +**After Full Fix:** |
| 144 | +- ~234/342 tests passing (68.4%) |
| 145 | +- 45+/49 sketch tests passing (92%+) |
| 146 | + |
| 147 | +**Time to Fix:** |
| 148 | +- Manual: ~2-3 hours (45 tests × 3-4 min each) |
| 149 | +- Script: ~5 minutes |
| 150 | +- Testing: ~15 minutes to run full suite |
| 151 | + |
| 152 | +## Recommendation |
| 153 | + |
| 154 | +### Option 1: Automated Script (Recommended) |
| 155 | +1. Create and run the sed script above |
| 156 | +2. Manually verify 5-10 random tests |
| 157 | +3. Run full test suite |
| 158 | +4. Fix any edge cases |
| 159 | + |
| 160 | +**Pros:** Fast, consistent, less error-prone |
| 161 | +**Cons:** Need to verify script correctness |
| 162 | + |
| 163 | +### Option 2: Manual Fix |
| 164 | +1. Fix tests in batches of 10 |
| 165 | +2. Test after each batch |
| 166 | +3. Commit incrementally |
| 167 | + |
| 168 | +**Pros:** More control, can test as you go |
| 169 | +**Cons:** Time-consuming, potential for inconsistency |
| 170 | + |
| 171 | +### Option 3: Semi-Automated (Best) |
| 172 | +1. Use multi-edit tool to batch-fix similar tests |
| 173 | +2. Group by category (keyboard shortcuts, exports, etc.) |
| 174 | +3. Test each category after fixing |
| 175 | + |
| 176 | +**Pros:** Balance of speed and safety |
| 177 | +**Cons:** Still requires some manual work |
| 178 | + |
| 179 | +## Next Immediate Steps |
| 180 | + |
| 181 | +1. **Choose fix strategy** (recommend semi-automated) |
| 182 | + |
| 183 | +2. **Fix keyboard shortcut tests first** (8 tests, similar pattern): |
| 184 | + ```bash |
| 185 | + # All follow same pattern, can batch-fix |
| 186 | + ``` |
| 187 | + |
| 188 | +3. **Then fix export tests** (4 tests, also similar) |
| 189 | + |
| 190 | +4. **Then remaining advanced features** (20+ tests) |
| 191 | + |
| 192 | +5. **Run test suite** after each batch: |
| 193 | + ```bash |
| 194 | + npm run test:e2e -- tests/ui.sketches.spec.ts -g "keyboard shortcut" |
| 195 | + ``` |
| 196 | + |
| 197 | +6. **Commit incrementally:** |
| 198 | + ```bash |
| 199 | + git add tests/ui.sketches.spec.ts |
| 200 | + git commit -m "Fix keyboard shortcut tests - use waitForDesignerReady" |
| 201 | + ``` |
| 202 | + |
| 203 | +## Success Criteria |
| 204 | + |
| 205 | +- ✅ All 49 sketch tests use `waitForDesignerReady()` |
| 206 | +- ✅ All canvas references use `#designer-stage canvas` |
| 207 | +- ✅ 45+ sketch tests passing (92%+) |
| 208 | +- ✅ Tests complete in < 5 minutes |
| 209 | +- ✅ No flaky tests (consistent results) |
| 210 | + |
| 211 | +## Related Issues to Fix After |
| 212 | + |
| 213 | +Once sketch tests are passing, tackle: |
| 214 | + |
| 215 | +1. **Ledger tests** (9 failing) - company selector timeouts |
| 216 | +2. **Accounting tests** (24 skipped) - determine why skipped |
| 217 | +3. **Onboarding tests** (6 skipped) - flaky/environmental |
| 218 | +4. **Target:** 90%+ pass rate overall (308+ tests) |
0 commit comments