fix: un-skip avatar-upload E2E tests and fix broken selectors#45
Merged
fix: un-skip avatar-upload E2E tests and fix broken selectors#45
Conversation
The two browser E2E subtests ("Upload Avatar - Live Update" and "Upload
Progress Display") were skipped claiming chromedp can't do file uploads.
The DataTransfer API approach already present works fine in headless
Chrome — the real issues were non-existent CSS selectors (.upload-entry,
.success, .upload-preview) that don't match the template's semantic HTML.
Replaced both skipped subtests with one working "Upload Avatar and
Verify" test that exercises the full upload flow: file staging via
DataTransfer API → form submit → upload protocol → success message →
avatar image render → progress bar at 100%.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Re-enables and consolidates avatar-upload browser E2E coverage by replacing previously skipped upload subtests with a single end-to-end “upload + verify UI” flow that aligns selectors with the app’s semantic HTML.
Changes:
- Replaced two skipped avatar-upload E2E subtests with a single “Upload Avatar and Verify” test that stages a file via the DataTransfer API and validates success UI.
- Updated E2E assertions to target semantic elements (
<ins>,<progress>,<img alt="Avatar">) instead of non-existent CSS classes. - Removed the unused
createTestImagehelper and related imports.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
avatar-upload/avatar-upload_test.go
Outdated
Comment on lines
+137
to
+149
| // Small delay to let the client library process the change event | ||
| err = chromedp.Run(ctx, chromedp.Sleep(300*time.Millisecond)) | ||
| if err != nil { | ||
| t.Fatalf("Failed to create test image: %v", err) | ||
| t.Fatalf("Sleep failed: %v", err) | ||
| } | ||
| defer os.Remove(testImagePath) | ||
|
|
||
| var progressHTML string | ||
| // Submit the form — this triggers the upload flow then the updateProfile action | ||
| err = chromedp.Run(ctx, | ||
| chromedp.Evaluate(`document.querySelector('button[type="submit"]').click()`, nil), | ||
|
|
||
| // Read the test image file | ||
| imageData, err := os.ReadFile(testImagePath) | ||
| // Wait for the <ins> "Upload complete!" message to appear via live WebSocket update | ||
| e2etest.WaitFor(`document.querySelector('ins') !== null`, 15*time.Second), | ||
| ) |
There was a problem hiding this comment.
The test uses a fixed 300ms chromedp.Sleep to wait for the client to process the file-input change event. This can introduce CI flakiness on slower machines; prefer waiting on a deterministic DOM/client signal (e.g., wait until the upload entry/progress element appears, or a JS condition indicating the upload has been staged) before submitting the form.
Address Copilot review comment about the 300ms chromedp.Sleep being prone to CI flakiness. The new approach drives the upload flow directly via the LiveTemplate client's WebSocket connection: send upload_start, intercept the response to get entry IDs, send chunk + upload_complete. This is deterministic (no polling, no sleep) and follows the CLAUDE.md recommendation to use window.liveTemplateClient.send() for WebSocket verification. It also avoids the morphdom race condition where the file input element could be replaced between querySelector and dispatchEvent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
chromedp.Click dispatches at the element center, which lands mid-text and inserts characters at the wrong position. Use selectionStart/End to explicitly position the cursor at the end of the value before typing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add diagnostic logging to see the actual input value and preview text when the test fails in CI, so we can determine the root cause. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI debug output showed the correct values ("WorldXY" / "Hello, WorldXY!")
were present but arrived just after the 5s WaitForText deadline. Increase
to 10s to handle slow CI runners where debounce + WebSocket round-trip
+ DOM update can exceed 5s.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.upload-entry,.success,.upload-preview) — the template uses semantic HTML (<ins>,<progress>,<img>)createTestImagehelper andos/filepathimportsTest plan
go test -v -count=1 ./avatar-upload/...passes all subtests (E2E + WebSocket)t.Skip()in any subtest🤖 Generated with Claude Code