Fix .todoIf() on test.skip chain in webview.test.ts#28733
Conversation
Line 51 aliases `it = isMacOS ? test : test.skip`. On non-macOS platforms, `it.todoIf(isCI)` at line 470 becomes `test.skip.todoIf()` which throws "Cannot call .todoIf() on test.skip" at load time. Use the same pattern as line 431: `(isMacOS ? test.todoIf(isCI) : test.skip)` to select the modifier chain upfront instead of chaining on the already-skipped `it` alias.
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
Updated 3:56 PM PT - Mar 31st, 2026
❌ @cirospaciari, your commit 98cb61a has 4 failures in
🧪 To try this PR locally: bunx bun-pr 28733That installs a local version of the PR into your bun-28733 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA test case registration in the webview test file is modified to depend on the platform. The test's conditional logic now applies Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
`click(selector)` / `scrollTo(selector)` poll for actionability by waiting on `requestAnimationFrame` between checks. On macOS CI the headless `WKWebView` gets no `CVDisplayLink` callbacks (no display attached), so rAF never fires and these tests hang to the per-file timeout. Consolidates the eight rAF-dependent tests under an `itRendering` alias (`todoIf(isCI)` on macOS, `skip` elsewhere). Replaces the `it.todoIf(isCI)` at :493 which on non-macOS became `test.skip.todoIf()` and threw at file load — regression of #28733. Also fixes `webview-chrome.test.ts:668`: `await expect(await view.click(...)).rejects.toThrow()` — the stray inner `await` (introduced in 2f6f266) made the rejection throw before `.rejects` could catch it. Only surfaced on alpine CI since that's the only image with Chrome installed. Supersedes #28618 (which covered 3 of the 5 newly-skipped tests). All eight still run locally. - `webview.test.ts` local: 57 pass / 1 todo / 0 fail - `webview.test.ts` `CI=true`: 49 pass / 9 todo / 0 fail - `webview-chrome.test.ts` local: 52 pass / 0 fail --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Line 51 aliases `it = isMacOS ? test : test.skip`. On non-macOS platforms, `it.todoIf(isCI)` at line 470 becomes `test.skip.todoIf()` which throws `Cannot call .todoIf() on test.skip` at file load time — the test file fails to load on Linux/Windows CI. Use the same pattern already established at line 431: `(isMacOS ? test.todoIf(isCI) : test.skip)(...)` — select the modifier chain upfront instead of chaining `.todoIf()` onto the already-skipped `it` alias.
`click(selector)` / `scrollTo(selector)` poll for actionability by waiting on `requestAnimationFrame` between checks. On macOS CI the headless `WKWebView` gets no `CVDisplayLink` callbacks (no display attached), so rAF never fires and these tests hang to the per-file timeout. Consolidates the eight rAF-dependent tests under an `itRendering` alias (`todoIf(isCI)` on macOS, `skip` elsewhere). Replaces the `it.todoIf(isCI)` at :493 which on non-macOS became `test.skip.todoIf()` and threw at file load — regression of oven-sh#28733. Also fixes `webview-chrome.test.ts:668`: `await expect(await view.click(...)).rejects.toThrow()` — the stray inner `await` (introduced in 2f6f266) made the rejection throw before `.rejects` could catch it. Only surfaced on alpine CI since that's the only image with Chrome installed. Supersedes oven-sh#28618 (which covered 3 of the 5 newly-skipped tests). All eight still run locally. - `webview.test.ts` local: 57 pass / 1 todo / 0 fail - `webview.test.ts` `CI=true`: 49 pass / 9 todo / 0 fail - `webview-chrome.test.ts` local: 52 pass / 0 fail --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Line 51 aliases `it = isMacOS ? test : test.skip`. On non-macOS platforms, `it.todoIf(isCI)` at line 470 becomes `test.skip.todoIf()` which throws `Cannot call .todoIf() on test.skip` at file load time — the test file fails to load on Linux/Windows CI. Use the same pattern already established at line 431: `(isMacOS ? test.todoIf(isCI) : test.skip)(...)` — select the modifier chain upfront instead of chaining `.todoIf()` onto the already-skipped `it` alias.
`click(selector)` / `scrollTo(selector)` poll for actionability by waiting on `requestAnimationFrame` between checks. On macOS CI the headless `WKWebView` gets no `CVDisplayLink` callbacks (no display attached), so rAF never fires and these tests hang to the per-file timeout. Consolidates the eight rAF-dependent tests under an `itRendering` alias (`todoIf(isCI)` on macOS, `skip` elsewhere). Replaces the `it.todoIf(isCI)` at :493 which on non-macOS became `test.skip.todoIf()` and threw at file load — regression of oven-sh#28733. Also fixes `webview-chrome.test.ts:668`: `await expect(await view.click(...)).rejects.toThrow()` — the stray inner `await` (introduced in 2f6f266) made the rejection throw before `.rejects` could catch it. Only surfaced on alpine CI since that's the only image with Chrome installed. Supersedes oven-sh#28618 (which covered 3 of the 5 newly-skipped tests). All eight still run locally. - `webview.test.ts` local: 57 pass / 1 todo / 0 fail - `webview.test.ts` `CI=true`: 49 pass / 9 todo / 0 fail - `webview-chrome.test.ts` local: 52 pass / 0 fail --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Line 51 aliases
it = isMacOS ? test : test.skip. On non-macOS platforms,it.todoIf(isCI)at line 470 becomestest.skip.todoIf()which throwsCannot call .todoIf() on test.skipat file load time — the test file fails to load on Linux/Windows CI.Use the same pattern already established at line 431:
(isMacOS ? test.todoIf(isCI) : test.skip)(...)— select the modifier chain upfront instead of chaining.todoIf()onto the already-skippeditalias.