fix(e2e): replace hardcoded URLs with Playwright baseURL configuration#1842
fix(e2e): replace hardcoded URLs with Playwright baseURL configuration#1842aaradhychinche-alt wants to merge 5 commits intoruxailab:developfrom
Conversation
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
… wait strategy
- playwright.config.ts was the active config (takes priority over .js) but had
baseURL commented out, causing 'Cannot navigate to invalid URL' for all relative
paths. Uncomment and set to process.env.BASE_URL || 'http://localhost:8080'.
- playwright.config.js: remove redundant ...devices['Desktop Chrome'] spread from
the global use block (device presets belong only inside each project entry).
Add explanatory comments on baseURL and project inheritance.
- e2e/createHeuristicTest.spec.js: replace waitUntil: 'networkidle' with
'domcontentloaded' + explicit waitFor on the Email field. Firebase keeps
persistent WebSocket connections open so networkidle never resolves on this app.
Pass testInfo to logIn() and log testInfo.project.use.baseURL for debugging.
Fixes: relative page.goto('/signin') calls introduced in the URL refactor.
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to make Playwright E2E tests environment-agnostic by configuring a Playwright baseURL and switching test navigation from hardcoded absolute URLs to relative paths.
Changes:
- Set
baseURLin Playwright config (with env override). - Replace
page.goto('http://localhost:8080/...')withpage.goto('/...')across several E2E specs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| playwright.config.js | Adds env-driven baseURL to support relative navigation in tests. |
| e2e/template_testing.spec.ts | Updates navigation to use / instead of hardcoded localhost URL. |
| e2e/signIntest.spec.js | Updates Sign In navigation to use /signin relative path. |
| e2e/ruxailabtest.spec.js | Updates Sign In navigation to use /signin relative path. |
| e2e/createHeuristicTest.spec.js | Updates Sign In navigation to use /signin relative path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Base URL so relative paths like page.goto('/signin') resolve correctly. | ||
| Override by setting BASE_URL env var, e.g. BASE_URL=https://staging.example.com */ | ||
| baseURL: process.env.BASE_URL || 'http://localhost:8080', |
There was a problem hiding this comment.
playwright test is invoked without --config, and this repo contains both playwright.config.ts and playwright.config.js (Dockerfile-playwright copies both). If Playwright resolves the TS config by default, baseURL from this JS file won’t be applied and page.goto('/...') will fail. To avoid ambiguous/incorrect config selection, either move this baseURL setting into the config file that’s actually used (likely playwright.config.ts), or remove/rename the unused config so only one default config exists.
| /* Base URL so relative paths like page.goto('/signin') resolve correctly. | |
| Override by setting BASE_URL env var, e.g. BASE_URL=https://staging.example.com */ | |
| baseURL: process.env.BASE_URL || 'http://localhost:8080', |
| use: { | ||
| baseURL: devBaseUrl, | ||
| ...devices['Desktop Chrome'], | ||
| /* Base URL so relative paths like page.goto('/signin') resolve correctly. |
There was a problem hiding this comment.
Using process.env.BASE_URL here is easy to confuse with Vue CLI’s BASE_URL (often set to a public path like /). If that env var is present when running Playwright, it can produce an invalid baseURL. Consider switching to a more specific env var (e.g. PLAYWRIGHT_BASE_URL/E2E_BASE_URL) and/or validating that it’s a full origin URL (scheme + host).
| use: { | ||
| baseURL: devBaseUrl, | ||
| ...devices['Desktop Chrome'], | ||
| /* Base URL so relative paths like page.goto('/signin') resolve correctly. |
There was a problem hiding this comment.
The default fallback here is http://localhost:8080, but the repo’s Dockerfile-playwright serves the app on port 5000 and runs npx playwright test without setting a base URL. If that Docker workflow is expected to work, the fallback should align with the served port or the Docker command should export the env var used here.
…x SonarCloud duplication Introduce a typed baseUse constant that holds the single definition of baseURL. Spread it into both the global use block and each project's use block so the string literal 'process.env.BASE_URL || http://localhost:8080' appears exactly once in the file, eliminating the duplicated-new-code finding in SonarCloud. Also remove commented-out boilerplate (mobile/branded browser projects, dotenv import, webServer block) that inflated the scanned line count and contributed to the duplication percentage on new code. Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
…etition
Replace three hand-written project objects with a typed browserProjects tuple
array and a single projects.map() callback. The use-block shape
{ ...baseUse, ...device } is now written exactly once instead of three times,
removing the structural duplication that SonarCloud flagged on new code.
Adding or removing a browser now requires a single line in browserProjects
rather than copying an entire project block.
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,81 +1,47 @@ | |||
| import { defineConfig, devices } from '@playwright/test' | |||
| import { defineConfig, devices, PlaywrightTestOptions } from '@playwright/test' | |||
There was a problem hiding this comment.
PlaywrightTestOptions is only used as a type here, but it’s imported as a runtime value. Consider switching this to a type-only import (or using type PlaywrightTestOptions in the import) to avoid carrying an unnecessary runtime import and to prevent issues if the config is ever loaded in ESM mode.
| import { defineConfig, devices, PlaywrightTestOptions } from '@playwright/test' | |
| import { defineConfig, devices, type PlaywrightTestOptions } from '@playwright/test' |
| * Shared base options spread into the global use block and every project. | ||
| * Defined once here — the single source of truth that SonarCloud checks. | ||
| * |
There was a problem hiding this comment.
This file’s header comment says it is the “single source of truth”, but the repo also maintains a parallel playwright.config.js with overlapping settings (including baseURL). That duplication makes it easy for the two configs to drift; consider consolidating into a single Playwright config or updating test scripts/CI to explicitly select one config via --config so there’s a clear source of truth.
| * Shared base options spread into the global use block and every project. | |
| * Defined once here — the single source of truth that SonarCloud checks. | |
| * | |
| * Shared base options for this Playwright config, spread into the global | |
| * use block and every project defined below. | |
| * | |
| * Keep this in sync with any other Playwright config files (for example, | |
| * playwright.config.js) so that settings like baseURL do not drift. |
| // --- Reusable login function --- | ||
| const logIn = async (page) => { | ||
| const logIn = async (page, testInfo) => { | ||
| console.log('Base URL:', testInfo.project.use.baseURL); |
There was a problem hiding this comment.
The new console.log('Base URL:', ...) will add noise to test output and can leak environment details in CI logs. Consider removing it or gating it behind an explicit debug flag (e.g., an env var) so it’s only printed when troubleshooting.
| console.log('Base URL:', testInfo.project.use.baseURL); | |
| if (process.env.E2E_DEBUG_BASE_URL === 'true') { | |
| console.log('Base URL:', testInfo.project.use.baseURL); | |
| } |
Extract three repeated code sequences into named helper functions: - login(page): signs in via /signin using the shared test account. Replaces the 4-line block repeated verbatim in every test. - goToPersonalTemplates(page): clicks Templates then Personal tab. Replaces the 2-line block repeated in every test. - openNewUserTestDialog(page): clicks create-test-btn, selects 'Create a blank test' > 'Testing' > 'Webcam, audio & screen record'. Replaces the 4-line block repeated in 4 of the 5 tests. All tests are wrapped in test.describe with beforeEach calling login() so sign-in runs once per test without repetition in test bodies. No assertions or test logic changed. Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
|
|
@marcgc21 ready to be reviewed !!! |
|
@KarinePistili can You review this once when you have time ? ! Thank You ! |



Closes #1827
Summary
This PR replaces hardcoded URLs in Playwright tests with relative paths and configures
baseURLin the Playwright configuration.Previously, some tests used absolute URLs such as:
await page.goto('http://localhost:8080/signin')
This makes the tests environment-specific and harder to run in CI or staging environments.
With this change, tests now use relative paths while relying on Playwright's
baseURLconfiguration.Changes
baseURLconfiguration inplaywright.config.jsExample:
Before
await page.goto('http://localhost:8080/signin')
After
await page.goto('/signin')
Benefits
Screenshots / Media
Before
Tests used hardcoded navigation URLs:
await page.goto('http://localhost:8080/signin')
After
Tests now rely on the configured
baseURL:await page.goto('/signin')
Testing
baseURL