test: add Playwright E2E frontend test suite for 7 core user flows#456
test: add Playwright E2E frontend test suite for 7 core user flows#456bh462007 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughAdds a Playwright-based frontend E2E test setup with CI, local server tooling, shared fixtures, and page coverage for login, registration, dashboard, campaigns, leads, and settings. The README now documents how to run the suite locally and in CI. ChangesFrontend Playwright E2E Suite
Sequence Diagram(s)sequenceDiagram
participant RunTests as run-tests.js
participant StaticServer as static-server.js
participant PlaywrightCLI as Playwright CLI
RunTests->>StaticServer: start on 127.0.0.1:8081
RunTests->>StaticServer: poll until ready
RunTests->>PlaywrightCLI: launch frontend E2E tests
PlaywrightCLI->>StaticServer: load pages and assets
PlaywrightCLI-->>RunTests: exit code on completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a Playwright-based E2E test suite for the LeadOrbit frontend, intended to run fully offline via mocked API responses and to be enforced in CI on PRs to main.
Changes:
- Added Playwright test specs + shared fixtures for key frontend flows.
- Added a Node-based static server + runner script to start/stop the server around Playwright runs.
- Added CI workflow and README instructions for running E2E tests locally.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents the new Playwright E2E suite and how to run it locally/CI. |
frontend/package.json |
Adds E2E scripts and Playwright devDependencies. |
frontend/package-lock.json |
Locks Playwright (and other) dependencies for CI reproducibility. |
frontend/playwright.config.js |
Configures Playwright (baseURL, artifacts, retries, single worker). |
frontend/static-server.js |
Adds a lightweight static file server used during E2E runs. |
frontend/run-tests.js |
Adds a cross-platform test runner to start the server and invoke Playwright. |
frontend/.gitignore |
Ignores Playwright artifacts and node_modules under frontend/. |
frontend/tests/fixtures.js |
Adds shared Playwright fixtures/helpers for API mocking and auth seeding. |
frontend/tests/login.spec.js |
Login flow coverage using mocked auth + dashboard API. |
frontend/tests/register.spec.js |
Registration flow coverage with success/failure scenarios. |
frontend/tests/dashboard.spec.js |
Dashboard rendering + unauth redirect behavior coverage. |
frontend/tests/campaigns.spec.js |
Campaigns list page coverage (grid, search, stats, empty state). |
frontend/tests/leads.spec.js |
Leads page coverage (table, KPIs, search, filters, CSV import modal). |
frontend/tests/settings.spec.js |
Settings page coverage (profile data, buttons, toggles). |
.github/workflows/e2e.yml |
Runs the frontend E2E suite on pushes/PRs targeting main. |
Files not reviewed (1)
- frontend/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let urlPath = decodeURIComponent(req.url.split('?')[0]); | ||
| if (urlPath === '/') urlPath = '/index.html'; | ||
|
|
||
| const filePath = path.join(ROOT, urlPath); | ||
|
|
||
| // Prevent path traversal outside the served root | ||
| if (!filePath.startsWith(ROOT)) { | ||
| res.writeHead(403); | ||
| res.end('Forbidden'); | ||
| return; | ||
| } | ||
|
|
||
| fs.readFile(filePath, (err, data) => { | ||
| if (err) { | ||
| res.writeHead(404, { 'Content-Type': 'text/plain' }); | ||
| res.end('Not Found'); | ||
| return; | ||
| } | ||
| const ext = path.extname(filePath); | ||
| res.writeHead(200, { | ||
| 'Content-Type': MIME_TYPES[ext] || 'application/octet-stream', | ||
| 'Cache-Control': 'no-store', | ||
| }); |
| "serve": "node static-server.js", | ||
| "test:e2e": "node run-tests.js", | ||
| "test:e2e:headed": "node run-tests.js --headed", | ||
| "test:e2e:debug": "node_modules/.bin/playwright test --debug", |
| "devDependencies": { | ||
| "@playwright/test": "^1.61.0", | ||
| "http-server": "^14.1.1" | ||
| } |
| test.describe('Campaigns list', () => { | ||
| test.beforeEach(async ({ page }) => { | ||
| await seedAuthTokens(page); |
| - Tests live in frontend/tests/ | ||
| - API responses are mocked — no real backend needed | ||
| - A lightweight static file server serves the frontend on port 8081 | ||
| - Tests cover 7 core user flows: Login, Registration, Dashboard, Campaigns, Leads, Settings |
| ### Frontend E2E Tests (Playwright) | ||
|
|
||
| **Prerequisites:** Node.js 18+ | ||
|
|
||
| **Install dependencies:** | ||
| ```bash | ||
| cd frontend | ||
| npm install | ||
| npx playwright install --with-deps chromium | ||
| ``` | ||
|
|
||
| **Run all E2E tests (single command):** | ||
| ```bash | ||
| cd frontend | ||
| npm run test:e2e | ||
| ``` | ||
|
|
||
| **Other commands:** | ||
| ```bash | ||
| npm run test:e2e:headed # Run with visible browser window | ||
| npm run test:e2e:debug # Run in debug mode (step through tests) | ||
| npm run test:e2e:report # View the last HTML test report | ||
| ``` |
| **How it works:** | ||
| - Tests live in `frontend/tests/` | ||
| - API responses are mocked via Playwright's `page.route()` — no real backend needed | ||
| - A lightweight static file server (`static-server.js`) serves the frontend on port 8081 | ||
| - Tests cover 7 core user flows: Login, Registration, Dashboard, Campaigns, Leads, Settings, and Campaign Builder | ||
|
|
||
| **CI:** E2E tests run automatically on every PR targeting `main` via GitHub Actions (`.github/workflows/e2e.yml`). |
| http.get('http://127.0.0.1:8081/', (res) => { | ||
| const args = ['test', ...process.argv.slice(2)]; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 15-23: The workflow is using floating action tags and keeping
checkout credentials enabled unnecessarily. Update the GitHub Actions references
in the checkout/setup steps and the later job steps under the same workflow to
fixed version pins instead of mutable tags, and disable persisted credentials on
the repository checkout since this job does not push changes back. Use the
existing “Checkout repository” and “Set up Node.js” steps, plus the other
affected actions in the later section, as the places to harden.
In `@frontend/package.json`:
- Around line 7-10: The `test:e2e:debug` script bypasses the shared
`run-tests.js` harness, so it does not start `frontend/static-server.js` like
the other E2E commands. Update the `test:e2e:debug` entry in `package.json` to
route through `run-tests.js` with the debug flag, matching `test:e2e` and
`test:e2e:headed`, so the standalone debug flow uses the same server startup
path.
In `@frontend/run-tests.js`:
- Around line 5-36: The readiness check in waitForServer can falsely pass if
static-server.js exits early and some other process answers on 8081. Add
explicit monitoring for the server child’s exit/close event and fail immediately
if it dies before readiness, then only proceed to spawn Playwright when the
expected server process is still alive and the HTTP probe succeeds. Use the
existing server spawn and waitForServer flow to wire this in, and keep the
server shutdown/exit handling centralized so the test runner exits with a clear
error when the static server cannot start.
In `@frontend/static-server.js`:
- Around line 25-33: The path traversal guard in static-server.js is vulnerable
because filePath.startsWith(ROOT) can be bypassed by sibling paths that share a
prefix. In the request handling flow around urlPath, filePath, and the root
containment check, resolve the requested path to a canonical absolute path and
verify it is truly inside ROOT using a safe directory-boundary check before
serving any file. Keep the existing logic in the static file handler, but
replace the string-prefix containment test with a normalized path comparison
that rejects escapes outside the served root.
In `@frontend/tests/campaigns.spec.js`:
- Around line 69-76: The empty-state test in campaigns.spec.js only verifies
zeroed stats and can pass without actually rendering the empty-state branch.
Update the test around the campaigns page assertions to also check the
empty-state UI copy/CTA that is shown when the campaigns list is empty, and
assert that no campaign cards/items are rendered. Use the existing test case for
empty campaigns and the page locators tied to the campaigns grid or empty-state
container so the test proves the request/render path executed.
In `@frontend/tests/dashboard.spec.js`:
- Around line 24-25: The auth setup in dashboard.spec.js is mixing token seeding
and localStorage clearing in the same describe, which makes the unauthenticated
redirect test order-dependent and flaky. Update the test structure around
test.beforeEach and seedAuthTokens(page) so the unauthenticated case runs in a
separate describe or fixture that does not seed auth, or make auth seeding
opt-in per test instead of global. Keep the redirect test’s page.addInitScript
localStorage.clear behavior isolated from the seeded-auth tests.
In `@frontend/tests/fixtures.js`:
- Around line 16-19: The clearAuthTokens helper is over-clearing localStorage
instead of matching the app’s token cleanup behavior. Update clearAuthTokens in
fixtures.js to remove only the auth-related keys used by the token helpers in
api.js, namely access_token and refresh_token, rather than calling
localStorage.clear(). Keep the helper’s name and usage the same, but make its
behavior mirror the real sign-out/token removal flow so unrelated state stays
intact.
In `@README.md`:
- Around line 217-245: The README has duplicated Playwright E2E setup/docs in
the main E2E section and the later frontend-testing section, and they have
already drifted on the covered flows. Consolidate the Playwright documentation
into a single source of truth by removing one copy and updating the remaining
section’s coverage list in the E2E test instructions so it matches the actual
flows referenced by the frontend E2E docs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 500fff12-5753-4fe5-a8a0-fda4448aed75
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/workflows/e2e.ymlREADME.mdfrontend/.gitignorefrontend/package.jsonfrontend/playwright.config.jsfrontend/run-tests.jsfrontend/static-server.jsfrontend/tests/campaigns.spec.jsfrontend/tests/dashboard.spec.jsfrontend/tests/fixtures.jsfrontend/tests/leads.spec.jsfrontend/tests/login.spec.jsfrontend/tests/register.spec.jsfrontend/tests/settings.spec.js
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20' | ||
| cache: 'npm' | ||
| cache-dependency-path: frontend/package-lock.json |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the GitHub Actions and stop persisting checkout credentials.
Lines 16, 19, and 39 use floating tags, so upstream action changes can alter this workflow without a PR here. Also, this job never pushes back to the repo, so leaving the checkout token configured is unnecessary exposure.
Suggested hardening
- - name: Checkout repository
- uses: actions/checkout@v4
+ - name: Checkout repository
+ uses: actions/checkout@<full-commit-sha>
+ with:
+ persist-credentials: false
- - name: Set up Node.js
- uses: actions/setup-node@v4
+ - name: Set up Node.js
+ uses: actions/setup-node@<full-commit-sha>
- - name: Upload Playwright report
+ - name: Upload Playwright report
if: failure()
- uses: actions/upload-artifact@v4
+ uses: actions/upload-artifact@<full-commit-sha>Also applies to: 37-43
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 15-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e.yml around lines 15 - 23, The workflow is using
floating action tags and keeping checkout credentials enabled unnecessarily.
Update the GitHub Actions references in the checkout/setup steps and the later
job steps under the same workflow to fixed version pins instead of mutable tags,
and disable persisted credentials on the repository checkout since this job does
not push changes back. Use the existing “Checkout repository” and “Set up
Node.js” steps, plus the other affected actions in the later section, as the
places to harden.
Source: Linters/SAST tools
| "test:e2e": "node run-tests.js", | ||
| "test:e2e:headed": "node run-tests.js --headed", | ||
| "test:e2e:debug": "node_modules/.bin/playwright test --debug", | ||
| "test:e2e:report": "node_modules/.bin/playwright show-report" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Route the debug command through the same harness.
test:e2e:debug is the only documented E2E command that bypasses run-tests.js, so it never starts frontend/static-server.js. Because frontend/playwright.config.js also has no webServer, npm run test:e2e:debug needs a separate manual server even though the README presents it as a standalone command.
Suggested fix
- "test:e2e:debug": "node_modules/.bin/playwright test --debug",
+ "test:e2e:debug": "node run-tests.js --debug",📝 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:e2e": "node run-tests.js", | |
| "test:e2e:headed": "node run-tests.js --headed", | |
| "test:e2e:debug": "node_modules/.bin/playwright test --debug", | |
| "test:e2e:report": "node_modules/.bin/playwright show-report" | |
| "test:e2e": "node run-tests.js", | |
| "test:e2e:headed": "node run-tests.js --headed", | |
| "test:e2e:debug": "node run-tests.js --debug", | |
| "test:e2e:report": "node_modules/.bin/playwright show-report" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/package.json` around lines 7 - 10, The `test:e2e:debug` script
bypasses the shared `run-tests.js` harness, so it does not start
`frontend/static-server.js` like the other E2E commands. Update the
`test:e2e:debug` entry in `package.json` to route through `run-tests.js` with
the debug flag, matching `test:e2e` and `test:e2e:headed`, so the standalone
debug flow uses the same server startup path.
| const server = spawn(process.execPath, ['static-server.js'], { stdio: 'pipe' }); | ||
|
|
||
| // Use the actual Playwright CLI entry point, not the shell wrapper | ||
| const playwrightCli = path.join(__dirname, 'node_modules', '@playwright', 'test', 'cli.js'); | ||
|
|
||
| function waitForServer(retries = 20) { | ||
| http.get('http://127.0.0.1:8081/', (res) => { | ||
| const args = ['test', ...process.argv.slice(2)]; | ||
| const playwright = spawn(process.execPath, [playwrightCli, ...args], { | ||
| stdio: 'inherit', | ||
| }); | ||
| playwright.on('exit', (code) => { | ||
| server.kill(); | ||
| process.exit(code ?? 0); | ||
| }); | ||
| }).on('error', () => { | ||
| if (retries > 0) { | ||
| setTimeout(() => waitForServer(retries - 1), 300); | ||
| } else { | ||
| console.error('Server did not start in time'); | ||
| server.kill(); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| server.on('error', (err) => { | ||
| console.error('Failed to start server:', err); | ||
| process.exit(1); | ||
| }); | ||
|
|
||
| setTimeout(() => waitForServer(), 500); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail fast when the static server exits before readiness.
This script never watches the child exit event, and the readiness probe accepts any HTTP response. If static-server.js dies immediately (for example, EADDRINUSE on 8081), the probe can still succeed against some other local process and run the suite against the wrong server.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/run-tests.js` around lines 5 - 36, The readiness check in
waitForServer can falsely pass if static-server.js exits early and some other
process answers on 8081. Add explicit monitoring for the server child’s
exit/close event and fail immediately if it dies before readiness, then only
proceed to spawn Playwright when the expected server process is still alive and
the HTTP probe succeeds. Use the existing server spawn and waitForServer flow to
wire this in, and keep the server shutdown/exit handling centralized so the test
runner exits with a clear error when the static server cannot start.
| let urlPath = decodeURIComponent(req.url.split('?')[0]); | ||
| if (urlPath === '/') urlPath = '/index.html'; | ||
|
|
||
| const filePath = path.join(ROOT, urlPath); | ||
|
|
||
| // Prevent path traversal outside the served root | ||
| if (!filePath.startsWith(ROOT)) { | ||
| res.writeHead(403); | ||
| res.end('Forbidden'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Harden the traversal check against sibling-path escapes.
path.join(ROOT, urlPath) plus filePath.startsWith(ROOT) is not a safe containment check. A request like /../frontend-backup/secret.txt resolves outside the served root but still shares the same string prefix, so it passes this guard.
Suggested fix
- let urlPath = decodeURIComponent(req.url.split('?')[0]);
+ let urlPath = decodeURIComponent(req.url.split('?')[0]);
if (urlPath === '/') urlPath = '/index.html';
- const filePath = path.join(ROOT, urlPath);
+ const filePath = path.resolve(ROOT, `.${urlPath}`);
// Prevent path traversal outside the served root
- if (!filePath.startsWith(ROOT)) {
+ if (filePath !== ROOT && !filePath.startsWith(`${ROOT}${path.sep}`)) {
res.writeHead(403);
res.end('Forbidden');
return;
}📝 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.
| let urlPath = decodeURIComponent(req.url.split('?')[0]); | |
| if (urlPath === '/') urlPath = '/index.html'; | |
| const filePath = path.join(ROOT, urlPath); | |
| // Prevent path traversal outside the served root | |
| if (!filePath.startsWith(ROOT)) { | |
| res.writeHead(403); | |
| res.end('Forbidden'); | |
| let urlPath = decodeURIComponent(req.url.split('?')[0]); | |
| if (urlPath === '/') urlPath = '/index.html'; | |
| const filePath = path.resolve(ROOT, `.${urlPath}`); | |
| // Prevent path traversal outside the served root | |
| if (filePath !== ROOT && !filePath.startsWith(`${ROOT}${path.sep}`)) { | |
| res.writeHead(403); | |
| res.end('Forbidden'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/static-server.js` around lines 25 - 33, The path traversal guard in
static-server.js is vulnerable because filePath.startsWith(ROOT) can be bypassed
by sibling paths that share a prefix. In the request handling flow around
urlPath, filePath, and the root containment check, resolve the requested path to
a canonical absolute path and verify it is truly inside ROOT using a safe
directory-boundary check before serving any file. Keep the existing logic in the
static file handler, but replace the string-prefix containment test with a
normalized path comparison that rejects escapes outside the served root.
Source: Linters/SAST tools
| test('empty state shows when no campaigns exist', async ({ page, mockApi }) => { | ||
| await mockApi('**/api/v1/campaigns/**', []); | ||
|
|
||
| await page.goto('/campaigns.html'); | ||
|
|
||
| await expect(page.locator('#campaigns-grid')).toBeVisible({ timeout: 10000 }); | ||
| await expect(page.locator('#stat-total')).toHaveText('0'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the empty-state test prove the empty-state branch rendered.
Right now this can pass on default zeroed UI without exercising the request/render path. The test name says "empty state shows", but it never checks the empty-state copy/CTA or that no campaign cards are rendered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/campaigns.spec.js` around lines 69 - 76, The empty-state test
in campaigns.spec.js only verifies zeroed stats and can pass without actually
rendering the empty-state branch. Update the test around the campaigns page
assertions to also check the empty-state UI copy/CTA that is shown when the
campaigns list is empty, and assert that no campaign cards/items are rendered.
Use the existing test case for empty campaigns and the page locators tied to the
campaigns grid or empty-state container so the test proves the request/render
path executed.
| test.beforeEach(async ({ page }) => { | ||
| await seedAuthTokens(page); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does Playwright 1.61.0 guarantee the execution order of multiple page.addInitScript() calls on the same page? Please cite the official Playwright docs.
💡 Result:
No, Playwright does not guarantee the execution order of multiple page.addInitScript calls. According to the official Playwright documentation, the order of evaluation for multiple scripts installed via page.addInitScript (as well as browserContext.addInitScript) is explicitly defined as not defined [1][2][3].
Citations:
- 1: https://playwright.dev/docs/api/class-page
- 2: https://playwright.dev/docs/api/class-browsercontext
- 3: https://playwright.dev/java/docs/api/class-page
🏁 Script executed:
sed -n '1,120p' frontend/tests/dashboard.spec.js && printf '\n---\n' && sed -n '1,120p' frontend/tests/fixtures.jsRepository: Kuldeeep18/LeadOrbit
Length of output: 3527
Avoid mixing auth seeding and auth clearing via page.addInitScript() in the same describe.
seedAuthTokens(page) in test.beforeEach and the unauthenticated test's page.addInitScript(() => window.localStorage.clear()) rely on an undefined init-script order, so the clear can run before the tokens are seeded and make this redirect test flaky. Move the unauthenticated case to a separate describe/fixture that skips auth seeding, or make seeding opt-in per test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/dashboard.spec.js` around lines 24 - 25, The auth setup in
dashboard.spec.js is mixing token seeding and localStorage clearing in the same
describe, which makes the unauthenticated redirect test order-dependent and
flaky. Update the test structure around test.beforeEach and seedAuthTokens(page)
so the unauthenticated case runs in a separate describe or fixture that does not
seed auth, or make auth seeding opt-in per test instead of global. Keep the
redirect test’s page.addInitScript localStorage.clear behavior isolated from the
seeded-auth tests.
| async function clearAuthTokens(page) { | ||
| await page.addInitScript(() => { | ||
| window.localStorage.clear(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
clearAuthTokens should only remove the auth keys.
The app-side token helpers in frontend/api.js:8-18 delete only access_token and refresh_token, but this helper clears all local storage. That can erase unrelated state like api_base_url or UI preferences and make test behavior diverge from the real auth flow.
Suggested fix
async function clearAuthTokens(page) {
await page.addInitScript(() => {
- window.localStorage.clear();
+ window.localStorage.removeItem('access_token');
+ window.localStorage.removeItem('refresh_token');
});
}📝 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.
| async function clearAuthTokens(page) { | |
| await page.addInitScript(() => { | |
| window.localStorage.clear(); | |
| }); | |
| async function clearAuthTokens(page) { | |
| await page.addInitScript(() => { | |
| window.localStorage.removeItem('access_token'); | |
| window.localStorage.removeItem('refresh_token'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/tests/fixtures.js` around lines 16 - 19, The clearAuthTokens helper
is over-clearing localStorage instead of matching the app’s token cleanup
behavior. Update clearAuthTokens in fixtures.js to remove only the auth-related
keys used by the token helpers in api.js, namely access_token and refresh_token,
rather than calling localStorage.clear(). Keep the helper’s name and usage the
same, but make its behavior mirror the real sign-out/token removal flow so
unrelated state stays intact.
| ### Frontend E2E Tests (Playwright) | ||
|
|
||
| **Prerequisites:** Node.js 18+ | ||
|
|
||
| **Install dependencies:** | ||
|
|
||
| cd frontend | ||
| npm install | ||
| npx playwright install --with-deps chromium | ||
|
|
||
| **Run all E2E tests (single command):** | ||
|
|
||
| cd frontend | ||
| npm run test:e2e | ||
|
|
||
| **Other commands:** | ||
|
|
||
| npm run test:e2e:headed # Run with visible browser window | ||
| npm run test:e2e:debug # Run in debug mode | ||
| npm run test:e2e:report # View the last HTML test report | ||
|
|
||
| **How it works:** | ||
| - Tests live in frontend/tests/ | ||
| - API responses are mocked — no real backend needed | ||
| - A lightweight static file server serves the frontend on port 8081 | ||
| - Tests cover 7 core user flows: Login, Registration, Dashboard, Campaigns, Leads, Settings | ||
|
|
||
| **CI:** E2E tests run automatically on every PR targeting main via GitHub Actions (.github/workflows/e2e.yml). | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Consolidate the duplicated Playwright README section.
The README now has two separate E2E sections, and they already disagree: Line 242 says there are 7 flows but only lists 6, while Line 533 adds Campaign Builder explicitly. Keeping both copies will keep the setup instructions and coverage claims drifting.
Also applies to: 504-535
🧰 Tools
🪛 LanguageTool
[uncategorized] ~244-~244: The official name of this software platform is spelled with a capital “H”.
Context: ... PR targeting main via GitHub Actions (.github/workflows/e2e.yml). ### Backend Tests ...
(GITHUB)
🪛 markdownlint-cli2 (0.22.1)
[warning] 223-223: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
[warning] 229-229: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
[warning] 234-234: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 217 - 245, The README has duplicated Playwright E2E
setup/docs in the main E2E section and the later frontend-testing section, and
they have already drifted on the covered flows. Consolidate the Playwright
documentation into a single source of truth by removing one copy and updating
the remaining section’s coverage list in the E2E test instructions so it matches
the actual flows referenced by the frontend E2E docs.
Related Issue
Closes #293
Summary
This PR adds a complete Playwright end-to-end (E2E) test suite for the LeadOrbit frontend.
The test suite covers the seven required core user flows using mocked API responses, so the tests can run without a backend. It also adds GitHub Actions integration to run the tests automatically and updates the README with instructions for running them locally.
No production code was modified.
Type of Change
Files Added
frontend/package.jsonfrontend/playwright.config.jsfrontend/static-server.jsfrontend/run-tests.jsfrontend/.gitignorefrontend/tests/fixtures.jsfrontend/tests/login.spec.jsfrontend/tests/register.spec.jsfrontend/tests/dashboard.spec.jsfrontend/tests/campaigns.spec.jsfrontend/tests/leads.spec.jsfrontend/tests/settings.spec.js.github/workflows/e2e.ymlREADME.mdTest Coverage
Acceptance Criteria
npm run test:e2eRunning the Tests
cd frontend npm install npx playwright install --with-deps chromium npm run test:e2eTest Results
Notes
page.route()and return deterministic mock responses. The tests never contact a real backend or any external service.run-tests.jslauncher is used instead of Playwright's built-inwebServeroption to avoid process cleanup issues on Windows.idattributes and accessible roles rather than fragile CSS selectors, making them less prone to breaking after UI changes.localStorage, ensuring tests remain isolated from one another.Summary by CodeRabbit
New Features
Documentation
Bug Fixes