perf(tests): speed up E2E test suite#93
Merged
Merged
Conversation
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/50c7ed30-3b70-4e1d-b2b0-6887115a948a Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
nitrocode
April 26, 2026 21:49
View session
Contributor
📝 WalkthroughWalkthroughI pity the fool who don't parallelize their tests, fool! This PR enables full parallel test execution in Playwright and sets worker concurrency to 4 in CI environments. It also revamps e2e test wait strategies, replacing fixed timeouts with polling mechanisms and adjusting mobile timeout buffers for stability, sucka! Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
👁️ PR Preview
Screenshots
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 242 242
Branches 109 109
=========================================
Hits 242 242
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/e2e/death-clock.spec.js (1)
60-66: Mr. T says: Guard against null before callingtrim()/toLowerCase().Right now, a transient
nulltextContent can throw and make failures noisy. Add a truthy assertion first, same style aswaitForCounter.🛡️ Null-safe assertion tweak
await expect(async () => { const text = await page.locator('#sessionCounter').textContent(); + expect(text).toBeTruthy(); expect(text.trim()).not.toBe(''); expect(text).toMatch(/\d/); }).toPass({ timeout: 3000 });await expect(async () => { const text = await page.locator('#rateEvent').textContent(); + expect(text).toBeTruthy(); expect(text.trim()).not.toBe(''); expect(text.toLowerCase()).toContain('tokens'); }).toPass({ timeout: 3000 });Also applies to: 78-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/death-clock.spec.js` around lines 60 - 66, The test's inline expect callback reads page.locator('#sessionCounter').textContent() and calls trim()/toMatch directly, which can throw if text is null; update the anonymous async assertion in the 'session counter populates after a moment' test (and the similar block around lines 78-82) to first assert the text is truthy (e.g., expect(text).toBeTruthy()) before calling text.trim() or text.toLowerCase(), matching the null-safe pattern used by waitForCounter; ensure you reference the same locator '#sessionCounter' and keep the overall polling/assertion structure intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/death-clock.spec.js`:
- Around line 391-392: Increase the auto-hide assertion timeout to reduce CI
flakiness: update the expect call that checks the bubble element (the await
expect(bubble).not.toHaveClass(/visible/, { timeout: 4200 })) to use a larger
buffer (e.g. timeout: 6000 or 7000) so the UI timer-based assertion has enough
headroom on slow CI workers.
- Around line 85-97: The test named "total counter grows over time" only checks
non-empty text, so either rename the test to reflect current behavior (e.g.,
"total counter stays non-empty over time") or update assertions to confirm
actual growth by using the existing waitForCounter and
page.locator('#totalCounter') values: parse the visible text from
page.locator('#totalCounter').textContent() into a numeric value (accounting for
formatting/units) and assert the second value is greater than the first; locate
the test block named 'total counter grows over time' and modify its title or
replace the non-empty assertions with numeric comparison logic referencing
waitForCounter and page.locator('#totalCounter').
---
Nitpick comments:
In `@tests/e2e/death-clock.spec.js`:
- Around line 60-66: The test's inline expect callback reads
page.locator('#sessionCounter').textContent() and calls trim()/toMatch directly,
which can throw if text is null; update the anonymous async assertion in the
'session counter populates after a moment' test (and the similar block around
lines 78-82) to first assert the text is truthy (e.g.,
expect(text).toBeTruthy()) before calling text.trim() or text.toLowerCase(),
matching the null-safe pattern used by waitForCounter; ensure you reference the
same locator '#sessionCounter' and keep the overall polling/assertion structure
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9c69a1c-4770-4367-96cc-0e0d3a70e038
📒 Files selected for processing (3)
.github/workflows/e2e-tests.ymlplaywright.config.jstests/e2e/death-clock.spec.js
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/c940ec65-4b4c-4e96-ab02-1aa38a66c704 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Owner
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/9658237a-bfeb-4951-ada4-2b39190ebf67 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Contributor
Author
Merge conflicts resolved in bf3f1f9. The only conflict was |
Owner
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Signed-off-by: RB <7775707+nitrocode@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/death-clock.spec.js (1)
68-74: Optional cleanup: de-duplicate the two inline polling blocks.I pity the fool maintainin’ the same polling shape twice—extract one tiny helper with a predicate so these checks stay in one place.
♻️ Small refactor sketch
+async function waitForTextReady(page, selector, assertText, timeout = 3000) { + await expect(async () => { + const text = await page.locator(selector).textContent(); + expect(text).toBeTruthy(); + expect(text.trim()).not.toBe(''); + await assertText(text); + }).toPass({ timeout }); +} + - await expect(async () => { - const text = await page.locator('#sessionCounter').textContent(); - expect(text).toBeTruthy(); - expect(text.trim()).not.toBe(''); - expect(text).toMatch(/\d/); - }).toPass({ timeout: 3000 }); + await waitForTextReady(page, '#sessionCounter', async (text) => { + expect(text).toMatch(/\d/); + }); - await expect(async () => { - const text = await page.locator('#rateEvent').textContent(); - expect(text).toBeTruthy(); - expect(text.trim()).not.toBe(''); - expect(text.toLowerCase()).toContain('tokens'); - }).toPass({ timeout: 3000 }); + await waitForTextReady(page, '#rateEvent', async (text) => { + expect(text.toLowerCase()).toContain('tokens'); + });Also applies to: 86-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/death-clock.spec.js` around lines 68 - 74, Extract the duplicated polling logic into a small helper (e.g., an async waitFor or assertEventually) that accepts an async predicate callback and a timeout and internally runs the existing expect(async () => { ... }).toPass({ timeout }) pattern; then replace both inline blocks that call page.locator('#sessionCounter').textContent() and assert truthiness/trim/match with a single call to that helper, keeping the same assertions inside the predicate so behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/e2e/death-clock.spec.js`:
- Around line 68-74: Extract the duplicated polling logic into a small helper
(e.g., an async waitFor or assertEventually) that accepts an async predicate
callback and a timeout and internally runs the existing expect(async () => { ...
}).toPass({ timeout }) pattern; then replace both inline blocks that call
page.locator('#sessionCounter').textContent() and assert truthiness/trim/match
with a single call to that helper, keeping the same assertions inside the
predicate so behavior is unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 22945a29-100a-4975-a90e-af6c0affbee7
📒 Files selected for processing (1)
tests/e2e/death-clock.spec.js
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.
expect(text).toBeTruthy()null-guard beforetext.trim()/text.toLowerCase()in session counter and rate event pollsSummary by CodeRabbit