Playwright end-to-end testing #896
Conversation
WalkthroughAdds Playwright end-to-end testing: Playwright config and devDependency, CI steps and .gitignore entries, multiple E2E test suites and helpers, a testing guide, and data-testid attributes across Svelte components and route pages to support reliable selectors for tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Playwright test
participant Runner as Playwright runner
participant Browser as Browser
participant App as Web App
participant Storage as localStorage
Runner->>Browser: launch per project/device
Test->>Browser: open app URL
Browser->>App: request page
App-->>Browser: serve UI (with data-testid attrs)
Test->>App: interact via selectors (testid/role/placeholder)
App->>Storage: read/write state (e.g., 'react-todos')
Test->>App: assert UI state, classes, counts, routing
Runner->>Runner: capture trace on first retry
Runner->>CI: produce html report and artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
src/lib/components/TextAppearanceSelector.svelte (1)
172-199: Stable selectors added to theme buttons — looks good; consider namespacing to avoid collisionsThe added data-testid attributes will help Playwright tests remain stable. To reduce the chance of cross-component collisions and improve selector clarity, consider namespacing these IDs (e.g., theme-normal, theme-sepia, theme-dark) or adding a parent container test ID (e.g., theme-options).
Note: Per retrieved learnings from judah-sotomayor, avoiding extra a11y labels on theme buttons is intentional. This change respects that preference.
Optional diff if you want to namespace now:
- data-testid="normal" + data-testid="theme-normal" ... - data-testid="sepia" + data-testid="theme-sepia" ... - data-testid="dark" + data-testid="theme-dark"src/lib/components/Settings.svelte (1)
62-66: Good test selector; guard against undefined keys and sanitize for robustnessUsing data-testid={setting.key + '-setting'} aligns with the test strategy. If setting.key could ever be missing or contain unsafe characters, consider guarding/sanitizing to avoid producing "undefined-setting" or brittle selectors.
Minimal inline hardening:
- data-testid={setting.key + '-setting'} + data-testid={setting.key ? `${setting.key}-setting` : undefined}Or define a small helper in <script> for consistency across components:
const toTestId = (v: string | undefined) => v ? v.toLowerCase().replace(/[^a-z0-9_-]/gi, '-') : undefined;And then:
data-testid={toTestId(setting.key) && `${toTestId(setting.key)}-setting`}src/lib/components/LexiconReversalListView.svelte (1)
6-10: Consider a more specific test ID to avoid ambiguity with vernacular viewUsing a shared data-testid="list-view" across reversal and vernacular lists can become ambiguous if both exist in the DOM (e.g., hidden panes/tabs). Prefer a distinct ID (e.g., reversal-list-view) or add a parent container test ID.
Optional diff:
-<ul - data-testid="list-view" +<ul + data-testid="reversal-list-view"src/lib/components/LexiconVernacularListView.svelte (1)
6-10: Mirror specificity with a distinct test ID to prevent selector collisionsFor parity with the reversal list, consider data-testid="vernacular-list-view" so tests can target views unambiguously.
Optional diff:
-<ul - data-testid="list-view" +<ul + data-testid="vernacular-list-view".gitignore (1)
46-50: Playwright artifacts ignored — good; add a couple of common ones for completenessSolid baseline. Consider also ignoring the nested cache contents and traces that are often generated locally.
Optional additions:
# Playwright /test-results/ /playwright-report/ /blob-report/ /playwright/.cache/ + /playwright/.cache/** + /playwright/.last-run.json + trace.zippackage.json (1)
26-34: Playwright test runner added — consider scripts for DX and CIAdding @playwright/test is good. To streamline local/CI usage, add scripts to install browsers and run tests.
Suggested scripts:
"scripts": { + "test:e2e": "playwright test", + "test:e2e:headed": "playwright test --headed", + "test:e2e:ui": "playwright test --ui", + "e2e:browsers:install": "playwright install --with-deps", + "e2e:report": "playwright show-report", "dev": "concurrently --restart-tries -1 --restart-after 10 -t HH:mm:ss -p \"[{time} {name}]\" -n Converter,Svelte \"ts-node convert/index.ts --watch\" \"vite dev --host\"",If you want, I can add these in a follow-up commit.
docs/test/e2e-testing-basics.md (1)
3-3: Polish: HTTPS links, no bare URLs, and accurate Playwright usage
- Use HTTPS and markdown link syntax to satisfy MD034.
getByTestIdreturns a Locator; don’tawaitit.- Prefer
await page.goto('/')with PlaywrightbaseURLconfigured (orhttp://localhost:5173/), not a hard-coded IP.Suggested edits:
-See the official documentation at http://playwright.dev/docs/intro +See the official documentation at https://playwright.dev/docs/intro-`const myItem = await page.getByTestId("your-test-id");` +`const myItem = page.getByTestId("your-test-id");````js test('Test Navigation', async ({ page }) => { - await page.goto('http://100.76.234.109:5173/'); + await page.goto('/'); await page.getByTestId("about-icon").click(); await expect(page.getByTestId("title")).toHaveText("About"); });If you enable `use.baseURL` in Playwright config (see separate comment), the `/` path will work as shown. Also applies to: 7-9, 20-27, 31-31, 35-35 </blockquote></details> <details> <summary>playwright.config.ts (1)</summary><blockquote> `27-33`: **Enable baseURL and webServer to make tests self-contained** Starting the app automatically and using a base URL removes the need to run the dev server manually and allows using `await page.goto('/')` in tests. Example: ```diff use: { - /* Base URL to use in actions like `await page.goto('/')`. */ - // baseURL: 'http://127.0.0.1:3000', + /* Base URL to use in actions like `await page.goto('/')`. */ + baseURL: 'http://localhost:5173', /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ trace: 'on-first-retry', }, /* Run your local dev server before starting the tests */ -// webServer: { -// command: 'npm run start', -// url: 'http://127.0.0.1:3000', -// reuseExistingServer: !process.env.CI, -// }, +// For local dev, you can keep manual startup; for CI, enable: +// webServer: { +// command: 'npm run dev', +// url: 'http://localhost:5173', +// reuseExistingServer: !process.env.CI, +// },If you prefer manual startup locally, at least set
baseURLand use relativepage.goto('/')in tests for consistency.Also applies to: 73-79
src/lib/scripts/playwright/general.spec.ts (7)
5-5: Use baseURL and navigate withpage.goto('/')With
use.baseURLset in Playwright config, switch to root-relative navigation. This avoids hard-coded ports.- await page.goto('http://localhost:5173/'); + await page.goto('/');Repeat for other occurrences in this file.
Also applies to: 36-36, 66-66, 82-82, 149-149
7-7: Prefer strict equalityUse
===for program type checks.- if (cfg.default.programType == "DAB") { + if (cfg.default.programType === 'DAB') {Apply to all similar comparisons, including
"SAB".Also applies to: 17-17, 31-31, 54-54, 68-68, 83-83, 116-116, 148-148
4-4: Verify that$libalias resolves in Playwright Node contextDynamic imports like
await import('$lib/data/config')rely on path alias resolution, which Playwright doesn't handle by default. Either:
- import via a relative path (e.g.,
import config from '../../data/config'), or- configure tsconfig paths at runtime (e.g., with
tsconfig-paths/register) and ensure Playwright picks it up.I can generate a small patch to either convert to relative imports or wire up
tsconfig-pathsif you confirm your tsconfig paths.Also applies to: 29-29, 67-67, 81-81, 147-147
8-11: Remove unnecessaryawaiton Locators
page.getByTestId(...)returns a Locator immediately; noawaitrequired.- let title = await page.getByTestId("title-text") + const title = page.getByTestId('title-text'); ... - menuButton = await page.getByTestId('title'); + menuButton = page.getByTestId('title'); ... - menuButton = await page.getByTestId('hamburger-icon'); + menuButton = page.getByTestId('hamburger-icon');Also applies to: 32-35
41-45: Theme assertions may be brittle across browsersHard-coded
background-colorRGB values can differ due to platform/UA differences. Prefer asserting a theme class/data-attribute toggle or a CSS variable value on a root element.Example approach:
- Assert body has a theme class (e.g.,
class~="dark"), or- Read
getComputedStyle(root).getPropertyValue('--BackgroundColor')viapage.evaluateand compare to expected theme variable.Also applies to: 49-53, 57-61
24-26: Test hygiene: remove commented-out codeClean out commented logs and assertions to keep the test suite focused.
- /*await page.getByTestId("settings-icon").click(); - await expect(page.getByTestId("title")).toHaveText("Settings");*/ ... - //console.log(cfg.default.interfaceLanguages.writingSystems.yao.displayNames.en: Ciyawo); ... - //await expect(page.locator('[class="mt2"]')).toHaveText("The Good News According to");Also applies to: 89-114, 140-142
176-181: Guard alphabet length before index-based comparisonAdd an assertion that the button count equals the configured alphabet length to avoid out-of-bounds mismatches.
- const buttons = await alphabet.locator('button'); - for (let i = 0; i < await buttons.count(); i++) { + const buttons = alphabet.locator('button'); + await expect(buttons).toHaveCount(vAlphabet.length); + for (let i = 0; i < vAlphabet.length; i++) { const buttonText = await buttons.nth(i).textContent(); expect(vAlphabet[i]).toEqual(buttonText.trim()); }Apply similarly for the reversal alphabet loop.
Also applies to: 192-195
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.gitignore(1 hunks)docs/test/e2e-testing-basics.md(1 hunks)docs/test/e2e-testing-demo/demo-todo-app.spec.ts(1 hunks)package.json(1 hunks)playwright.config.ts(1 hunks)src/lib/components/LexiconLanguageTabs.svelte(4 hunks)src/lib/components/LexiconReversalListView.svelte(1 hunks)src/lib/components/LexiconVernacularListView.svelte(1 hunks)src/lib/components/Navbar.svelte(1 hunks)src/lib/components/Settings.svelte(1 hunks)src/lib/components/Sidebar.svelte(10 hunks)src/lib/components/TextAppearanceSelector.svelte(3 hunks)src/lib/scripts/playwright/example.spec.ts(1 hunks)src/lib/scripts/playwright/general.spec.ts(1 hunks)src/routes/about/+page.svelte(1 hunks)src/routes/lexicon/+page.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T18:47:15.122Z
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#840
File: src/lib/components/TextAppearanceSelector.svelte:165-197
Timestamp: 2025-06-16T18:47:15.122Z
Learning: In src/lib/components/TextAppearanceSelector.svelte, the user judah-sotomayor considers theme selection buttons not relevant for screen-reader users and prefers not to add accessibility labels to theme buttons.
Applied to files:
src/lib/components/TextAppearanceSelector.svelte
🧬 Code Graph Analysis (1)
src/lib/scripts/playwright/general.spec.ts (1)
src/routes/contents/[id]/+page.js (1)
title(17-17)
🪛 markdownlint-cli2 (0.17.2)
docs/test/e2e-testing-basics.md
3-3: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (8)
src/lib/components/Navbar.svelte (2)
46-51: LGTM on hamburger icon selectordata-testid="hamburger-icon" provides a stable hook without affecting behavior.
56-60: Bug: Svelte event binding should useon:click, notonclickUsing onclick={handleBackNavigation} will not wire up the Svelte event listener correctly and likely breaks the back button. Use on:click.
Fix:
- <button - onclick={handleBackNavigation} + <button + on:click={handleBackNavigation} class="dy-btn dy-btn-ghost dy-btn-circle" data-testid="back-icon" >⛔ Skipped due to learnings
Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:68-70 Timestamp: 2025-06-16T18:48:21.767Z Learning: In Svelte 5, the event handler syntax has changed from Svelte 4. The `onclick` attribute is valid in Svelte 5, unlike Svelte 4 where `on:click` was required.Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:68-70 Timestamp: 2025-06-16T18:48:21.767Z Learning: In Svelte 5, event handler syntax changed from Svelte 4. Use `onclick={handler}` instead of `on:click={handler}`. Event modifiers like `|preventDefault` are no longer supported and must be handled inside the handler function. Only one handler per event is allowed per element.Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:94-95 Timestamp: 2025-06-16T18:49:02.188Z Learning: The appbuilder-pwa project uses Svelte 5, as evidenced by "svelte": "^5" in package.json and the use of Svelte 5 runes syntax like $props() and $derived(). In Svelte 5, onclick is the preferred event handler syntax over on:click.src/routes/about/+page.svelte (1)
18-20: LGTM on about page title selectordata-testid="about-title" is a clear, stable target for E2E tests.
src/lib/components/LexiconLanguageTabs.svelte (1)
15-15: Test IDs added appropriately for stable selectorsThese additions look good and align with the Playwright selectors used in the suite. No behavior change introduced.
Also applies to: 24-24, 33-33, 42-42
src/lib/components/Sidebar.svelte (2)
138-139: Test IDs are consistent and cover key navigation actionsGood coverage for E2E selectors across sidebar actions (account, contents, search, layouts, history, bookmarks, notes, highlights, share, plans, settings, text-appearance, about).
Also applies to: 150-151, 162-163, 175-176, 190-191, 203-203, 214-215, 226-227, 241-242, 254-255, 266-267, 279-279, 318-319
137-139: Use Svelte event listeners (on:click) instead ofonclickIn Svelte, event handlers should use the
on:directive. Usingonclick={...}won’t attach a listener as intended and can break interactions. Apply this change to all affected buttons in the modified blocks.Example diff for a few blocks (apply similarly elsewhere):
- onclick={() => gotoRoute('/account')} + on:click={() => gotoRoute('/account')} ... - onclick={goToSearch} + on:click={goToSearch} ... - onclick={() => modal.open(MODAL_COLLECTION)} + on:click={() => modal.open(MODAL_COLLECTION)} ... - onclick={() => gotoRoute('/about')} + on:click={() => gotoRoute('/about')}Also applies to: 149-151, 158-163, 175-176, 189-191, 202-203, 210-215, 226-227, 237-242, 250-255, 266-267, 278-279, 314-319
⛔ Skipped due to learnings
Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:68-70 Timestamp: 2025-06-16T18:48:21.767Z Learning: In Svelte 5, event handler syntax changed from Svelte 4. Use `onclick={handler}` instead of `on:click={handler}`. Event modifiers like `|preventDefault` are no longer supported and must be handled inside the handler function. Only one handler per event is allowed per element.Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:68-70 Timestamp: 2025-06-16T18:48:21.767Z Learning: In Svelte 5, the event handler syntax has changed from Svelte 4. The `onclick` attribute is valid in Svelte 5, unlike Svelte 4 where `on:click` was required.Learnt from: judah-sotomayor PR: sillsdev/appbuilder-pwa#840 File: src/lib/components/NoteDialog.svelte:94-95 Timestamp: 2025-06-16T18:49:02.188Z Learning: The appbuilder-pwa project uses Svelte 5, as evidenced by "svelte": "^5" in package.json and the use of Svelte 5 runes syntax like $props() and $derived(). In Svelte 5, onclick is the preferred event handler syntax over on:click.docs/test/e2e-testing-demo/demo-todo-app.spec.ts (1)
1-438: Demo spec looks good and is isolated from the configured testDirNo action needed since
testDirexcludesdocs/. Keeping this as a reference is fine.src/routes/lexicon/+page.svelte (1)
252-256: Test IDs on title controls look goodThese additions match the selectors used by the Playwright suite. No functional impact.
| test('has title', async ({ page }) => { | ||
| await page.goto('https://playwright.dev/'); | ||
|
|
||
| // Expect a title "to contain" a substring. | ||
| await expect(page).toHaveTitle(/Playwright/); | ||
| }); |
There was a problem hiding this comment.
Avoid external network dependency in CI; mark these as skipped or move to docs
These tests hit https://playwright.dev/ and will be flaky/offline in CI. Either skip them or relocate under docs so they’re not discovered by testDir.
Apply:
-test('has title', async ({ page }) => {
+test.skip('has title', async ({ page }) => {
...
-test('get started link', async ({ page }) => {
+test.skip('get started link', async ({ page }) => {Alternatively, move this file out of src/lib/scripts/playwright so it’s not picked up by the configured testDir.
Also applies to: 10-18
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/example.spec.ts around lines 3 to 8, the test
performs an external HTTP request to https://playwright.dev which makes CI
flaky; either mark the test to be skipped in CI (e.g., use test.skip or a
describe.skip / conditional check so it won’t run in automated test runs) or
move the file out of the configured testDir (for example into a docs or examples
folder) so it isn’t discovered by Playwright’s test runner; apply the same
change to the other affected files at lines 10-18.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/lib/scripts/playwright/example.spec.ts (1)
3-8: Skip external-site examples or move them out of testDir to avoid CI flakinessThese tests hit https://playwright.dev which makes CI flaky/offline. Either skip them or relocate to docs/examples so they aren’t discovered by the configured testDir.
Apply:
-test('has title', async ({ page }) => { +test.skip('has title', async ({ page }) => { await page.goto('https://playwright.dev/'); // Expect a title "to contain" a substring. await expect(page).toHaveTitle(/Playwright/); }); -test('get started link', async ({ page }) => { +test.skip('get started link', async ({ page }) => { await page.goto('https://playwright.dev/'); // Click the get started link. await page.getByRole('link', { name: 'Get started' }).click(); // Expects page to have a heading with the name of Installation. await expect(page.getByRole('heading', { name: 'Installation' })).toBeVisible(); });Alternative: Move this file out of src/lib/scripts/playwright (e.g., docs/test/examples) so Playwright won’t pick it up via testDir.
Also applies to: 10-18
🧹 Nitpick comments (3)
src/lib/scripts/playwright/general.spec.ts (3)
7-13: Use strict equality and simplify boolean checksPrefer strict equality and omit redundant “== true”.
Apply:
- if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { ... - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { ... - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { ... - if (thm.name == 'Normal' && thm.enabled == true) { + if (thm.name === 'Normal' && thm.enabled) { ... - if (thm.name == 'Sepia' && thm.enabled == true) { + if (thm.name === 'Sepia' && thm.enabled) { ... - if (thm.name == 'Dark' && thm.enabled == true) { + if (thm.name === 'Dark' && thm.enabled) { ... - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { ... - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') {Also applies to: 17-23, 31-35, 38-41, 49-55, 60-66, 77-81, 92-96
8-11: Avoid awaiting Locators; remove unnecessary awaitgetByTestId/locator return Locator synchronously. Awaiting them is unnecessary and confusing.
Apply:
- let title = await page.getByTestId('title-text'); + const title = page.getByTestId('title-text'); ... - menuButton = await page.getByTestId('title'); + menuButton = page.getByTestId('title'); ... - const title = await page.locator('.dy-navbar-center'); + const title = page.locator('.dy-navbar-center'); ... - const selbox = await page.getByTestId('interface-language-setting'); + const selbox = page.getByTestId('interface-language-setting'); ... - await expect(await page.getByTestId('vernacular-language-tab')).toBeVisible(); - await expect(await page.getByTestId('reversal-language-tab')).toBeVisible(); + await expect(page.getByTestId('vernacular-language-tab')).toBeVisible(); + await expect(page.getByTestId('reversal-language-tab')).toBeVisible();Optional: add explicit types for clarity:
import type { Locator } from '@playwright/test'; let menuButton: Locator;Also applies to: 31-35, 84-86, 105-107, 188-190
41-47: Minor readability: rename clbton and DRY repeated stepsUse clearer variable names and deduplicate repeated “open menu → open text appearance” steps.
Apply:
- const clbton = page.getByTestId('normal'); - await expect(clbton).toBeVisible(); - await clbton.click(); + const themeButton = page.getByTestId('normal'); + await expect(themeButton).toBeVisible(); + await themeButton.click();Consider extracting:
const openTextAppearance = async () => { await menuButton.click(); await page.getByTestId('text-appearance-icon').click(); };Also applies to: 52-59, 64-70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
docs/test/e2e-testing-demo/demo-todo-app.spec.ts(1 hunks)package.json(1 hunks)playwright.config.ts(1 hunks)src/lib/scripts/playwright/demo-todo-app.spec.ts(1 hunks)src/lib/scripts/playwright/example.spec.ts(1 hunks)src/lib/scripts/playwright/general.spec.ts(1 hunks)src/routes/about/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/routes/about/+page.svelte
- docs/test/e2e-testing-demo/demo-todo-app.spec.ts
- package.json
- playwright.config.ts
🧰 Additional context used
🪛 GitHub Actions: Lint
src/lib/scripts/playwright/demo-todo-app.spec.ts
[warning] 1-1: Prettier formatting issues found in src/lib/scripts/playwright/demo-todo-app.spec.ts. Run 'prettier --write' to fix.
src/lib/scripts/playwright/general.spec.ts
[warning] 1-1: Prettier formatting issues found in src/lib/scripts/playwright/general.spec.ts. Run 'prettier --write' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
| test.beforeEach(async ({ page }) => { | ||
| await page.goto('https://demo.playwright.dev/todomvc'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid external dependency in CI: skip or relocate the TodoMVC demo tests
This suite hits https://demo.playwright.dev/todomvc. It will be flaky/offline in CI. Either skip by default or move to docs/examples and exclude from testDir.
Options:
- Wrap file content in a skipped describe:
import { test, expect, type Page } from '@playwright/test';
+test.describe.skip('TodoMVC demo (skipped in CI; docs example)', () => {
test.beforeEach(async ({ page }) => {
await page.goto('https://demo.playwright.dev/todomvc');
});
...
+});- Or ignore this file in playwright.config.ts:
testIgnore: ['**/src/lib/scripts/playwright/demo-todo-app.spec.ts']- Or move it to docs/test/e2e-testing-demo/ and keep it out of testDir.
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/demo-todo-app.spec.ts around lines 3 to 5, the
test suite depends on the external URL https://demo.playwright.dev/todomvc which
causes CI flakiness; fix by either (A) wrapping the file contents in a skipped
suite (e.g., describe.skip or test.skip) so it does not run in CI by default,
(B) excluding this file from the test runner by adding its path to
playwright.config.ts testIgnore (e.g., add
'**/src/lib/scripts/playwright/demo-todo-app.spec.ts'), or (C) move the file out
of the testDir into docs/examples or docs/test/e2e-testing-demo/ so it isn’t
executed by CI; pick one option and apply it consistently across the
repo/config.
| const todoCount = page.getByTestId('todo-count') | ||
|
|
||
| // Check test using different methods. | ||
| await expect(page.getByText('3 items left')).toBeVisible(); | ||
| await expect(todoCount).toHaveText('3 items left'); | ||
| await expect(todoCount).toContainText('3'); | ||
| await expect(todoCount).toHaveText(/3/); | ||
|
|
||
| // Check all items in one call. |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix Prettier/lint warnings (missing semicolons) and run prettier
Pipeline flagged formatting issues. Two concrete fixes:
Apply:
- const todoCount = page.getByTestId('todo-count')
+ const todoCount = page.getByTestId('todo-count');and
- const todoCount = page.getByTestId('todo-count')
+ const todoCount = page.getByTestId('todo-count');Then run:
Also applies to: 263-266
🏁 Script executed:
#!/bin/bash
npx prettier --write src/lib/scripts/playwright/demo-todo-app.spec.tsLength of output: 253
I can’t share my internal chain-of-thought, but I will run focused checks and give a concise, evidence-backed rewritten review comment and next steps.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/scripts/playwright/demo-todo-app.spec.ts"
if [ ! -f "$file" ]; then
echo "FILE_NOT_FOUND: $file"
exit 0
fi
echo "=== HEAD (lines 1-140) ==="
nl -ba "$file" | sed -n '1,140p'
echo "=== TAIL (lines 240-280) ==="
nl -ba "$file" | sed -n '240,280p'Length of output: 371
Sorry — I can’t share my internal chain-of-thought. I can, however, run targeted checks and provide a concise verification summary plus a rewritten review comment.
I'll print the relevant lines, locate occurrences of "todo-count", and check whether those lines end with semicolons.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/scripts/playwright/demo-todo-app.spec.ts"
if [ ! -f "$file" ]; then
echo "FILE_NOT_FOUND: $file"
exit 0
fi
echo "=== LINES 50-75 ==="
awk 'NR>=50 && NR<=75{printf("%4d: %s\n", NR, $0)}' "$file"
echo
echo "=== LINES 260-270 ==="
awk 'NR>=260 && NR<=270{printf("%4d: %s\n", NR, $0)}' "$file"
echo
echo "=== LOCATIONS OF 'todo-count' (line numbers) ==="
grep -n "todo-count" "$file" || true
echo
echo "=== SEMICOLON CHECK FOR LINES CONTAINING 'todo-count' ==="
grep -n "todo-count" "$file" | cut -d: -f1 | while read -r ln; do
line=$(sed -n "${ln}p" "$file")
# trim leading spaces for printing
trimmed=$(echo "$line" | sed -e 's/^[[:space:]]*//')
if echo "$line" | grep -Eq ';\s*$'; then
ends="true"
else
ends="false"
fi
printf "%s: %s -> endsWithSemicolon=%s\n" "$ln" "$trimmed" "$ends"
doneLength of output: 3721
Fix Prettier/lint warnings: add missing semicolons in demo-todo-app.spec.ts
Quick summary: Pipeline flagged formatting issues. I verified two occurrences of a missing semicolon for the todoCount locator in src/lib/scripts/playwright/demo-todo-app.spec.ts (lines 58 and 265). Attempting to run Prettier in this environment failed with "Cannot find package 'prettier-plugin-svelte'". Apply the edits below and then run your formatter in an environment with the repo dev-dependencies installed (or install the missing plugin) to ensure no other files are affected.
Files / locations to update
- src/lib/scripts/playwright/demo-todo-app.spec.ts
- Line 58: const todoCount = page.getByTestId('todo-count') (missing semicolon)
- Line 265: const todoCount = page.getByTestId('todo-count') (missing semicolon)
Suggested patch (apply both places):
- const todoCount = page.getByTestId('todo-count')
+ const todoCount = page.getByTestId('todo-count');Commands to re-run formatting locally / in CI
- If you have project dev-deps installed:
- npx prettier --write src/lib/scripts/playwright/demo-todo-app.spec.ts
- If Prettier errors with "Cannot find package 'prettier-plugin-svelte'":
- Install the missing plugin and Prettier as devDependencies:
- npm install --save-dev prettier prettier-plugin-svelte
- Then re-run:
- npx prettier --write src/lib/scripts/playwright/demo-todo-app.spec.ts
- Install the missing plugin and Prettier as devDependencies:
- Alternatively run your repository's formatter script after installing dependencies, e.g.:
- npm ci && npm run format
Reasoning (concise): The two lines identified do not end with semicolons and the pipeline formatter/linter flagged them. Adding the semicolons fixes the syntax/formatting warnings; re-running Prettier may require the missing plugin to be installed in the environment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const todoCount = page.getByTestId('todo-count') | |
| // Check test using different methods. | |
| await expect(page.getByText('3 items left')).toBeVisible(); | |
| await expect(todoCount).toHaveText('3 items left'); | |
| await expect(todoCount).toContainText('3'); | |
| await expect(todoCount).toHaveText(/3/); | |
| // Check all items in one call. | |
| const todoCount = page.getByTestId('todo-count'); | |
| // Check test using different methods. | |
| await expect(page.getByText('3 items left')).toBeVisible(); | |
| await expect(todoCount).toHaveText('3 items left'); | |
| await expect(todoCount).toContainText('3'); | |
| await expect(todoCount).toHaveText(/3/); | |
| // Check all items in one call. |
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/demo-todo-app.spec.ts around lines 58 and 265, the
two declarations "const todoCount = page.getByTestId('todo-count')" are missing
trailing semicolons; add a semicolon to the end of both lines to satisfy
Prettier/lint rules, then re-run the repository formatter (e.g., npx prettier
--write src/lib/scripts/playwright/demo-todo-app.spec.ts or npm run format)
after installing dev-deps including prettier-plugin-svelte if required.
| @@ -0,0 +1,219 @@ | |||
| import { expect, test } from '@playwright/test'; | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Fix Prettier warnings
The pipeline reported Prettier issues for this file. Please run:
🏁 Script executed:
#!/bin/bash
npx prettier --write src/lib/scripts/playwright/general.spec.tsLength of output: 159
Install the missing Prettier plugin (prettier-plugin-svelte) and re-run formatting
Short explanation: Running Prettier failed with "Cannot find package 'prettier-plugin-svelte'". This indicates the plugin referenced by your Prettier config is not resolvable in the current environment. Install the plugin (or ensure project deps are installed) and then re-run the formatter.
Files to re-format
- src/lib/scripts/playwright/general.spec.ts
Suggested fix (from project root)
- If you use npm:
npm install --save-dev prettier-plugin-svelte npx prettier --write src/lib/scripts/playwright/general.spec.ts - If you use pnpm:
pnpm add -D prettier-plugin-svelte npx prettier --write src/lib/scripts/playwright/general.spec.ts - If you use yarn:
yarn add -D prettier-plugin-svelte npx prettier --write src/lib/scripts/playwright/general.spec.ts
Notes:
- If the plugin is already in package.json, run your package install step (npm ci / pnpm install / yarn install) before running Prettier so node_modules contains the plugin.
- Ensure CI pipelines install dependencies before running Prettier-based checks.
🧰 Tools
🪛 GitHub Actions: Lint
[warning] 1-1: Prettier formatting issues found in src/lib/scripts/playwright/general.spec.ts. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/general.spec.ts at line 1, Prettier failed because
the configured plugin prettier-plugin-svelte is missing; install the plugin (or
run your package install if it’s already listed in package.json) using your
project package manager as a dev dependency, then re-run Prettier to format
src/lib/scripts/playwright/general.spec.ts so the formatter can resolve the
plugin and apply formatting.
| const cfg = await import('$lib/data/config'); | ||
| await page.goto('http://localhost:5173/'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
$lib alias import will likely fail in Playwright tests; switch to a relative import
Playwright runs in Node and won’t resolve SvelteKit’s $lib alias by default. Prefer a top-level relative import and reuse it in tests.
Apply:
-import { expect, test } from '@playwright/test';
+import { expect, test } from '@playwright/test';
+import cfg from '../../data/config';Then remove per-test dynamic imports:
- const cfg = await import('$lib/data/config');
+ // using top-level cfg importRepeat the removal at lines 29, 76, 90, and 165.
Also consider typing the imported config if available to improve IntelliSense.
Also applies to: 29-29, 76-76, 90-90, 165-165
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/general.spec.ts around lines 4 to 6 (and also
addressing lines 29, 76, 90, 165), replace the top-of-file dynamic import using
the SvelteKit $lib alias with a top-level relative import to the actual config
file path so Node/Playwright can resolve it; remove the per-test dynamic imports
at lines 29, 76, 90, and 165 and reuse the single top-level imported config
variable in all tests, and if a config type exists add that type to the import
to improve IntelliSense.
💡 Verification agent
🧩 Analysis chain
Ensure local web server is started and use baseURL to simplify page.goto
These tests navigate to http://localhost:5173. Confirm playwright.config.ts defines webServer and baseURL, otherwise tests will fail in CI.
- Suggested config:
// playwright.config.ts
import { defineConfig, devices } from '@playwright/test';
export default defineConfig({
testDir: './src/lib/scripts/playwright',
use: {
baseURL: 'http://localhost:5173',
trace: 'on-first-retry',
},
webServer: {
command: 'npm run dev',
url: 'http://localhost:5173',
reuseExistingServer: !process.env.CI,
timeout: 120_000,
},
projects: [
{ name: 'chromium', use: { ...devices['Desktop Chrome'] } },
{ name: 'firefox', use: { ...devices['Desktop Firefox'] } },
{ name: 'webkit', use: { ...devices['Desktop Safari'] } },
],
});- Then replace page.goto('http://localhost:5173/') with:
- await page.goto('http://localhost:5173/');
+ await page.goto('/');Run to verify presence of settings:
Also applies to: 36-36, 75-87, 91-101, 163-170
🏁 Script executed:
#!/bin/bash
rg -n "webServer|baseURL" -- playwright.config.* || trueLength of output: 122
Enable Playwright webServer + set baseURL and switch tests to use relative paths
Short justification: playwright.config.ts currently has both baseURL and webServer commented out (rg shows lines 29 and 74), while tests use absolute URLs like http://localhost:5173/. Without the webServer/baseURL enabled Playwright won't start your app in CI and tests that hit that URL will fail.
Files/locations to update:
- playwright.config.ts — uncomment/add a webServer block and a use.baseURL entry (rg output shows commented lines at 29 and 74).
- src/lib/scripts/playwright/general.spec.ts — replace absolute navigations with relative ones. Applies to the ranges called out previously: lines 4-6 and also 36-36, 75-87, 91-101, 163-170.
Recommended test change (example diff):
- await page.goto('http://localhost:5173/');
+ await page.goto('/');Recommended playwright.config.ts snippet (adjust command/port if your project uses a different dev command or port):
// playwright.config.ts
import { defineConfig, devices } from '@playwright/test';
export default defineConfig({
testDir: './src/lib/scripts/playwright',
use: {
baseURL: 'http://localhost:5173',
trace: 'on-first-retry',
},
webServer: {
command: 'npm run dev', // adjust if needed
url: 'http://localhost:5173',
reuseExistingServer: !process.env.CI,
timeout: 120_000,
},
projects: [
{ name: 'chromium', use: { ...devices['Desktop Chrome'] } },
{ name: 'firefox', use: { ...devices['Desktop Firefox'] } },
{ name: 'webkit', use: { ...devices['Desktop Safari'] } },
],
});Verification command you already ran:
- rg -n "webServer|baseURL" -- playwright.config.* (output shows both are currently commented)
Next steps for the author:
- Confirm the correct dev command and port (5173 vs 3000) and update the snippet accordingly.
- Uncomment/add the config above in playwright.config.ts.
- Replace absolute page.goto URLs in the test file(s) with relative paths (page.goto('/')).
- Run Playwright locally and in CI to ensure the server starts and tests pass.
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/general.spec.ts around lines 4-6 (and also update
occurrences at lines 36, 75-87, 91-101, 163-170), tests are using absolute URLs
like "http://localhost:5173/" which will fail unless Playwright's webServer and
use.baseURL are enabled; change page.goto calls to use relative paths (e.g.,
page.goto('/' or '/some-path')) so they resolve against baseURL, and then update
playwright.config.ts to add/uncomment a webServer block that starts your dev
server (command and port must match your project) and set use.baseURL to the
same URL (http://localhost:5173 or your actual port), ensuring
reuseExistingServer and a reasonable timeout for CI; confirm the correct dev
command/port, make these config edits, replace all absolute navigations in the
listed ranges with relative paths, and run Playwright locally/CI to verify the
server is started and tests pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/lib/scripts/playwright/example.e2e.ts (1)
3-18: Consider removing or customizing example tests from external sitesThese tests target Playwright's documentation site, which may not be relevant to your project's testing needs. External site tests can fail due to changes outside your control and don't validate your application's functionality.
Consider either:
- Removing this file if it's just a placeholder example
- Updating these tests to target your actual application endpoints
- Moving external site examples to a documentation folder if they're meant as learning resources
-test('has title', async ({ page }) => { - await page.goto('https://playwright.dev/'); - - // Expect a title "to contain" a substring. - await expect(page).toHaveTitle(/Playwright/); -}); - -test('get started link', async ({ page }) => { - await page.goto('https://playwright.dev/'); - - // Click the get started link. - await page.getByRole('link', { name: 'Get started' }).click(); - - // Expects page to have a heading with the name of Installation. - await expect(page.getByRole('heading', { name: 'Installation' })).toBeVisible(); -}); +// Replace with tests for your application +test('has title', async ({ page }) => { + await page.goto('http://localhost:5173/'); + // Add assertions for your app's title +});playwright.config.ts (2)
31-31: Consider enabling baseURL for cleaner test codeThe baseURL is commented out, but it would simplify your tests by allowing relative URLs instead of full URLs.
- // baseURL: 'http://127.0.0.1:3000', + baseURL: process.env.CI ? 'http://127.0.0.1:5173' : 'http://localhost:5173',Then update your tests to use relative paths:
await page.goto('/'); // Instead of 'http://localhost:5173/'
76-80: Consider enabling webServer configuration for automated test setupThe webServer configuration is commented out but would be valuable for automatically starting your dev server before tests run, especially in CI environments.
- /* Run your local dev server before starting the tests */ - // webServer: { - // command: 'npm run start', - // url: 'http://127.0.0.1:3000', - // reuseExistingServer: !process.env.CI, - // }, + /* Run your local dev server before starting the tests */ + webServer: { + command: 'npm run dev', + url: 'http://127.0.0.1:5173', + reuseExistingServer: !process.env.CI, + timeout: 120 * 1000, + },src/lib/scripts/playwright/general.e2e.ts (3)
28-72: Refactor repetitive theme testing codeThe theme testing logic contains significant duplication. Each theme check follows the same pattern but with different values.
Consider refactoring to reduce duplication:
test('Test Text Appearance', async ({ page }) => { const cfg = await import('$lib/data/config'); const menuButton = cfg.default.programType == 'DAB' ? page.getByTestId('title') : page.getByTestId('hamburger-icon'); await page.goto('/'); const themeTests = [ { name: 'Normal', testId: 'normal', bgColor: 'rgb(240, 240, 240)' }, { name: 'Sepia', testId: 'sepia', bgColor: 'rgb(233, 216, 186)' }, { name: 'Dark', testId: 'dark', bgColor: 'rgb(0, 0, 0)' } ]; for (const theme of themeTests) { const configTheme = cfg.default.themes.find(t => t.name === theme.name); if (configTheme?.enabled) { await menuButton.click(); await page.getByTestId('text-appearance-icon').click(); const button = page.getByTestId(theme.testId); await expect(button).toBeVisible(); await button.click(); await expect(page.locator('.dy-drawer-content')).toHaveCSS( 'background-color', theme.bgColor ); } } });
109-118: Inefficient nested loops for language testingThe nested loop structure is inefficient and difficult to follow. It iterates through all writing systems twice for each language selection.
Consider simplifying the language testing logic:
for (const [code, writingSystem] of Object.entries(cfg.default.writingSystems)) { await selbox.selectOption(code); // Verify the selected language appears correctly const displayName = writingSystem.displayNames[code] || writingSystem.displayNames['default']; await expect(selbox).toContainText(displayName); await expect(selbox).toBeVisible(); }
172-182: Improve code clarity with better variable namingThe variables
vernCodeandrevCodeare initialized with confusing placeholder values'wrong-v'and'wrong-r'. Also, variable names likesysandsysNamecould be more descriptive.- let vernCode = 'wrong-v'; - let revCode = 'wrong-r'; - for (let sys in cfg.default.writingSystems) { - let sysName = cfg.default.writingSystems[sys].displayNames['default']; + let vernCode = null; + let revCode = null; + for (const [systemCode, writingSystem] of Object.entries(cfg.default.writingSystems)) { + const systemName = writingSystem.displayNames['default']; - if (sysName.trim() === vernName.trim()) { - vernCode = sys; + if (systemName.trim() === vernName.trim()) { + vernCode = systemCode; } - if (sysName.trim() === revName.trim()) { - revCode = sys; + if (systemName.trim() === revName.trim()) { + revCode = systemCode; } } + + // Ensure codes were found + expect(vernCode).not.toBeNull(); + expect(revCode).not.toBeNull();src/lib/scripts/playwright/demo-todo-app.e2e.ts (1)
3-5: External demo site dependency may cause test instabilityThis test suite targets an external demo site (https://demo.playwright.dev/todomvc) which could change or become unavailable, causing test failures unrelated to your application.
Consider:
- If this is meant as a learning example, move it to a documentation folder
- If you need TodoMVC tests, consider hosting a local copy
- Add a skip condition for CI to avoid external dependencies:
+test.describe.skip(!!process.env.CI, 'Skip external demo tests in CI'); test.beforeEach(async ({ page }) => { await page.goto('https://demo.playwright.dev/todomvc'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
playwright.config.ts(1 hunks)src/lib/scripts/playwright/demo-todo-app.e2e.ts(1 hunks)src/lib/scripts/playwright/example.e2e.ts(1 hunks)src/lib/scripts/playwright/general.e2e.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/scripts/playwright/general.e2e.ts (1)
src/routes/contents/[id]/+page.js (1)
title(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (2)
playwright.config.ts (1)
15-17: LGTM! Good choice of test directory and naming patternThe configuration correctly sets up the test directory and uses a distinct
.e2e.tspattern to avoid conflicts with Vitest unit tests.src/lib/scripts/playwright/demo-todo-app.e2e.ts (1)
421-437: LGTM! Well-structured helper functionsThe helper functions for localStorage validation are well-implemented and provide good abstraction for test assertions.
| test('Test Navigation', async ({ page }) => { | ||
| const cfg = await import('$lib/data/config'); | ||
| await page.goto('http://localhost:5173/'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use baseURL configuration instead of hardcoded URLs
The tests use hardcoded http://localhost:5173/ URLs. This should be configured in playwright.config.ts as baseURL for better maintainability and environment flexibility.
After enabling baseURL in the config file, update all occurrences:
- await page.goto('http://localhost:5173/');
+ await page.goto('/');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('Test Navigation', async ({ page }) => { | |
| const cfg = await import('$lib/data/config'); | |
| await page.goto('http://localhost:5173/'); | |
| test('Test Navigation', async ({ page }) => { | |
| const cfg = await import('$lib/data/config'); | |
| await page.goto('/'); |
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/general.e2e.ts around lines 3 to 6, the test is
using a hardcoded URL "http://localhost:5173/"; update your Playwright config
(playwright.config.ts) to set baseURL to the appropriate environment value (e.g.
http://localhost:5173) and then replace the hardcoded page.goto call with a
relative path like await page.goto('/') so Playwright resolves it via baseURL;
also search and replace other occurrences of the hardcoded URL across tests to
use relative paths after setting baseURL.
| if (cfg.default.programType == 'DAB') { | ||
| await page.getByTestId('back-icon').click(); | ||
| await page.getByTestId('hamburger-icon').click(); | ||
| } else { | ||
| await page.getByTestId('back-icon').click(); | ||
| await page.getByTestId('hamburger-icon').click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicated code in conditional branches
Lines 18-22 are identical to the else branch, making the conditional check redundant.
if (cfg.default.programType == 'DAB') {
await page.getByTestId('back-icon').click();
await page.getByTestId('hamburger-icon').click();
} else {
await page.getByTestId('back-icon').click();
await page.getByTestId('hamburger-icon').click();
}Since both branches are identical, simplify to:
- if (cfg.default.programType == 'DAB') {
- await page.getByTestId('back-icon').click();
- await page.getByTestId('hamburger-icon').click();
- } else {
- await page.getByTestId('back-icon').click();
- await page.getByTestId('hamburger-icon').click();
- }
+ await page.getByTestId('back-icon').click();
+ await page.getByTestId('hamburger-icon').click();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (cfg.default.programType == 'DAB') { | |
| await page.getByTestId('back-icon').click(); | |
| await page.getByTestId('hamburger-icon').click(); | |
| } else { | |
| await page.getByTestId('back-icon').click(); | |
| await page.getByTestId('hamburger-icon').click(); | |
| } | |
| await page.getByTestId('back-icon').click(); | |
| await page.getByTestId('hamburger-icon').click(); |
🤖 Prompt for AI Agents
In src/lib/scripts/playwright/general.e2e.ts around lines 17 to 23, the if/else
branches contain identical code (clicking 'back-icon' and 'hamburger-icon'), so
remove the conditional and call those page.getByTestId(...).click() actions once
in sequence; replace the entire if/else block with the two click calls executed
unconditionally and retain existing awaits and ordering.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/lib/scripts/playwright/general.e2e.ts (2)
5-5: Use Playwright baseURL instead of hardcoded localhostReplace hardcoded URLs with relative paths and configure baseURL in playwright.config.ts. This improves portability across environments.
Apply this diff:
- await page.goto('http://localhost:5173/'); + await page.goto('/');Run this script to find any other hardcoded occurrences:
#!/bin/bash rg -n -C2 -S "http://localhost:5173"Also applies to: 36-36, 75-75, 91-91, 164-164
17-23: Remove redundant if/else (identical branches)Both branches perform the same actions; the condition can be dropped.
- if (cfg.default.programType == 'DAB') { - await page.getByTestId('back-icon').click(); - await page.getByTestId('hamburger-icon').click(); - } else { - await page.getByTestId('back-icon').click(); - await page.getByTestId('hamburger-icon').click(); - } + await page.getByTestId('back-icon').click(); + await page.getByTestId('hamburger-icon').click();
🧹 Nitpick comments (10)
src/lib/scripts/playwright/general.e2e.ts (10)
28-36: Initialize page before creating locators and drop unnecessary awaitsCreate locators after navigation (clearer) and avoid awaiting locator creation (locators are lazy, not promises).
test('Test Text Appearance', async ({ page }) => { const cfg = await import('$lib/data/config'); - let menuButton; - if (cfg.default.programType == 'DAB') { - menuButton = await page.getByTestId('title'); - } else { - menuButton = await page.getByTestId('hamburger-icon'); - } - await page.goto('http://localhost:5173/'); + await page.goto('/'); + const menuButton = + cfg.default.programType === 'DAB' + ? page.getByTestId('title') + : page.getByTestId('hamburger-icon');
8-9: Remove unnecessary awaits on Locator creationAwaiting locators is unnecessary and can confuse readers. Expect/Locator APIs handle waiting.
- let title = await page.getByTestId('title-text'); + const title = page.getByTestId('title-text'); @@ - const title = await page.locator('.dy-navbar-center'); + const title = page.locator('.dy-navbar-center'); @@ - await expect(await page.getByTestId('vernacular-language-tab')).toBeVisible(); - await expect(await page.getByTestId('reversal-language-tab')).toBeVisible(); + await expect(page.getByTestId('vernacular-language-tab')).toBeVisible(); + await expect(page.getByTestId('reversal-language-tab')).toBeVisible(); @@ - const alphabet = await page.getByTestId('alphabet-strip'); + const alphabet = page.getByTestId('alphabet-strip'); @@ - const lv1 = await page.getByTestId('list-view'); + const lv1 = page.getByTestId('list-view'); @@ - const alphabet2 = await page.getByTestId('alphabet-strip'); + const alphabet2 = page.getByTestId('alphabet-strip'); @@ - const lv2 = await page.getByTestId('list-view'); + const lv2 = page.getByTestId('list-view');Also applies to: 84-85, 186-188, 191-191, 198-198, 205-205, 212-212
38-48: Prefer strict equality and simplify boolean checks in theme loopsUse === for comparisons and leverage truthiness for booleans.
- if (thm.name == 'Normal' && thm.enabled == true) { + if (thm.name === 'Normal' && thm.enabled) { @@ - if (thm.name == 'Sepia' && thm.enabled == true) { + if (thm.name === 'Sepia' && thm.enabled) { @@ - if (thm.name == 'Dark' && thm.enabled == true) { + if (thm.name === 'Dark' && thm.enabled) {Also applies to: 49-59, 60-71
7-13: Use strict equality for programType checksMinor consistency/style update; prevents accidental coercion.
- if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { @@ - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { @@ - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') { @@ - if (cfg.default.programType == 'SAB') { + if (cfg.default.programType === 'SAB') { @@ - if (cfg.default.programType == 'DAB') { + if (cfg.default.programType === 'DAB') {Also applies to: 77-81, 92-96, 125-156, 163-167
82-87: Skip the test at runtime when Share is not availableThe current conditional makes the test pass without assertions. Prefer test.skip for clarity and reporting.
- if (await page.getByTestId('share-icon').isVisible()) { - await page.getByTestId('share-icon').click(); - const title = await page.locator('.dy-navbar-center'); - await expect(title).toHaveText('Share App'); - } + const shareIcon = page.getByTestId('share-icon'); + const hasShare = await shareIcon.isVisible(); + test.skip(!hasShare, 'Share icon not available for this programType'); + await shareIcon.click(); + const title = page.locator('.dy-navbar-center'); + await expect(title).toHaveText('Share App');
97-101: Skip Settings test when Settings is not availableUse test.skip to make the condition explicit and visible in reports.
- if (await page.getByTestId('settings-icon').isVisible()) { - //console.log(cfg.default.interfaceLanguages.writingSystems.yao.displayNames.en: Ciyawo); - - await page.getByTestId('settings-icon').click(); + const settingsIcon = page.getByTestId('settings-icon'); + const settingsVisible = await settingsIcon.isVisible(); + test.skip(!settingsVisible, 'Settings icon not available for this programType'); + await settingsIcon.click();
109-118: Strengthen assertions after selectOptionAfter selecting, assert the selected value matches, in addition to checking visible text.
for (let sys in cfg.default.writingSystems) { await selbox.selectOption(sys); + await expect(selbox).toHaveValue(sys); for (let sys2 in cfg.default.writingSystems) { if (cfg.default.writingSystems[sys2].displayNames[sys] != undefined) { langString = cfg.default.writingSystems[sys2].displayNames[sys]; } else { langString = cfg.default.writingSystems[sys2].displayNames['default']; } await expect(selbox).toContainText(langString); }
41-47: Reduce brittleness of theme assertions (avoid exact background-color checks)Exact RGB values are fragile across platforms/themes. Prefer asserting a stable UI state signal (e.g., selected state on the theme button or a data-theme/class on the root).
Example approach (if available in the app):
- Assert aria-pressed="true" on the selected theme button, or
- Assert document.documentElement has a data-theme attribute/class for the selected theme.
If you want, I can propose a concrete change once I see how the theme is applied in TextAppearanceSelector.svelte.
Also applies to: 55-59, 63-69
4-4: Import config default once per test to avoid cfg.default noiseMinor readability improvement.
- const cfg = await import('$lib/data/config'); + const { default: cfg } = await import('$lib/data/config');Then use cfg.programType, cfg.themes, etc.
Also applies to: 29-29, 77-77, 90-90, 162-162
128-156: Stabilize Settings selectors with test IDs (future-proofing)Current locators rely on option text filtering, which can change with localization or copy edits. Prefer data-testid on each specific select (e.g., verse-layout-setting, images-setting, videos-setting) and select by testid.
Happy to add these test IDs and update the selectors if you want them included in this PR.
Also applies to: 137-145, 146-154
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
package.json(1 hunks)src/lib/scripts/playwright/demo-todo-app.e2e.ts(1 hunks)src/lib/scripts/playwright/general.e2e.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/lib/scripts/playwright/demo-todo-app.e2e.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: lint
| const vernName = await page.getByTestId('vernacular-language-tab').textContent(); | ||
| const revName = await page.getByTestId('reversal-language-tab').textContent(); | ||
|
|
||
| // switch for code in config | ||
| let vernCode = 'wrong-v'; | ||
| let revCode = 'wrong-r'; | ||
| for (let sys in cfg.default.writingSystems) { | ||
| let sysName = cfg.default.writingSystems[sys].displayNames['default']; | ||
| if (sysName.trim() === vernName.trim()) { | ||
| vernCode = sys; | ||
| } | ||
| if (sysName.trim() === revName.trim()) { | ||
| revCode = sys; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix potential runtime errors from null textContent and strengthen assertions
textContent() can return null; calling .trim() would throw. Also, precompute button counts and compare against config alphabets to catch mismatches earlier.
- const vernName = await page.getByTestId('vernacular-language-tab').textContent();
- const revName = await page.getByTestId('reversal-language-tab').textContent();
+ const vernName = (await page.getByTestId('vernacular-language-tab').innerText()).trim();
+ const revName = (await page.getByTestId('reversal-language-tab').innerText()).trim();
+ expect(vernName).not.toEqual('');
+ expect(revName).not.toEqual('');
@@
- const alphabet = await page.getByTestId('alphabet-strip');
- const buttons = await alphabet.locator('button');
- for (let i = 0; i < (await buttons.count()); i++) {
- const buttonText = await buttons.nth(i).textContent();
- expect(vAlphabet[i]).toEqual(buttonText.trim());
- }
+ const alphabet = page.getByTestId('alphabet-strip');
+ const buttons = alphabet.locator('button');
+ const vCount = await buttons.count();
+ expect(vCount).toBe(vAlphabet.length);
+ for (let i = 0; i < vCount; i++) {
+ const buttonText = (await buttons.nth(i).innerText()).trim();
+ expect(vAlphabet[i]).toEqual(buttonText);
+ }
@@
- const alphabet2 = await page.getByTestId('alphabet-strip');
- const buttons2 = await alphabet2.locator('button');
- for (let i = 0; i < (await buttons2.count()); i++) {
- const buttonText = await buttons2.nth(i).textContent();
- expect(rAlphabet[i]).toEqual(buttonText.trim());
- }
+ const alphabet2 = page.getByTestId('alphabet-strip');
+ const buttons2 = alphabet2.locator('button');
+ const rCount = await buttons2.count();
+ expect(rCount).toBe(rAlphabet.length);
+ for (let i = 0; i < rCount; i++) {
+ const buttonText = (await buttons2.nth(i).innerText()).trim();
+ expect(rAlphabet[i]).toEqual(buttonText);
+ }Optional: assert that language codes were resolved before indexing alphabets:
+ expect(vernCode).not.toEqual('wrong-v');
+ expect(revCode).not.toEqual('wrong-r');Also applies to: 191-197, 205-211
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
.github/workflows/test.yml (3)
83-85: Prefer the official Playwright GitHub Action (with caching) over a raw install commandThe microsoft/playwright-github-action installs browsers, required OS deps, and leverages built-in caching; it’s faster and more reliable on GH-hosted runners than a plain npx install.
Apply this diff to replace the raw install with the official action:
- - name: Install Playwright Browsers - run: npx playwright install --with-deps + - name: Install Playwright Browsers + uses: microsoft/playwright-github-action@v1 + with: + with-deps: true
86-88: Ensure the app is running before E2E, and surface logs in CI outputIf your playwright.config.ts does not start a webServer, these tests will hit a non-running app and fail. Also, adding the line reporter helps see failing tests directly in the CI logs.
Apply this diff to improve CI visibility:
- - name: Run Playwright E2E Tests - run: npx playwright test --reporter=html + - name: Run Playwright E2E Tests + run: npx playwright test --reporter=line,htmlIf you are not using
webServerinplaywright.config.ts, add a server step before running tests (example below). If you are usingwebServer, ignore this:# Place this step just before "Run Playwright E2E Tests" - name: Start app (preview) run: | npm run build npm run preview & npx -y wait-on http://127.0.0.1:4173Alternatively, configure
webServerinplaywright.config.tsto auto-start the app:// playwright.config.ts (excerpt) import { defineConfig } from '@playwright/test'; export default defineConfig({ use: { baseURL: 'http://127.0.0.1:4173' }, webServer: { command: 'npm run preview', url: 'http://127.0.0.1:4173', reuseExistingServer: !process.env.CI, timeout: 120_000 } });
89-95: Prevent artifact upload failures when report is missing; consider attaching traces tooIf tests abort early,
playwright-reportmay not exist and artifact upload will fail the job. Also consider uploadingtest-resultsorplaywright-reporttraces for deeper debugging.Apply this diff to make artifact upload resilient:
- name: Upload Playwright Report if: always() # ensures report uploads even if tests fail uses: actions/upload-artifact@v4 with: name: playwright-report path: playwright-report retention-days: 7 + if-no-files-found: ignoreOptional: add a second artifact for raw results/traces (especially useful if you enable
trace: 'on-first-retry'):- name: Upload Playwright Results (raw) if: always() uses: actions/upload-artifact@v4 with: name: playwright-test-results path: playwright-test-results if-no-files-found: ignore retention-days: 7docs/test/e2e-testing-basics.md (3)
3-3: Fix bare URL (markdownlint MD034)
Wrap the URL in a markdown link to satisfy docs linting and improve readability.-See the official documentation at http://playwright.dev/docs/intro +See the official documentation at <https://playwright.dev/docs/intro>
13-16: Clarify file naming patterns to match Playwright defaults and/or your config
Playwright defaults include .spec.[jt]s and .test.[jt]s. If your config also uses .e2e.ts (as suggested elsewhere), call that out for consistency.-- Make sure your test file ends with `.spec.ts` (or .js for JavaScript). - If you're testing `foo.ts`, you'd probably name the test file `foo.spec.ts`. +- Name your test files using Playwright conventions: `.spec.ts`/`.spec.js` or `.test.ts`/`.test.js`. + If your repository also matches `.e2e.ts` in `playwright.config.ts`, document that pattern here. + Example: testing `foo.ts` → `foo.spec.ts`.
33-35: Recommend using webServer in config so tests auto-start the app
If you configurewebServerin playwright.config.ts, developers don’t need to manually start the app in another terminal, reducing flakiness in local runs and CI.-Run `npx playwright test` in a terminal window to run all the tests in the project. Currently, Playwright does not start the local dev app before testing, so that must be done in another thread first (such as `npm run dev`). +Run `npx playwright test` in a terminal window to run all tests. Prefer configuring `webServer` in `playwright.config.ts` so Playwright starts your app automatically. For example: + +```ts +// playwright.config.ts (excerpt) +import { defineConfig } from '@playwright/test'; +export default defineConfig({ + use: { baseURL: 'http://127.0.0.1:4173' }, + webServer: { + command: 'npm run preview', + url: 'http://127.0.0.1:4173', + reuseExistingServer: !process.env.CI, + timeout: 120_000 + } +}); +``` +If you choose not to use `webServer`, start the app in another terminal first (e.g., `npm run dev`) before running tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/test.yml(1 hunks)docs/test/e2e-testing-basics.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/test/e2e-testing-basics.md
3-3: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: test
| Although you can locate many items based on text in their content, classes, or css, it is often preferable to give them test ids to refer to. This is done by adding `data-testid="your-test-id"` in their Svelte HTML tag. Then use: | ||
|
|
||
| `const myItem = await page.getByTestId("your-test-id");` | ||
|
|
||
| You can install the Playwright plugin for VSCode and run individual tests using the "play" symbol appearing next to each test block (note the app itself must already be running locally). |
There was a problem hiding this comment.
Don’t await getByTestId; it returns a Locator, not a Promise
Using await here is incorrect and may confuse readers. Also, capitalize CSS.
-Although you can locate many items based on text in their content, classes, or css, it is often preferable to give them test ids to refer to. This is done by adding `data-testid="your-test-id"` in their Svelte HTML tag. Then use:
+Although you can locate many items based on text in their content, classes, or CSS, it is often preferable to give them test ids to refer to. This is done by adding `data-testid="your-test-id"` in their Svelte HTML tag. Then use:
-`const myItem = await page.getByTestId("your-test-id");`
+`const myItem = page.getByTestId("your-test-id");`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Although you can locate many items based on text in their content, classes, or css, it is often preferable to give them test ids to refer to. This is done by adding `data-testid="your-test-id"` in their Svelte HTML tag. Then use: | |
| `const myItem = await page.getByTestId("your-test-id");` | |
| You can install the Playwright plugin for VSCode and run individual tests using the "play" symbol appearing next to each test block (note the app itself must already be running locally). | |
| Although you can locate many items based on text in their content, classes, or CSS, it is often preferable to give them test ids to refer to. This is done by adding `data-testid="your-test-id"` in their Svelte HTML tag. Then use: | |
| `const myItem = page.getByTestId("your-test-id");` |
🤖 Prompt for AI Agents
In docs/test/e2e-testing-basics.md around lines 7 to 11, the example incorrectly
awaits page.getByTestId (getByTestId returns a Locator, not a Promise) and also
mentions "css" in lowercase; remove the await so the call is used synchronously
(e.g. const myItem = page.getByTestId("your-test-id");) and capitalize "CSS"
where referenced. Ensure the surrounding text and example reflect that
getByTestId returns a Locator and should not be awaited.
| ```js | ||
| test('Test Navigation', async ({ page }) => { | ||
| await page.goto('http://localhost:5173/'); | ||
|
|
||
| await page.getByTestId("about-icon").click(); | ||
| await expect(page.getByTestId("title")).toHaveText("About"); | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Include the correct import line in the sample and remove the unnecessary await
Docs should show the canonical import from @playwright/test and a minimal, working example.
```js
+import { test, expect } from '@playwright/test';
test('Test Navigation', async ({ page }) => {
await page.goto('http://localhost:5173/');
- await page.getByTestId("about-icon").click();
+ await page.getByTestId("about-icon").click();
await expect(page.getByTestId("title")).toHaveText("About");
});
<details>
<summary>🤖 Prompt for AI Agents</summary>
In docs/test/e2e-testing-basics.md around lines 20 to 27, the sample is missing
the canonical Playwright import and uses an unnecessary await before
page.getByTestId(...).click(); add the import line "import { test, expect } from
'@playwright/test';" at the top of the snippet and remove the extra await before
the .click() call so the snippet is minimal and correct.
</details>
<!-- fingerprinting:phantom:triton:chinchilla -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| Gotchas: | ||
|
|
||
| - Make sure you import `test`, and `expect` from playwright, not from node.test or vitest. |
There was a problem hiding this comment.
Correct import guidance: import from @playwright/test (not “playwright”)
The proper package is @playwright/test; importing from “playwright” is incorrect for the test API.
-- Make sure you import `test`, and `expect` from playwright, not from node.test or vitest.
+- Make sure you import `test` and `expect` from `@playwright/test`, not from Node’s test module or Vitest.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Make sure you import `test`, and `expect` from playwright, not from node.test or vitest. | |
| - Make sure you import `test` and `expect` from `@playwright/test`, not from Node’s test module or Vitest. |
🤖 Prompt for AI Agents
In docs/test/e2e-testing-basics.md around line 31, the guidance currently
suggests importing test and expect from "playwright" (or is ambiguous); update
the text to explicitly instruct developers to import test and expect from
"@playwright/test" (e.g., "import { test, expect } from '@playwright/test'") and
remove or correct any mention of importing from "playwright", "node.test", or
"vitest" so the doc clearly points to the @playwright/test package as the
correct test API source.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
playwright.config.ts (5)
5-5: Broaden test discovery to include nested folders.The current pattern only matches files at the root of
testDir. Use a recursive glob so future subfolders are picked up.- testMatch: '*.e2e.ts', + testMatch: '**/*.e2e.ts',
4-4: Consider moving E2E tests out of src/ to avoid accidental inclusion in app builds.Keeping tests under
src/can cause TypeScript, bundlers, or IDE tooling to treat them as app code. A conventional path liketests/e2eore2eavoids this.- testDir: './src/lib/scripts/playwright', + testDir: './tests/e2e',If you take this, remember to update any docs and CI paths accordingly.
12-14: Set a baseURL and capture failure artifacts for easier debugging.With a baseURL, tests can
page.goto('/'). Screenshots/videos on failure speed up triage without being too heavy.- use: { - trace: 'on-first-retry' - }, + use: { + baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://127.0.0.1:5173', + trace: 'on-first-retry', + screenshot: 'only-on-failure', + video: 'retain-on-failure' + },
10-10: Prefer GitHub reporter in CI and prevent auto-opening the HTML report locally.This yields inline annotations in GitHub Actions while still producing the HTML artifact.
- reporter: 'html', + reporter: process.env.CI + ? [['github'], ['html', { open: 'never' }]] + : ['list', ['html', { open: 'never' }]],
24-27: Harden the dev server command and use preview in CI for prod parity.
- Force the host/port and strictPort to avoid a port change that breaks the
urlprobe.- Use
previewin CI for a production-like server; keepdevlocally for faster iteration.- command: 'npm run dev', - url: 'http://127.0.0.1:5173', - reuseExistingServer: !process.env.CI, - timeout: 60_000 // wait up to 60s for server to be ready + command: process.env.CI + ? 'npm run build && npm run preview -- --host 127.0.0.1 --port 5173' + : 'npm run dev -- --host 127.0.0.1 --port 5173 --strictPort', + url: 'http://127.0.0.1:5173', + reuseExistingServer: !process.env.CI, + timeout: 60_000 // wait up to 60s for server to be ready
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
playwright.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: lint
🔇 Additional comments (1)
playwright.config.ts (1)
6-9: CI-friendly toggles look good.
fullyParallel,forbidOnly, retries, and CI-only single worker are sensible defaults for stable, reproducible runs.
| projects: [ | ||
| { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, | ||
| { name: 'firefox', use: { ...devices['Desktop Firefox'] } }, | ||
| { name: 'webkit', use: { ...devices['Desktop Safari'] } } | ||
| ], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Playwright browsers are installed in CI.
Multiple browser projects require npx playwright install --with-deps (or caching) in the workflow before tests run.
Run this script to confirm your workflow installs browsers:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching CI workflows for Playwright install steps..."
rg -n -C3 -g ".github/workflows/**" -P '(playwright\s+install|with-deps|setup-node.*cache.*playwright)' || {
echo "No explicit Playwright install step found. Ensure CI runs: npx playwright install --with-deps"
}Length of output: 590
Add Playwright browser installation step to CI workflows
Our search did not find any npx playwright install --with-deps (or equivalent) in your CI pipeline. Without this, the Chromium, Firefox, and WebKit browsers used by Playwright won’t be available and tests will fail.
Please update your workflow (e.g., .github/workflows/ci.yml) to include before running tests:
- name: Install Playwright browsers
run: npx playwright install --with-deps• Insert this step early in the job that invokes npx playwright test or your test runner.
• Consider caching ~/.cache/ms-playwright (or your OS-appropriate cache directory) to speed up subsequent runs.
🤖 Prompt for AI Agents
In playwright.config.ts around lines 16 to 20, the CI is missing the Playwright
browser installation step which will cause tests to fail because
Chromium/Firefox/WebKit binaries are not present; update your CI workflow (e.g.,
.github/workflows/ci.yml) to run "npx playwright install --with-deps" before any
"npx playwright test" or test-run step, place it early in the job, and
optionally add caching for the Playwright browser cache directory
(~/.cache/ms-playwright or OS equivalent) to speed up subsequent runs.
2 participants
Summary by CodeRabbit
New Features
Documentation
Tests
Chores