Conversation
- Added 'tooling/*' to the pnpm workspace configuration to include tooling packages. - Moved the GitHub App Token generation step in the release workflow for better organization and clarity. - Removed duplicate token generation step to streamline the workflow. - Introduced a new package.json file for the 'playwright-www' tooling package.
- Added new workspace entries for 'docs' and 'assets' under 'apps/www/content' and 'apps/www/public' respectively. - Included 'tooling' and 'playwright-www' under the 'tooling' directory to improve project organization and accessibility. - Removed duplicate entries for 'docs' and 'assets' to streamline the workspace structure.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds Playwright E2E tooling and tests for the www app, expands workspace and Turbo tasks, adds a PR-aware CI job for E2E runs, moves GitHub App token generation earlier in release workflow and removes its duplicate, and normalizes pnpm cache quoting in workflows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer / CI
participant GH as GitHub Actions
participant PNPM as pnpm
participant Turbo as Turbo
participant PW as Playwright
participant WWW as apps/www
rect rgba(200,220,255,0.15)
note over GH: test-e2e job decision (PR vs push)
GH->>GH: Detect event (pull_request or push)
GH->>GH: If PR: fetch base SHA, set TURBO_BASE/TURBO_HEAD
end
GH->>PNPM: Setup Node + pnpm (cache "pnpm")
GH->>PNPM: pnpm install (workspace)
GH->>PW: Cache/install Playwright browsers
alt PR
GH->>Turbo: turbo run test:e2e --affected
else Push
GH->>Turbo: turbo run test:e2e
end
rect rgba(220,255,220,0.15)
note over PW,WWW: Playwright runs and manages web server
Turbo->>PW: Run @repo/playwright-www tests
PW->>WWW: Start or reuse dev server
PW->>PW: Execute tests across browsers
end
GH-->>GH: Upload Playwright HTML report (if produced)
sequenceDiagram
autonumber
participant Rel as Release Workflow
participant App as actions/create-github-app-token@v2
participant CO as actions/checkout
note over Rel: release.yml ordering change
Rel->>App: Generate GitHub App Token (app-id, private-key)
Rel->>CO: Checkout using generated App token
Rel-->>Rel: Duplicate token generation removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
- Introduced a new package '@repo/playwright-www' for managing Playwright tests. - Added scripts for running end-to-end tests in various modes (UI, headed, debug). - Configured Playwright with a dedicated configuration file and test structure. - Updated pnpm and turbo configurations to support new testing tasks. - Included a .gitignore for Playwright-specific files and directories. - Enhanced documentation with setup instructions and test execution guidelines.
- Standardized cache syntax for pnpm in multiple steps. - Added a new job for end-to-end testing using Playwright, including installation of dependencies and browsers. - Configured the workflow to run Playwright tests and upload the test report as an artifact.
…uration - Introduced new scripts for end-to-end testing in package.json, including report generation and updates. - Enhanced turbo.json with new tasks for e2e reporting and UI testing. - Added a new script to update Playwright and its dependencies in tooling/playwright-www/bin/update-playwright.sh.
…ns workflow - Changed the command for installing Playwright browsers from `npx` to `pnpm exec` to ensure consistency with the package manager used in the workflow.
- Added caching for Playwright browsers to improve workflow efficiency. - Updated the command for installing Playwright browsers to use `pnpm dlx`. - Modified the test command to include a filter for better test selection. - Adjusted the path for uploading Playwright reports to include all report directories.
- Added fetch-depth configuration to checkout step for full history access. - Introduced environment variables for Turbo base and head commits during pull requests. - Modified E2E test command to run affected tests for pull requests and all tests for main branch pushes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
tooling/playwright-www/tests/hello-world.spec.ts (1)
19-19: Consider using Playwright's configured output directory for screenshots.The hardcoded relative path
"tests/screenshots/hello-world.png"works but bypasses Playwright's output configuration. For better maintainability and CI integration, consider using the test output directory pattern or storing screenshots in the standard Playwright output location.For example, you could use:
await page.screenshot({ path: `test-results/${test.info().titlePath.join('-')}.png` });Or configure
outputDirinplaywright.config.tsand reference it via test info.tooling/playwright-www/bin/update-playwright.sh (1)
1-10: Harden the script with strict modeAdd bash strict mode so failures stop the updater early; keeps CI/dev flows predictable.
#!/bin/bash + +set -euo pipefail + # Update Playwright and download new browser binaries and their dependencies: pnpm install --save-dev @playwright/test@latest pnpm exec playwright install --with-depsAlso ensure this file is executable (git +x).
tooling/playwright-www/package.json (1)
5-12: Minor: consider adding engines and aligning version rangesOptional polish:
- Add engines to mirror CI Node (22) for clearer constraints.
- Align @types/node to a caret range like others, unless you intentionally want a hard pin.
{ "name": "@repo/playwright-www", "version": "0.0.0", "private": true, + "engines": { "node": ">=20 <23" }, "scripts": { "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", "test:e2e:headed": "playwright test --headed", "test:e2e:debug": "playwright test --debug", "test:e2e:report": "playwright show-report", "test:e2e:update": "bin/update-playwright.sh" }, "dependencies": { "www": "workspace:*" }, "devDependencies": { "@playwright/test": "^1.56.0", - "@types/node": "24.7.2", + "@types/node": "^24.7.2", "typescript": "^5.9.3" } } ```<!-- review_comment_end --> Also applies to: 16-20 </blockquote></details> <details> <summary>turbo.json (1)</summary><blockquote> `46-61`: **LGTM: Playwright task graph integration** Good separation of e2e, report, and UI tasks with pass-through env. Consider adding "CI" to passThroughEnv if you rely on Playwright’s CI heuristics, but not required. <!-- review_comment_end --> </blockquote></details> <details> <summary>.github/workflows/tests.yml (2)</summary><blockquote> `108-115`: **Remove unused variable (ShellCheck SC2034)** BASE_REF is assigned but never used. Drop it to quiet lint and reduce noise. ```diff - - name: Fetch base commit for PRs + - name: Fetch base commit for PRs if: ${{ github.event_name == 'pull_request' }} shell: bash run: | BASE_SHA='${{ github.event.pull_request.base.sha }}' - BASE_REF='${{ github.event.pull_request.base.ref }}' BASE_REPO='${{ github.event.pull_request.base.repo.full_name }}' THIS_REPO='${{ github.repository }}'
125-130: Quote $GITHUB_ENV (ShellCheck SC2086)Quote $GITHUB_ENV to prevent word splitting and globbing.
- echo "TURBO_SCM_BASE=${{ github.event.pull_request.base.sha }}" >> $GITHUB_ENV - echo "TURBO_SCM_HEAD=${{ github.event.pull_request.head.sha }}" >> $GITHUB_ENV + echo "TURBO_SCM_BASE=${{ github.event.pull_request.base.sha }}" >> "$GITHUB_ENV" + echo "TURBO_SCM_HEAD=${{ github.event.pull_request.head.sha }}" >> "$GITHUB_ENV"You can re-run actionlint locally to confirm the warnings are gone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.github/workflows/release.yml(2 hunks).github/workflows/tests.yml(4 hunks)arkenv.code-workspace(2 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)tooling/playwright-www/.gitignore(1 hunks)tooling/playwright-www/.vscode/extensions.json(1 hunks)tooling/playwright-www/README.md(1 hunks)tooling/playwright-www/bin/update-playwright.sh(1 hunks)tooling/playwright-www/package.json(1 hunks)tooling/playwright-www/playwright.config.ts(1 hunks)tooling/playwright-www/tests/hello-world.spec.ts(1 hunks)tooling/playwright-www/tsconfig.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Document environment requirements in README files
Files:
tooling/playwright-www/README.md
🪛 actionlint (1.7.8)
.github/workflows/tests.yml
111-111: shellcheck reported issue in this script: SC2034:warning:2:1: BASE_REF appears unused. Verify use (or export if used externally)
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:2:68: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 LanguageTool
tooling/playwright-www/README.md
[grammar] ~48-~48: There might be a mistake here.
Context: ...figuration The tests are configured to: - Start the www app on `http://localhost:3...
(QB_NEW_EN)
🔇 Additional comments (6)
pnpm-workspace.yaml (1)
5-5: LGTM!The workspace expansion correctly includes the new tooling directory, supporting the Playwright test infrastructure introduced in this PR.
tooling/playwright-www/.vscode/extensions.json (1)
1-3: LGTM!The Playwright extension recommendation is appropriate and will help developers with test authoring and debugging.
tooling/playwright-www/tsconfig.json (1)
1-103: LGTM!The TypeScript configuration is appropriate for a Playwright test project with strict type checking enabled and proper interop settings.
arkenv.code-workspace (1)
7-14: LGTM!The workspace folder additions properly organize the new tooling structure and content directories for better navigation in VSCode.
Also applies to: 27-34
package.json (1)
18-21: LGTM!The new E2E test scripts properly delegate to Turbo and provide clear entry points for different test modes (run, report, UI, update).
.github/workflows/release.yml (1)
18-24: LGTM: token generation order and cache configEarly token generation and consistent cache quoting look good. If you ever need private submodules, pass the token to actions/checkout; otherwise current setup is fine.
Also applies to: 35-35
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… changes - Added environment requirements section detailing Node.js version, supported operating systems, and browser installation. - Updated test command examples to use the new `pnpm run` syntax for consistency.
- Moved the page navigation and HTML structure validation checks to the correct position within the test for better clarity and execution flow. - Ensured that the checks for visibility and attachment of HTML elements occur after the page has been navigated to the root URL.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tooling/playwright-www/README.md (1)
43-46: Fix Turbo task name and verify package scope.
Likely should be test:e2e, not e2e. Also confirm the filter matches the actual package name.Apply this diff:
-# Run e2e tests for this package -turbo run e2e --filter=@repo/playwright-www +# Run e2e tests for this package +turbo run test:e2e --filter=@repo/playwright-wwwVerify Turbo task and package name:
#!/bin/bash # Find turbo config and verify task names fd -a turbo.json turbo.config.*.json turbo.*.json || true rg -nP '"test:e2e"|"\be2e\b"' -C3 --type=json # Confirm actual package name used for filter jq -r '.name' tooling/playwright-www/package.json 2>/dev/null
🧹 Nitpick comments (5)
tooling/playwright-www/tests/hello-world.spec.ts (2)
18-19: Consider using test artifacts for screenshots.The relative path
tests/screenshots/hello-world.pngmay not resolve correctly depending on the execution context. Playwright's test fixtures provide a more reliable approach viatestInfo.outputPath().Apply this diff to use test artifacts:
-test("hello world - basic page load", async ({ page }) => { +test("hello world - basic page load", async ({ page }, testInfo) => { // Navigate to the homepage await page.goto("/"); // Wait for the page to load await page.waitForLoadState("networkidle"); // Check that the page title is not empty const title = await page.title(); expect(title).toBeTruthy(); // Check that the page has some content const body = await page.locator("body"); await expect(body).toBeVisible(); // Take a screenshot for visual verification - await page.screenshot({ path: "tests/screenshots/hello-world.png" }); + await page.screenshot({ path: testInfo.outputPath("hello-world.png") }); });
22-44: Previous concern resolved; consider modernizing to Playwright 1.56.0 API.The console listener is now correctly registered before
page.goto("/"), resolving the previous review concern. However, Playwright 1.56.0 introduces a more reliable approach using thepage.consoleMessages()introspection API, which retrieves all console messages after page load without requiring event listener timing.Based on learnings
Apply this diff to use the newer API:
test("hello world - check for basic HTML structure", async ({ page }) => { - // Check that the page has loaded without major errors - const consoleErrors: string[] = []; - page.on("console", (msg) => { - if (msg.type() === "error") { - consoleErrors.push(msg.text()); - } - }); - await page.goto("/"); // Check that we have a proper HTML structure await expect(page.locator("html")).toBeVisible(); await expect(page.locator("head")).toBeAttached(); await expect(page.locator("body")).toBeVisible(); await page.waitForLoadState("networkidle"); - // Log any console errors for debugging + // Check for console errors using the new introspection API + const messages = await page.consoleMessages(); + const consoleErrors = messages + .filter((msg) => msg.type() === "error") + .map((msg) => msg.text()); + if (consoleErrors.length > 0) { console.log("Console errors found:", consoleErrors); } });tooling/playwright-www/playwright.config.ts (1)
66-70: Add timeout and output handling to webServer for reliability.The webServer configuration lacks a
timeoutand output stream handling, which can make debugging server startup failures difficult and cause tests to hang if the server is slow to start.Apply this diff to add recommended options:
/* Run your local dev server before starting the tests */ webServer: { command: "pnpm --filter=www run dev", url: "http://localhost:3000", reuseExistingServer: !process.env.CI, + timeout: 120 * 1000, // 2 minutes + stdout: "pipe", + stderr: "pipe", },These additions:
timeout: Explicitly sets a 2-minute startup timeout (adjust based on your server's typical startup time)stdout/stderr: "pipe": Pipes server output to test logs for easier debugging when startup failstooling/playwright-www/README.md (2)
15-21: Add dependency install step and prefer workspace exec.
Include pnpm install before installing browsers; use pnpm exec for local CLI.-### Install Dependencies - -First, install the Playwright browsers: +### Install dependencies and browsers + +Install package dependencies, then Playwright browsers: ```bash +npm i -g pnpm # if needed +pnpm install -npx playwright install +pnpm exec playwright install +# On Linux CI or fresh containers, consider: +# pnpm exec playwright install --with-deps--- `56-60`: **Align configuration notes with actual settings & add HTML report tip** After the list, add: ```diff + To view the HTML report locally after a run: + pnpm run test:e2e:report + + To change base URL/port or target browsers, edit tooling/playwright-www/playwright.config.ts (webServer.port, use.baseURL, projects).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tooling/playwright-www/README.md(1 hunks)tooling/playwright-www/playwright.config.ts(1 hunks)tooling/playwright-www/tests/hello-world.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Document environment requirements in README files
Files:
tooling/playwright-www/README.md
🪛 LanguageTool
tooling/playwright-www/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...ou have: - Node.js: v18.x or higher - Operating System: macOS, Linux, or Win...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ating System**: macOS, Linux, or Windows - Browsers: Playwright browsers installe...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...figuration The tests are configured to: - Start the www app on `http://localhost:3...
(QB_NEW_EN)
⏰ 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-e2e
🔇 Additional comments (3)
tooling/playwright-www/playwright.config.ts (1)
14-15: Verify single worker limitation in CI.Setting
workersto1in CI eliminates parallelism, which significantly slows test execution. While this may be intentional to avoid resource constraints or flaky tests, most modern CI environments handle multiple workers well. With only three browser projects configured, parallel execution would substantially reduce runtime.Consider increasing to
2or removing the override to use Playwright's default (50% of CPU cores):- /* Opt out of parallel tests on CI. */ - workers: process.env.CI ? 1 : undefined, + /* Use moderate parallelism on CI. */ + workers: process.env.CI ? 2 : undefined,tooling/playwright-www/README.md (2)
5-12: Environment requirements: LGTM.
Clear, concise, and meets guidelines.
25-37: Scripts in README match package.json Confirmedtest:e2e,test:e2e:ui,test:e2e:headed, andtest:e2e:debugare defined in tooling/playwright-www/package.json.
- Added Playwright as a devDependency in package.json. - Updated pnpm-lock.yaml and pnpm-workspace.yaml to reflect the new version. - Modified GitHub Actions workflow to use the updated command for installing Playwright browsers.
- Changed screenshot output path from `tests/screenshots/` to `tests/test-results/` in `hello-world.spec.ts`. - Updated `.gitignore` to reflect the new test results directory. - Modified `playwright.config.ts` to specify the output directory for test artifacts. - Revised README.md to describe the new structure for test artifacts.
- Modified the test command example from `turbo run e2e` to `turbo run test:e2e` for clarity and accuracy.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltooling/playwright-www/tests/screenshots/hello-world.pngis excluded by!**/*.png
📒 Files selected for processing (7)
.github/workflows/tests.yml(4 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)tooling/playwright-www/.gitignore(1 hunks)tooling/playwright-www/README.md(1 hunks)tooling/playwright-www/playwright.config.ts(1 hunks)tooling/playwright-www/tests/hello-world.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- tooling/playwright-www/playwright.config.ts
- tooling/playwright-www/tests/hello-world.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Document environment requirements in README files
Files:
tooling/playwright-www/README.md
🪛 LanguageTool
tooling/playwright-www/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...ou have: - Node.js: v18.x or higher - Operating System: macOS, Linux, or Win...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ating System**: macOS, Linux, or Windows - Browsers: Playwright browsers installe...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...figuration The tests are configured to: - Start the www app on `http://localhost:3...
(QB_NEW_EN)
- Changed the installation instructions to use `pnpm install` for project dependencies. - Updated the Playwright browser installation command to `pnpm exec playwright install` for consistency with the new setup.
1 participant
Set up Playwright within the project.
Addresses #51 but doesn't fully close it, as this is just the basic infra not the full test.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores