Skip to content

Commit bf0d8fd

Browse files
authored
Merge pull request #22 from nirukk52/final/green-tests-to-main
Final/green tests to main
2 parents 38763c7 + 12ad115 commit bf0d8fd

File tree

21 files changed

+1082
-641
lines changed

21 files changed

+1082
-641
lines changed

.claude-skills/frontend-debugging_skill/SKILL.md

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,10 @@ task founder:workflows:regen-client
3535
```
3636

3737
### Phase 4: Svelte 5 Runes
38-
- Check proper rune usage
39-
- $state for reactive state
40-
- $derived for computed values
41-
- $effect for side effects
42-
- $props for component props
38+
- Check proper rune usage **and declaration**
39+
- `$state`, `$derived`, `$bindable`, `$props` that you mutate must use `let` (never `const`)
40+
- `$bindable` requires `{value}` with `oninput` instead of `bind:value`
41+
- `$effect` for side effects only (no async return)
4342

4443
### Phase 5: Routing
4544
- Verify +page.svelte structure
@@ -74,11 +73,25 @@ task frontend:build
7473

7574
## Common Issues
7675

76+
### Rune Const vs Let (Critical 2025-11-12)
77+
- **Symptom**: Browser shows Vite overlay `[plugin:vite-plugin-svelte] Cannot assign/bind to constant` and Playwright screenshots display the 500 Internal Error page.
78+
- **Fix workflow**:
79+
1. Search `src/lib` and `src/routes` for `const .* $state`, `const .* $bindable`, `const { .* = $props`.
80+
2. Replace with `let` if the value is mutated or bound.
81+
3. Restart `bun run dev` to clear cached overlay.
82+
4. Re-run `grep -R "\\$state" src | grep 'const'` to confirm there are no remaining matches.
83+
- **Remember**: `$bindable` inputs must use `{value}` + `oninput={(e) => value = e.currentTarget.value}` (no `bind:value`).
84+
7785
### Rune Misuse
7886
- Can't use runes in `.ts` files (only `.svelte`)
7987
- Must be top-level declarations
8088
- No conditional runes
8189

90+
### Fast-Fail Signals
91+
- Landing page sticks at `page.waitForURL` → backend `startRun` likely failed (check backend logs, ensure Appium/device online).
92+
- Timeline shows `agent.app.launch_failed` → Appium/device misconfiguration (fails in ~2s with detailed payload).
93+
- Backend integration test `encore test run/start.integration.test.ts` must pass before Playwright has a chance to render screenshots.
94+
8295
### API Type Errors
8396
- Regenerate client after backend changes
8497
- Verify import paths use `~encore/clients`

.claude-skills/frontend-development_skill/SKILL.md

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,121 @@ console.log('User logged in:', user);
606606
- Cause: Client not regenerated after backend changes
607607
- Fix: `cd frontend && bun run gen`
608608

609+
**5. "Cannot assign to constant" with $props() or $bindable()**
610+
- **Cause:** Using `const` instead of `let` in destructuring assignment with Svelte 5 runes
611+
- **Symptom:** `[plugin:vite-plugin-svelte] Cannot assign to constant https://svelte.dev/e/constant_assignment`
612+
- **Fix:** Use `let` for destructuring when you need to reassign values
613+
614+
```typescript
615+
// ❌ WRONG - Cannot mutate const
616+
const {
617+
value = $bindable(""),
618+
items = $state([])
619+
} = $props();
620+
621+
// ✅ CORRECT - Use let for mutable runes
622+
let {
623+
value = $bindable(""),
624+
items = $state([])
625+
} = $props();
626+
627+
// For $bindable in components:
628+
let {
629+
value = $bindable("")
630+
} = $props();
631+
632+
// Then in template:
633+
<input
634+
{value}
635+
oninput={(e) => {
636+
value = e.currentTarget.value; // This requires 'let'
637+
}}
638+
/>
639+
```
640+
641+
**6. "Cannot bind to constant" with bind: directives**
642+
- **Cause:** Trying to use `bind:value` with $bindable() (Svelte 5 two-way binding)
643+
- **Fix:** Use value + oninput pattern instead
644+
645+
```svelte
646+
<!-- ❌ WRONG - Can't use bind: with $bindable -->
647+
<input bind:value />
648+
649+
<!-- ✅ CORRECT - Manual two-way binding -->
650+
<input
651+
{value}
652+
oninput={(e) => {
653+
value = e.currentTarget.value;
654+
oninput?.(e); // Call optional handler
655+
}}
656+
/>
657+
```
658+
659+
**7. Missing page title in E2E tests**
660+
- **Cause:** No `<svelte:head>` with `<title>` in root layout
661+
- **Symptom:** Playwright tests fail with `expected page to have title`
662+
- **Fix:** Add title in root layout AFTER closing `</script>` tag
663+
664+
```svelte
665+
<script lang="ts">
666+
// ... your code
667+
</script>
668+
669+
<svelte:head>
670+
<title>ScreenGraph</title>
671+
</svelte:head>
672+
673+
<!-- rest of layout -->
674+
```
675+
676+
---
677+
678+
## Svelte 5 Runes Critical Rules
679+
680+
### $props() Destructuring
681+
682+
**ALWAYS use `let` not `const`:**
683+
```typescript
684+
// ❌ BAD - causes "cannot assign to constant"
685+
const { value = $bindable("") } = $props();
686+
687+
// ✅ GOOD
688+
let { value = $bindable("") } = $props();
689+
```
690+
691+
### $bindable() Two-Way Binding
692+
693+
**Cannot use `bind:` directive:**
694+
```svelte
695+
<!-- ❌ BAD -->
696+
<script lang="ts">
697+
let { value = $bindable("") } = $props();
698+
</script>
699+
<input bind:value />
700+
701+
<!-- ✅ GOOD -->
702+
<script lang="ts">
703+
let { value = $bindable("") } = $props();
704+
</script>
705+
<input
706+
{value}
707+
oninput={(e) => value = e.currentTarget.value}
708+
/>
709+
```
710+
711+
### $state() Reactive Variables
712+
713+
**Use `let` for mutable state:**
714+
```typescript
715+
// ❌ BAD
716+
const count = $state(0);
717+
count++; // Error: cannot assign
718+
719+
// ✅ GOOD
720+
let count = $state(0);
721+
count++; // Works!
722+
```
723+
609724
---
610725

611726
## Quality Checklist

.claude-skills/webapp-testing_skill/SKILL.md

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,35 +32,50 @@ bun run test:e2e:ui
3232
- **Helpers**: `frontend/tests/e2e/helpers.ts` (reusable utilities)
3333
- **Default Timeout**: 30 seconds (Playwright default, no override)
3434

35-
### Timeout Policy (Standardized 2025-11-09)
36-
All Playwright tests must complete within **30 seconds total**. Individual waits must fit within this budget:
35+
### Timeout Policy (Standardized 2025-11-12)
36+
All Playwright tests must complete within **60 seconds total**. The primary integration test (`should discover and display screenshots`) sets `test.setTimeout(60000)` to give the agent time to explore while still keeping runs deterministic. Individual waits must fit within this budget:
3737

3838
| Operation | Timeout | Notes |
3939
|-----------|---------|-------|
40-
| Test Total | 30s | Playwright default, no `test.setTimeout()` override |
40+
| Test Total | 60s | `test.setTimeout(60000)` for screenshot discovery test |
4141
| Page Navigation | 30s | Initial page load via `page.goto()` |
4242
| Element Visibility | 10s | Headings, buttons, basic UI elements |
4343
| Agent Events | 15s | Screenshot capture, graph events (SSE) |
4444
| Image Rendering | 10s | Screenshots in gallery, artifacts |
4545

4646
**Example Breakdown:**
4747
```typescript
48-
test("example", async ({ page }) => {
49-
// No test.setTimeout() - use 30s default
50-
await page.goto("/"); // ~2s
51-
await page.getByRole("button").click(); // ~1s
52-
await page.waitForURL(/\/run\/.+/); // ~3s
53-
await expect(heading).toBeVisible({ timeout: 10000 }); // max 10s
54-
await page.waitForSelector('[data-event="..."]', { timeout: 15000 }); // max 15s
55-
// Total: ~31s max (fits in 30s with fast agent)
48+
test.describe("/run page smoke tests", () => {
49+
test.setTimeout(60_000); // keep total budget bounded
50+
51+
test("should discover and display screenshots", async ({ page }) => {
52+
await page.goto("/");
53+
await page.getByRole("button", { name: /detect.*drift/i }).click();
54+
await page.waitForURL(/\/run\/[a-f0-9-]+/i, { waitUntil: "domcontentloaded", timeout: 30_000 });
55+
await expect(
56+
page.locator("[data-testid='run-events'] [data-event-kind='agent.event.screenshot_captured']").first(),
57+
).toBeVisible({ timeout: 15_000 });
58+
await expect(page.locator("[data-testid='discovered-screens'] img").first()).toBeVisible({ timeout: 10_000 });
59+
});
5660
});
5761
```
5862

63+
### Svelte 5 Runes Pitfalls (Critical 2025-11-12)
64+
- **`const` vs `let`**: Any rune created with `$state`, `$derived`, `$bindable`, or `$props` that you mutate later MUST use `let`. Using `const` triggers `[plugin:vite-plugin-svelte] Cannot assign/bind to constant` and the entire app shows a 500 overlay, causing Playwright to fail instantly.
65+
-`let { value = $bindable("") } = $props();`
66+
-`let showSlackPreview = $state(false);`
67+
-`const ... = $state(...)`
68+
- **`bind:value` + `$bindable`**: You cannot combine `bind:value` with `$bindable`. Use `{value}` + `oninput={(e) => value = e.currentTarget.value}`.
69+
- **Symptoms in tests**: Playwright screenshot shows `500 Internal Error` overlay with the exact file/line from Vite. If you see this, fix the rune declaration first before debugging Playwright.
70+
- **Skill updates**: See `frontend-development_skill` and `frontend-debugging_skill` for the full playbook.
71+
5972
### Current Tests
6073
1. **Landing Page Load** - Verifies frontend health (no agent required)
6174
2. **Run Page Navigation** - Clicks "Detect My First Drift" → verifies Run Timeline heading (no agent required)
6275
3. **Screenshot Discovery** - Full integration test: start run → wait for screenshots → verify images visible (requires agent + Appium)
6376

77+
> ℹ️ Fast-fail guard: the test polls the `run_events` timeline for `agent.app.launch_failed`. If Appium/device is missing it fails in ~2s with a detailed error (package, attempt, duration, hints). Use this signal before chasing 30s timeouts.
78+
6479
### Pre-Push Hook Integration
6580
E2E tests run automatically before every push:
6681
```bash
@@ -112,6 +127,14 @@ test("my new test", async ({ page }) => {
112127
```
113128
4. Keep a clean workspace: run Playwright scripts from `/tmp` or a disposable directory—never add dependencies to the repo.
114129

130+
5. **Diagnosing stuck runs**
131+
```bash
132+
DEBUG=pw:api HEADLESS=true bun run playwright test --config=playwright.config.temp.ts --max-failures=1
133+
```
134+
- Watch for `page.waitForURL` timeouts → check backend logs for `startRun` failures and confirm Appium/device is online.
135+
- If screenshot gallery waits time out, ensure the backend test `encore test run/start.integration.test.ts` passes (agent must be healthy).
136+
- Use the generated error-context.md to confirm whether the page shows the 500 overlay (rune issues) or a healthy landing page (run start failing).
137+
115138
---
116139

117140
## 2. Playwright Workflow (Recommended)

0 commit comments

Comments
 (0)