refactor(lightspeed): split Lightspeed e2e into three spec files by HusneShabbir · Pull Request #2809 · redhat-developer/rhdh-plugins

HusneShabbir · 2026-04-17T11:34:53Z

Description

What: Replace lightspeed.test.ts with lightspeed.ui.test.ts, lightspeed.mcp.test.ts, and lightspeed.conversation.test.ts. Shared setup lives in utils/lightspeedE2eSetup.ts.

Why: Smaller, clearer suites grouped by UI vs MCP vs chat flows; easier to maintain.

Note: Guest login is inlined in the bootstrap helper; utils/login.ts removed.

CI / apps: E2e lives under workspaces/lightspeed/e2e-tests/ and runs against both frontends: yarn test:e2e:legacy (APP_MODE=legacy, packages/app-legacy) and yarn test:e2e:nfs (APP_MODE=nfs, packages/app); yarn playwright test runs both in sequence. Guest login post-checks are mode-specific (legacy: “Red Hat Catalog” heading; NFS: /catalog URL) so each shell can finish bootstrap reliably.

Resolves:
https://redhat.atlassian.net/browse/RHIDP-13186

rhdh-qodo-merge · 2026-04-17T11:35:00Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

🐞\ ☼ Reliability (1)

1. Project locale not applied 🐞 ☼

Description

bootstrapLightspeedE2ePage creates a new BrowserContext without passing the Playwright project
locale, so the suite derives locale from the machine default rather than the configured
per-project locale. This can silently defeat the intended multi-locale coverage and can break
locale-dependent assertions/fixtures when navigator.language differs from the project locale.

Code

workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/lightspeedE2eSetup.ts[R54-60]

+export async function bootstrapLightspeedE2ePage(
+  browser: Browser,
+): Promise<LightspeedE2eBootstrap> {
+  const context = await browser.newContext();
+  const page = await context.newPage();
+  const locale = await page.evaluate(() => globalThis.navigator.language);
+  const translations = getTranslations(locale);

Relevance

⭐⭐⭐ High
bootstrap creates newContext() without locale; likely defeats per-project locale coverage in
playwright.config projects.
PR-#1798
PR-#2502

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The Playwright config defines per-project locale via `projects: LOCALES.map(locale => ({ use: {
locale } })), but the bootstrap helper creates its own context via browser.newContext()` with no
options and then reads navigator.language, meaning it will not use the project’s configured locale
when creating the context.
workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/lightspeedE2eSetup.ts[54-60]
workspaces/lightspeed/playwright.config.ts[57-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`bootstrapLightspeedE2ePage` creates a fresh `browser.newContext()` without passing the Playwright project’s configured `locale`. As a result, `navigator.language` (and subsequent translation selection / locale switching) reflect the machine default, not the current Playwright project.

### Issue Context
The Playwright configuration runs the suite as multiple locale projects (`en`, `de`, `es`, `fr`, `it`, `ja`). Those settings only apply automatically when Playwright creates the context; manually creating a context must explicitly include the desired locale.

### Fix Focus Areas
- workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/lightspeedE2eSetup.ts[54-76]
- workspaces/lightspeed/playwright.config.ts[57-65]

### Suggested fix
- Update `bootstrapLightspeedE2ePage` to accept a `locale` parameter and create the context with `await browser.newContext({ locale })`.
- In each `test.beforeAll`, pass the project locale (e.g., via `test.info().project.use.locale` or a `testInfo` hook parameter, depending on your Playwright version).
- (Optional) return `context` from the bootstrap result and close it in `afterAll`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Invalid package.json upload path~~ ☑ 🐞 ≡

Description

The file attachment validation test uses the relative path '../../package.json', but the e2e
command is run from the Lightspeed workspace root, making that path resolve outside the workspace
and not to an existing file. This will cause fileChooser.setFiles() to fail because the file
cannot be found.

Code

workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[R187-189]
+    const testFiles = [
+      { path: '../../package.json', name: 'package.json' },
+      { path: __filename, name: 'fileAttachment.spec.ts' },

Relevance

⭐ Low
Same ../../package.json path pattern already merged and likely resolves to repo-root package.json;
not failing in CI.
PR-#1798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The e2e script runs playwright test from the workspace root
(workspaces/lightspeed/package.json). uploadFiles() passes the provided path directly to
fileChooser.setFiles() with no path normalization, so '../../package.json' will be resolved
relative to the process working directory and is very likely to point outside the workspace rather
than to workspaces/lightspeed/package.json.
workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[187-196]
workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/fileUpload.ts[33-40]
workspaces/lightspeed/package.json[22-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The file upload test uses a relative path that does not resolve to an existing file when running `yarn test:e2e:legacy` from the Lightspeed workspace root.

### Issue Context
`uploadFiles()` calls `fileChooser.setFiles(filePath)` directly, so the string must be a valid path from the test runner’s working directory.

### Fix Focus Areas
- workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[187-196]
- workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/fileUpload.ts[33-40]
- workspaces/lightspeed/package.json[22-26]

### Suggested fix
- Change `'../../package.json'` to `'package.json'` (workspace-root-relative), or compute an absolute path using `path.resolve(__dirname, '../../../package.json')`.
- Keep paths consistent with the other upload fixtures in this file that already use workspace-root-relative paths.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Wrong uploaded filename expected~~ ☑ 🐞 ≡

Description

The file attachment validation test uploads __filename but asserts the uploaded file name is
fileAttachment.spec.ts, which will not match the actual basename (lightspeed.ui.test.ts). This
will fail assertions that look for a button/label with the provided expected file name.

Code

workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[R186-190]

+  test.describe('File Attachment Validation', () => {
+    const testFiles = [
+      { path: '../../package.json', name: 'package.json' },
+      { path: __filename, name: 'fileAttachment.spec.ts' },
+    ];

Relevance

⭐ Low
Test uses name-based extension; __filename entry is .ts so it never asserts button name.
PR-#1798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test data sets { path: __filename, name: 'fileAttachment.spec.ts' }. The upload assertion
helper checks for a UI element matching fileName exactly (`page.getByRole('button', { name:
fileName })). Since __filename refers to the current file (lightspeed.ui.test.ts`), the expected
name is wrong and the assertion will fail.
workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[186-204]
workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/fileUpload.ts[42-75]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The file upload validation test uploads `__filename` but expects a hard-coded filename that no longer matches after the suite rename/split.

### Issue Context
`uploadAndAssertDuplicate` ultimately calls `validateSuccessfulUpload`, which asserts the UI shows the exact `fileName` passed from the test data.

### Fix Focus Areas
- workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts[186-190]
- workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/fileUpload.ts[42-75]

### Suggested fix
- Replace `'fileAttachment.spec.ts'` with `path.basename(__filename)` (import Node `path`), or update it to `'lightspeed.ui.test.ts'`.
- Prefer the computed basename so future renames don’t break the test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

rhdh-gh-app · 2026-04-17T11:35:14Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
app-legacy	workspaces/lightspeed/packages/app-legacy	none	`v0.0.2`

rhdh-qodo-merge · 2026-04-17T11:44:01Z

Review Summary by Qodo

Split Lightspeed e2e tests into three focused spec files

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Split monolithic lightspeed.test.ts into three focused test suites
  - lightspeed.ui.test.ts for UI and display modes
  - lightspeed.mcp.test.ts for MCP settings and configuration
  - lightspeed.conversation.test.ts for chat flows and management
• Extracted shared bootstrap logic into lightspeedE2eSetup.ts
  - Centralized setup with guest login inlined
  - Reusable bootstrapLightspeedE2ePage() function
• Removed standalone utils/login.ts in favor of inline guest login

Diagram

flowchart LR
  A["lightspeed.test.ts<br/>874 lines"] -->|split| B["lightspeed.ui.test.ts<br/>242 lines"]
  A -->|split| C["lightspeed.mcp.test.ts<br/>336 lines"]
  A -->|split| D["lightspeed.conversation.test.ts<br/>352 lines"]
  B -->|uses| E["lightspeedE2eSetup.ts<br/>bootstrapLightspeedE2ePage"]
  C -->|uses| E
  D -->|uses| E
  F["utils/login.ts"] -->|removed| E

File Changes

1. workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts 🧪 Tests +242/-0

New UI-focused test suite

• New test suite focused on UI, display modes, and file uploads
• Tests chatbot overlay, dock-to-window, and fullscreen modes
• Covers sidebar management, default prompts, and file attachment validation
• Uses shared bootstrapLightspeedE2ePage() for setup

workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.ui.test.ts

2. workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.mcp.test.ts 🧪 Tests +336/-0

New MCP-focused test suite
• New test suite for MCP settings and server configuration
• Tests MCP panel visibility across display modes
• Covers server toggling, sorting, and token credential flows
• Tests MCP tool calling rendering in chat UI
workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.mcp.test.ts

3. workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.conversation.test.ts 🧪 Tests +352/-0

New conversation-focused test suite
• New test suite for conversation and chat management flows
• Tests bot responses, feedback submission, and message copying
• Covers chat renaming, pinning, deletion, and search functionality
• Tests conversation sorting and scroll controls
workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.conversation.test.ts

View more (3)

4. workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/lightspeedE2eSetup.ts ✨ Enhancement +77/-0

New shared e2e setup utility

• New shared bootstrap utility for Lightspeed e2e tests
• Centralizes setup logic: mocks, login, locale switching
• Exports bootstrapLightspeedE2ePage() function for test initialization
• Inlines guest login logic previously in utils/login.ts
• Exports LIGHTSPEED_E2E_DEFAULT_BOT_QUERY constant

workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/lightspeedE2eSetup.ts

5. workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts 🧪 Tests +0/-874

Removed legacy monolithic test file
• Removed monolithic test file (874 lines)
• Content distributed across three new focused test suites
• Shared setup extracted to lightspeedE2eSetup.ts
workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts

6. workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/login.ts Miscellaneous +0/-48

Removed standalone login utility
• Removed standalone login utility file
• Guest login logic inlined in lightspeedE2eSetup.ts
• Keycloak login no longer needed in new setup
workspaces/lightspeed/packages/app-legacy/e2e-tests/utils/login.ts

- Replace monolithic lightspeed.test.ts with lightspeed.ui, .mcp, and .conversation specs; add bootstrapLightspeedE2eSetup for shared mocks/login. - Move e2e-tests to workspace root; Playwright testDir e2e-tests; fix translation and fixture paths. Remove utils/login.ts; guest flow in setup helper. - package.json: test:e2e:nfs, test:e2e:all, playwright script runs both modes. - Guest post-login: assert Red Hat Catalog heading only when APP_MODE is not nfs. - expectBackstagePageVisible no-ops on nfs (no legacy catalog chrome). - Ignore e2e-tests in ESLint for lint-staged; update knip path. Made-with: Cursor

sonarqubecloud · 2026-04-17T13:39:09Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.7% Duplication on New Code

See analysis details on SonarQube Cloud

HusneShabbir requested review from a team, karthikjeeyar, rohitkrai03 and yangcao77 as code owners April 17, 2026 11:34

github-actions Bot added the workspace/lightspeed label Apr 17, 2026

HusneShabbir requested a review from jrichter1 April 17, 2026 11:35

rhdh-qodo-merge Bot added enhancement New feature or request Tests labels Apr 17, 2026

HusneShabbir force-pushed the refactor/lightspeed-e2e-split branch from 70fba4d to 071a24b Compare April 17, 2026 13:37

jrichter1 approved these changes Apr 17, 2026

View reviewed changes

jrichter1 merged commit cd78380 into redhat-developer:main Apr 17, 2026
12 checks passed

rhdh-qodo-merge Bot mentioned this pull request Apr 20, 2026

refactor(lightspeed): use lightspeed-playwright-e2e for shared e2e code #2833

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(lightspeed): split Lightspeed e2e into three spec files#2809

refactor(lightspeed): split Lightspeed e2e into three spec files#2809
jrichter1 merged 1 commit intoredhat-developer:mainfrom
HusneShabbir:refactor/lightspeed-e2e-split

HusneShabbir commented Apr 17, 2026 •

edited

Loading

Uh oh!

rhdh-qodo-merge Bot commented Apr 17, 2026 •

edited

Loading

Uh oh!

rhdh-gh-app Bot commented Apr 17, 2026

Uh oh!

rhdh-qodo-merge Bot commented Apr 17, 2026

Uh oh!

sonarqubecloud Bot commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

HusneShabbir commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

rhdh-qodo-merge Bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

rhdh-gh-app Bot commented Apr 17, 2026

Changed Packages

Uh oh!

rhdh-qodo-merge Bot commented Apr 17, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

sonarqubecloud Bot commented Apr 17, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HusneShabbir commented Apr 17, 2026 •

edited

Loading

rhdh-qodo-merge Bot commented Apr 17, 2026 •

edited

Loading