test: add WebSocket CRUD E2E test for progressive-enhancement #26
Conversation
Adds TestProgressiveEnhancement_WebSocketCRUD that verifies add and delete operations work correctly with DOM updates. This test validates that the auto-generated keys feature works end-to-end. Test sequence: 1. Navigate to page and count initial todos (3) 2. Add a new todo, verify count increases (4) 3. Delete the added todo, verify count returns to initial (3) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds an end-to-end test (TestProgressiveEnhancement_WebSocketCRUD) to validate WebSocket-based CRUD operations with DOM verification in the progressive-enhancement example. The test ensures that adding and deleting todos via WebSocket correctly updates the DOM without page reloads, verifying the auto-generated range keys feature works correctly.
Changes:
- Add comprehensive E2E test for WebSocket CRUD operations
- Test validates initial todo count, add operation (count increases), and delete operation (count decreases back)
- Includes DOM verification to ensure UI updates match server-side state changes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ctx, cancel = context.WithTimeout(ctx, 15*time.Second) | ||
| defer cancel() |
There was a problem hiding this comment.
The second context.WithTimeout at line 505 reassigns the cancel variable, which means the first cancel function created at line 502 will never be called. This creates a resource leak where the browser context won't be properly cleaned up.
The pattern used in other tests like TestProgressiveEnhancement_JSFormSubmission (lines 100-102) uses a separate variable name (timeoutCancel) for the timeout cancel function to avoid this issue.
| ctx, cancel = context.WithTimeout(ctx, 15*time.Second) | |
| defer cancel() | |
| ctx, timeoutCancel := context.WithTimeout(ctx, 15*time.Second) | |
| defer timeoutCancel() |
| err := chromedp.Run(ctx, | ||
| chromedp.Navigate(server.URL), | ||
| chromedp.WaitReady(`body`, chromedp.ByQuery), | ||
| chromedp.Sleep(1*time.Second), // Wait for WebSocket to connect |
There was a problem hiding this comment.
Using chromedp.Sleep with arbitrary durations (1 second) makes tests slower and potentially flaky. While chromedp.Sleep is used elsewhere in this file, other examples in the codebase (e.g., todos-components/todos_test.go:140 and todos/todos_test.go:177) use more robust patterns like e2etest.WaitFor with conditions to verify DOM changes have occurred.
Consider replacing the sleep with a condition-based wait, such as: e2etest.WaitFor to check when the WebSocket client is ready before performing operations. For DOM updates after form submissions, consider using e2etest.WaitFor with a condition that checks for the expected todo count change.
| // Step 1: Navigate and count initial todos | ||
| err := chromedp.Run(ctx, | ||
| chromedp.Navigate(server.URL), | ||
| chromedp.WaitReady(`body`, chromedp.ByQuery), | ||
| chromedp.Sleep(1*time.Second), // Wait for WebSocket to connect | ||
| chromedp.Evaluate(`document.querySelectorAll('.todo-item').length`, &initialCount), | ||
| ) | ||
| if err != nil { | ||
| t.Fatalf("Step 1 (navigate) error: %v", err) | ||
| } | ||
| t.Logf("Initial todo count: %d", initialCount) |
There was a problem hiding this comment.
The test description says it validates "3 todos" initially, but it doesn't actually verify that initialCount equals 3. If the initial state changes (e.g., setupServer adds more or fewer todos), the test would still pass even though the assumptions documented in the PR description would be wrong.
Consider adding an explicit assertion like:
if initialCount != 3 {
t.Fatalf("Expected 3 initial todos, got %d", initialCount)
}
| } | ||
| } | ||
|
|
||
| // TestProgressiveEnhancement_WebSocketCRUD tests add, toggle, and delete operations via WebSocket |
There was a problem hiding this comment.
The test comment states it tests "add, toggle, and delete operations" but the implementation only tests add and delete operations. There is no toggle operation being tested. Either the comment should be updated to match the actual test implementation, or a toggle test should be added to match the description.
| // TestProgressiveEnhancement_WebSocketCRUD tests add, toggle, and delete operations via WebSocket | |
| // TestProgressiveEnhancement_WebSocketCRUD tests add and delete operations via WebSocket |
Expands TestProgressiveEnhancement_WebSocketCRUD to verify all operations: 1. Add new todo (count: 3 → 4) 2. Toggle to mark complete (verify .completed class added) 3. Toggle again to mark incomplete (verify .completed class removed) 4. Delete the todo (count: 4 → 3) This comprehensive test validates that the auto-generated keys feature correctly handles in-place updates, not just additions and deletions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Verifies that auto-generated keys work correctly after item deletion. The test deletes one item and then toggles a different item to ensure key-based DOM updates work correctly after range modifications. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix context cancel variable shadowing (use timeoutCancel) - Replace chromedp.Sleep with e2etest.WaitFor for robust waiting - Wait for WebSocket ready before operations - Wait for DOM changes using condition-based waits Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Dependencies
This PR depends on the following PRs being merged first:
After those PRs are merged and a new library version is published, this repo's go.mod will need to be updated to use the new version.
Test Details
TestProgressiveEnhancement_WebSocketCRUDvalidates:This test verifies that the auto-generated keys feature works end-to-end, ensuring DOM updates correctly reflect server-side state changes.
Test plan
🤖 Generated with Claude Code