feat: add GitHub Actions workflow for E2E tests - #36
Conversation
Add Playwright E2E test workflow that runs on PRs and pushes to master. Uses Xvfb for headless Electron testing on Ubuntu runners. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new GitHub Actions workflow Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (push/PR)
participant GH as GitHub Actions
participant Runner as Ubuntu Runner
participant Setup as Setup (pnpm / Node)
participant Build as Electron Build
participant Test as Playwright (Xvfb)
participant Pinecone as Pinecone API
participant Store as Artifact Storage
Dev->>GH: push / open PR (master)
GH->>Runner: start e2e-tests job
Runner->>Setup: checkout, setup pnpm & Node 22, install deps
Setup->>Build: build Electron app (CI: use --no-sandbox)
Runner->>Test: start Playwright E2E with app
Test->>Pinecone: use PINECONE_API_KEY for API calls
Test->>Store: upload playwright-report and test-results
GH->>Dev: workflow result & artifacts available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/e2e-tests.yml:
- Around line 48-52: The E2E step "Run E2E tests with Xvfb" references the
secret PINECONE_API_KEY which is absent for forked PRs; add a guard to the step
(e.g., add an if condition like if: ${{ secrets.PINECONE_API_KEY }} or another
expression that checks the secret presence) so the step is skipped when
PINECONE_API_KEY is missing, preserving the env entry PINECONE_API_KEY: ${{
secrets.PINECONE_API_KEY }} but preventing failure on forks.
- Around line 40-44: The workflow installs Electron deps but omits the X virtual
framebuffer; add the xvfb package to the apt-get install list so xvfb-run (used
later on line 49) is available; update the Install system dependencies for
Electron step by including "xvfb" alongside libnss3, libatk-bridge2.0-0,
libgtk-3-0, libgbm1 and libasound2 so the job can run xvfb-run successfully.
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yml (1)
31-35: Add Node.js engine specification to document version requirement.Node.js 22 is pinned in the workflow, but there's no corresponding engine specification in
package.jsonor.nvmrcfile. Add anenginesfield topackage.jsonto document that Node 22 is the required/minimum version:"engines": { "node": ">=22" }Alternatively, create a
.nvmrcfile with22to maintain consistency across development and CI environments.
| - name: Install system dependencies for Electron | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y libnss3 libatk-bridge2.0-0 libgtk-3-0 libgbm1 libasound2 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/e2e-tests.ymlRepository: stepandel/pinecone-explorer
Length of output: 1941
🌐 Web query:
GitHub Actions ubuntu-latest default packages xvfb xvfb-run preinstalled
💡 Result:
ubuntu-latestcurrently means the GitHub-hosted runner image for Ubuntu 24.04 (same asubuntu-24.04). [1] (This switch rolled out Dec 5, 2024 → Jan 17, 2025.) [2]- Whether
xvfb/xvfb-runis preinstalled is not something you should assume from memory because runner images change frequently; GitHub recommends checking the exact runner’s “Included Software” link in your workflow run logs (Set up job → Runner Image → Included Software). [3] - If you need it,
xvfb-runis provided by the Ubuntuxvfbpackage (so installingxvfbgives you bothXvfbandxvfb-run). [4]
Practical check in a job step
command -v Xvfb || true
command -v xvfb-run || true
dpkg -l | grep -E '^ii\s+xvfb\s' || trueInstall (if missing)
sudo apt-get update
sudo apt-get install -y xvfbSources: [1] actions/runner-images repo table (ubuntu-latest → 24.04), [2] runner-images announcement issue, [3] GitHub Docs on “Included Software” link in logs, [4] Ubuntu Launchpad package description for xvfb (mentions xvfb-run).
🌐 Web query:
Ubuntu 24.04 xvfb preinstalled default packages
💡 Result:
On a standard Ubuntu 24.04 LTS (Noble) install, Xvfb is not preinstalled as a default package (it is not part of the ubuntu-desktop metapackage’s dependency list). [1]
If you need it, it’s available as the xvfb package (from the Universe repository) for Ubuntu 24.04 and can be installed via APT. [2]
Sources
[1] Ubuntu package page for ubuntu-desktop (Noble 24.04) — dependency list does not include xvfb: packages.ubuntu.com
[2] Ubuntu package search — xvfb in Noble (24.04) (Universe): packages.ubuntu.com
Add xvfb to system dependencies.
Line 49 uses xvfb-run, which is not preinstalled on ubuntu-latest (Ubuntu 24.04). The workflow will fail without it.
🧩 Suggested patch
- sudo apt-get install -y libnss3 libatk-bridge2.0-0 libgtk-3-0 libgbm1 libasound2
+ sudo apt-get install -y libnss3 libatk-bridge2.0-0 libgtk-3-0 libgbm1 libasound2 xvfb📝 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.
| - name: Install system dependencies for Electron | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y libnss3 libatk-bridge2.0-0 libgtk-3-0 libgbm1 libasound2 | |
| - name: Install system dependencies for Electron | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y libnss3 libatk-bridge2.0-0 libgtk-3-0 libgbm1 libasound2 xvfb |
🤖 Prompt for AI Agents
In @.github/workflows/e2e-tests.yml around lines 40 - 44, The workflow installs
Electron deps but omits the X virtual framebuffer; add the xvfb package to the
apt-get install list so xvfb-run (used later on line 49) is available; update
the Install system dependencies for Electron step by including "xvfb" alongside
libnss3, libatk-bridge2.0-0, libgtk-3-0, libgbm1 and libasound2 so the job can
run xvfb-run successfully.
| - name: Run E2E tests with Xvfb | ||
| run: xvfb-run --auto-servernum --server-args="-screen 0 1920x1080x24" pnpm exec playwright test | ||
| env: | ||
| PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} | ||
|
|
There was a problem hiding this comment.
Guard E2E execution when the secret is unavailable.
Line 51 uses PINECONE_API_KEY. For fork PRs, secrets aren’t provided and this step will fail. Consider skipping or short‑circuiting when the secret is missing.
✅ Suggested guard
- name: Run E2E tests with Xvfb
+ if: ${{ secrets.PINECONE_API_KEY != '' }}
run: xvfb-run --auto-servernum --server-args="-screen 0 1920x1080x24" pnpm exec playwright test
env:
PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }}
+
+ - name: Skip E2E tests (missing PINECONE_API_KEY)
+ if: ${{ secrets.PINECONE_API_KEY == '' }}
+ run: echo "PINECONE_API_KEY not available for this event; skipping E2E tests."🤖 Prompt for AI Agents
In @.github/workflows/e2e-tests.yml around lines 48 - 52, The E2E step "Run E2E
tests with Xvfb" references the secret PINECONE_API_KEY which is absent for
forked PRs; add a guard to the step (e.g., add an if condition like if: ${{
secrets.PINECONE_API_KEY }} or another expression that checks the secret
presence) so the step is skipped when PINECONE_API_KEY is missing, preserving
the env entry PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} but preventing
failure on forks.
Ubuntu 24.04 (noble) renamed libasound2 to libasound2t64. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The layout-ui tests use a separate electron-app.ts helper that was missing the --no-sandbox flag needed for GitHub Actions runners. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@e2e/helpers/electron-app.ts`:
- Around line 52-60: The args array is built with appPath but prepends
'--no-sandbox' using args.unshift, causing argument order mismatch with other
setup code and potentially breaking electron.launch; change the logic to append
the flag (use args.push('--no-sandbox') instead of args.unshift) so args becomes
[appPath, '--no-sandbox'] when process.env.CI is set, ensuring the main script
(appPath) remains the first argument passed to electron.launch.
| // Build args - add --no-sandbox for CI environments (GitHub Actions, etc.) | ||
| const args = [appPath]; | ||
| if (process.env.CI) { | ||
| args.unshift('--no-sandbox'); | ||
| } | ||
|
|
||
| // Launch Electron app | ||
| const app = await electron.launch({ | ||
| args: [appPath], | ||
| args, |
There was a problem hiding this comment.
Inconsistent argument order: use push instead of unshift.
The --no-sandbox flag is prepended before appPath using unshift, resulting in ['--no-sandbox', appPath]. However, in e2e/electron.setup.ts (lines 79-82), the same logic uses push, placing the app path first: [electronPath, '--no-sandbox'].
For Playwright's electron.launch(), the main script path should typically be the first argument. This inconsistency could cause unexpected behavior and makes the codebase harder to maintain.
Proposed fix to align with electron.setup.ts
// Build args - add --no-sandbox for CI environments (GitHub Actions, etc.)
const args = [appPath];
if (process.env.CI) {
- args.unshift('--no-sandbox');
+ args.push('--no-sandbox');
}📝 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.
| // Build args - add --no-sandbox for CI environments (GitHub Actions, etc.) | |
| const args = [appPath]; | |
| if (process.env.CI) { | |
| args.unshift('--no-sandbox'); | |
| } | |
| // Launch Electron app | |
| const app = await electron.launch({ | |
| args: [appPath], | |
| args, | |
| // Build args - add --no-sandbox for CI environments (GitHub Actions, etc.) | |
| const args = [appPath]; | |
| if (process.env.CI) { | |
| args.push('--no-sandbox'); | |
| } | |
| // Launch Electron app | |
| const app = await electron.launch({ | |
| args, |
🤖 Prompt for AI Agents
In `@e2e/helpers/electron-app.ts` around lines 52 - 60, The args array is built
with appPath but prepends '--no-sandbox' using args.unshift, causing argument
order mismatch with other setup code and potentially breaking electron.launch;
change the logic to append the flag (use args.push('--no-sandbox') instead of
args.unshift) so args becomes [appPath, '--no-sandbox'] when process.env.CI is
set, ensuring the main script (appPath) remains the first argument passed to
electron.launch.
1 participant
Add Playwright E2E test workflow that runs on PRs and pushes to master. Uses Xvfb for headless Electron testing on Ubuntu runners.
Summary by CodeRabbit