Skip to content

CONSOLE-5244: Migrate webterminal-plugin Cypress tests to Playwright#16461

Draft
fsgreco wants to merge 1 commit into
openshift:mainfrom
fsgreco:CONSOLE-5244-e2e-migration-webterminal--main
Draft

CONSOLE-5244: Migrate webterminal-plugin Cypress tests to Playwright#16461
fsgreco wants to merge 1 commit into
openshift:mainfrom
fsgreco:CONSOLE-5244-e2e-migration-webterminal--main

Conversation

@fsgreco
Copy link
Copy Markdown
Contributor

@fsgreco fsgreco commented May 19, 2026

Summary

Migrates all 15 webterminal E2E test scenarios from Cypress to Playwright, replacing legacy data-test-id selectors with modern data-test selectors (while maintaining backward compatibility).

Jira Story: CONSOLE-5244

Changes

Test Migration (13 automated + 2 manual skip)

  • Created page objects: WebTerminalPage, WebTerminalConfigPage
  • Admin tests: multi-tab management, DevWorkspace verification with UID match
  • Config tests: timeout/image settings, persist checkboxes across tab switches
  • Developer tests: terminal open/close, new tab, inactivity timeout, project creation

Note regarding id selector migration:

  • Added data-test alongside existing data-test-id in: NumberSpinner, PageHeading, NamespaceBar, ActionMenuToggle
  • Legacy data-test-id was preserved for backward compatibility to test deployed console instances (since new selectors where only present on local build)

Deleted files:

  • frontend/packages/webterminal-plugin/integration-tests/ (22 files)
  • 4 orphaned shared helpers in dev-console/integration-tests/support/
  • Cleaned up yarn.lock workspace entry

Test Mapping

Cypress Scenario Playwright test Assertions
WT-01-TC01: Open terminal with timeout (basic) WT-01-TC01: open terminal with advanced timeout 2 → 2
WT-01-TC02: Terminal in new tab (basic) WT-01-TC02: verify Open in new tab button 2 → 2
WT-01-TC03: Inactivity timeout (basic) WT-01-TC03: inactivity timeout closes terminal 3 → 3
WT-02-TC01: Multiple terminals (admin) WT-02-TC01: open and close multiple terminal tabs 2 → 2
WT-02-TC02: Timeout + DevWorkspace (admin) WT-02-TC02: start terminal with timeout and verify DevWorkspace 3 → 3
WT-02-TC03: Defaults + DevWorkspace (admin) WT-02-TC03: start terminal with defaults and verify DevWorkspace 3 → 3
WT-03-TC01: Create project + terminal (devuser) WT-03-TC01: create new project and use Web Terminal 2 → 3
WT-03-TC02: Existing project (devuser) WT-03-TC02: open Web Terminal for existing project 1 → 2
WT-02-TC01: Navigate to config (config) WT-02-TC01: navigate to Web Terminal Configuration page 1 → 1
WT-02-TC02: Change settings (config) WT-02-TC02: change timeout and image with persist checkboxes 1 → 1
WT-02-TC03: Verify persist (config) WT-02-TC03: change timeout to Hours and verify values persist 4 → 4
WT-02-TC04: No persist (config) WT-02-TC04: save without persist checkboxes 1 → 1
WT-02-TC05: Verify unchecked (config) WT-02-TC05: verify unchecked checkboxes persist 2 → 2
WT-02-TC06: YAML timeout (@Manual) test.skip
WT-02-TC07: YAML image (@Manual) test.skip
Total 27 → 29

How to Run

# Admin tests
cd frontend && npx playwright test --project=webterminal --retries=0

# Developer tests
cd frontend && npx playwright test --project=webterminal-developer --retries=0

Related


Description double checked by Claude Code

Summary by CodeRabbit

Web Terminal E2E Test Suite

  • Tests

    • Migrated Web Terminal end-to-end tests from Cypress to Playwright framework for improved reliability and performance.
    • Added test coverage for Web Terminal basic functionality, developer and administrator user workflows, and configuration customization (timeout, image, persistence settings).
    • Tests validate terminal session management, multi-tab operations, and terminal state verification.
  • Chores

    • Removed legacy Cypress-based integration test files and configuration.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 19, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented May 19, 2026

@fsgreco: This pull request references CONSOLE-5244 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates all 15 webterminal E2E test scenarios from Cypress to Playwright, replacing legacy data-test-id selectors with modern data-test selectors (while maintaining backward compatibility).

Jira Story: CONSOLE-5244

Changes

Test Migration (13 automated + 2 manual skip)

  • Created page objects: WebTerminalPage, WebTerminalConfigPage
  • Admin tests: multi-tab management, DevWorkspace verification with UID match
  • Config tests: timeout/image settings, persist checkboxes across tab switches
  • Developer tests: terminal open/close, new tab, inactivity timeout, project creation

Note regarding id selector migration:

  • Added data-test alongside existing data-test-id in: NumberSpinner, PageHeading, NamespaceBar, ActionMenuToggle
  • Legacy data-test-id was preserved for backward compatibility to test deployed console instances (since new selectors where only present on local build)

Deleted files:

  • frontend/packages/webterminal-plugin/integration-tests/ (22 files)
  • 4 orphaned shared helpers in dev-console/integration-tests/support/
  • Cleaned up yarn.lock workspace entry

Test Mapping

Cypress Scenario Playwright test Assertions
WT-01-TC01: Open terminal with timeout (basic) WT-01-TC01: open terminal with advanced timeout 2 → 2
WT-01-TC02: Terminal in new tab (basic) WT-01-TC02: verify Open in new tab button 2 → 2
WT-01-TC03: Inactivity timeout (basic) WT-01-TC03: inactivity timeout closes terminal 3 → 3
WT-02-TC01: Multiple terminals (admin) WT-02-TC01: open and close multiple terminal tabs 2 → 2
WT-02-TC02: Timeout + DevWorkspace (admin) WT-02-TC02: start terminal with timeout and verify DevWorkspace 3 → 3
WT-02-TC03: Defaults + DevWorkspace (admin) WT-02-TC03: start terminal with defaults and verify DevWorkspace 3 → 3
WT-03-TC01: Create project + terminal (devuser) WT-03-TC01: create new project and use Web Terminal 2 → 3
WT-03-TC02: Existing project (devuser) WT-03-TC02: open Web Terminal for existing project 1 → 2
WT-02-TC01: Navigate to config (config) WT-02-TC01: navigate to Web Terminal Configuration page 1 → 1
WT-02-TC02: Change settings (config) WT-02-TC02: change timeout and image with persist checkboxes 1 → 1
WT-02-TC03: Verify persist (config) WT-02-TC03: change timeout to Hours and verify values persist 4 → 4
WT-02-TC04: No persist (config) WT-02-TC04: save without persist checkboxes 1 → 1
WT-02-TC05: Verify unchecked (config) WT-02-TC05: verify unchecked checkboxes persist 2 → 2
WT-02-TC06: YAML timeout (@Manual) test.skip
WT-02-TC07: YAML image (@Manual) test.skip
Total 27 → 29

How to Run

# Admin tests
cd frontend && npx playwright test --project=webterminal --retries=0

# Developer tests
cd frontend && npx playwright test --project=webterminal-developer --retries=0

Related


Description double checked by Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci Bot requested review from jhadvig and spadgett May 19, 2026 12:28
@openshift-ci openshift-ci Bot added the component/core Related to console core functionality label May 19, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 19, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fsgreco
Once this PR has been reviewed and has the lgtm label, please assign rawagner for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added component/dev-console Related to dev-console component/shared Related to console-shared labels May 19, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

This pull request migrates the OpenShift Console web terminal e2e testing from a Cypress/Cucumber architecture to a modern Playwright-based approach. It introduces dedicated page objects (WebTerminalPage and WebTerminalConfigPage) that encapsulate terminal UI interactions and configuration workflows, along with comprehensive test specs covering basic user operations, admin and developer user scenarios, and terminal customization. Supporting test selector improvements are applied to shared UI components. The legacy Cypress integration-test infrastructure—including feature files, step definitions, test data, and related support utilities—is removed entirely.

Suggested reviewers

  • cajieh
  • vikram-raj
  • logonoff
  • TheRealJon
🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: migrating webterminal Cypress tests to Playwright with a valid Jira prefix (CONSOLE-5244).
Description check ✅ Passed The description is comprehensive, covering migration scope, test mapping, file deletions, selector updates, and run instructions. However, several template sections (Analysis/Root cause, Test setup, Browser conformance, Reviewers/assignees) remain unfilled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed This check targets Ginkgo (Go BDD framework) test name stability. The PR is entirely frontend/Playwright (TypeScript), which is a different testing framework. The check is not applicable to this PR.
Test Structure And Quality ✅ Passed The custom check requires reviewing Ginkgo (Go) test code, but the PR contains Playwright (TypeScript/JavaScript) E2E tests. The check is not applicable to this PR's test framework.
Microshift Test Compatibility ✅ Passed PR adds Playwright frontend E2E tests, not Ginkgo/Go tests. MicroShift check applies only to Ginkgo tests using It(), Describe(), Context() patterns. No such tests present.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Custom check for Ginkgo e2e tests (Go) is not applicable. PR adds only Playwright e2e tests (TypeScript/JavaScript) that don't assume multi-node cluster topology.
Topology-Aware Scheduling Compatibility ✅ Passed PR migrates E2E tests (Cypress→Playwright) and adds test selectors to UI. No deployment manifests, operators, controllers, or K8s scheduling constraints are introduced. Check not applicable.
Ote Binary Stdout Contract ✅ Passed OTE Binary Stdout Contract check not applicable. PR migrates frontend E2E tests Cypress→Playwright (TypeScript only). No Go code, test binaries, Ginkgo v2, or stdout violations.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR adds Playwright e2e tests (TypeScript), not Ginkgo tests (Go). Custom check is explicitly scoped to Ginkgo tests with Go-specific remediation. Not applicable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 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 `@frontend/e2e/pages/web-terminal-config-page.ts`:
- Around line 7-10: Replace CSS-based test selectors with Playwright test-id
queries: change the locator for incrementButton (currently using
'[data-test="Increment"], [data-test-id="Increment"]') to use
this.page.getByTestId('Increment') and ensure similar replacements for any other
locators in this file that query [data-test="..."] (including the ones around
the other occurrences you noted); keep selectToggle as
getByTestId('console-select-menu-toggle') and ensure all test-hook selectors
consistently use page.getByTestId('<name>') instead of CSS selectors.
- Around line 21-27: The code currently swallows the visibility wait and then
branches on customizeButton.count()>0 which can be >0 even if the button is
hidden; change the condition to check visibility of the first element instead.
Replace the count() check with an awaited visibility check such as await
customizeButton.first().isVisible() (or equivalent), keep the existing waitFor
but still use isVisible to decide whether to call
this.robustClick(customizeButton.first()), and fall back to the actions-menu
branch when the element is not visible.
- Line 18: The navigation uses a root-absolute path; change the call to goTo so
it is base-path aware by removing the leading slash (e.g. use
this.goTo('k8s/cluster/operator.openshift.io~v1~Console/cluster')) or, if a
helper exists in the test harness, construct the route via the codebase path
helper (e.g. use the console/base-path helper) and pass that into this.goTo in
web-terminal-config-page.ts so the goTo call (method: goTo) no longer uses a
root-absolute '/...' route.

In `@frontend/e2e/pages/web-terminal-page.ts`:
- Around line 28-29: The locators and navigation use root-absolute paths (e.g.
"a[href='/terminal']") which will break when the app is served under a proxy
subpath; update the selectors and any navigation logic that reference
"/terminal" (and the other terminal-related locators/methods around
openInNewTabLink, closeTerminalButton and the terminal open/close helpers) to
build hrefs using the app's base path helper or runtime basePath variable (e.g.,
compose `${basePath}/terminal`) or switch to path-agnostic locators (by
role/text) so they remain correct behind a proxy; locate and replace all
hardcoded "/..." usages in the web-terminal-page's locators and navigation
methods accordingly.
- Around line 11-26: Several locators still use raw CSS selectors against
data-test/data-test-id; update them to use Playwright's getByTestId pattern to
follow the e2e selector contract. Replace usages in the class fields like
addTabButton, closeTabButtons, tabsList, incrementButton, startButton, and
resourceTitle so they call page.getByTestId('...') for the element that has the
data-test/data-test-id and then chain more specific queries (getByLabel,
getByRole, locator on that test-id root) as needed instead of
'[data-test="..."]' or '[data-test-id="..."]'; apply the same refactor to the
other occurrences mentioned (around the blocks referenced in the review: the
later selectors at lines ~136-157 and ~189-193) so all selectors that target
data-test use getByTestId consistently.
- Line 59: The tests currently swallow waitFor failures on
this.loadingBox.waitFor (and the similar call at line 69), which hides UI
issues; update those calls to either perform an explicit optional-state check
(e.g., assert the element is in the expected visible/hidden state via
isVisible/isHidden) or catch the error and rethrow with context (include element
identity and timeout) so failures fail fast and produce useful diagnostics;
locate the two occurrences of this.loadingBox.waitFor and replace the empty
.catch(() => {}) with one of these approaches so the test surface doesn't
silently continue on a real UI failure.

In `@frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts`:
- Around line 6-12: TEST_NAMESPACE is a fixed value causing flakiness; modify
the test.beforeEach block to generate a unique namespace per run (e.g., append
test.info().parallelIndex, timestamp or random suffix) and use that generated
name when calling k8sClient.createNamespace and cleanup.trackNamespace instead
of the constant TEST_NAMESPACE so each test invocation creates and tracks its
own unique namespace.

In `@frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts`:
- Around line 56-64: The test currently asserts status.phase on
devWorkspaces[0], which can be the wrong resource; instead, get the selected
resource name (the UI row/title variable used when selecting the DevWorkspace),
then find the matching item in the list by comparing metadata.name and assert
that its status?.phase is 'Running'. Update both places that use
devWorkspaces[0] (the code calling listCustomResources with
DEVWORKSPACE_GROUP/DEVWORKSPACE_VERSION/NEW_PROJECT/DEVWORKSPACE_PLURAL and the
later duplicate) to locate the correct item via metadata.name before checking
status.phase.

In `@frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts`:
- Around line 89-100: The test currently grabs devWorkspaces[0].metadata.uid
which can mismatch the YAML opened for devWsName; instead, after calling
k8sClient.listCustomResources (used in this block), find the DevWorkspace whose
metadata.name === devWsName and use that object's metadata.uid for the
assertion; update both occurrences (the block with k8sClient.listCustomResources
and the later similar block) so
webTerminal.navigateToDevWorkspaceYaml(TERMINAL_NAMESPACE, devWsName) is
validated against the uid of the DevWorkspace found by name, not index 0, using
the same variable names (devWorkspaces, devWsName, uid,
webTerminal.getMonacoEditor).
- Around line 10-29: The current test.beforeEach loop enumerates and deletes all
DevWorkspaces via k8sClient.listCustomResources and deleteCustomResource (using
DEVWORKSPACE_GROUP/DEVWORKSPACE_VERSION/TERMINAL_NAMESPACE/DEVWORKSPACE_PLURAL),
which is global and unsafe; remove that global pre-clean and instead track
resources created by each test and register them with the existing cleanup
fixture (or perform a targeted afterEach that only deletes names recorded by the
test). Update tests that create DevWorkspaces to call the cleanup fixture (or
push created resource names to a per-test array) so teardown only touches those
specific resources rather than listing/deleting everything in the namespace.

In `@frontend/e2e/tests/webterminal/web-terminal-config.spec.ts`:
- Around line 29-140: The suite mutates Console Web Terminal config but never
restores it; add capture-and-rollback using the cleanup fixture: in this test
file call into WebTerminalConfigPage to snapshot current settings at setup
(e.g., add/ use a method WebTerminalConfigPage.getCurrentConfig()) and register
a restore rollback with cleanup in teardown (e.g., cleanup.add(async () => await
configPage.restoreConfig(originalConfig)) using a new
WebTerminalConfigPage.restoreConfig(config) method), then run the tests as-is;
this ensures navigateToWebTerminalConfig(), clickSaveButton(), setImageValue(),
selectTimeoutUnit(), checkPersistCheckboxes()/uncheckPersistCheckboxes(), etc.
leave the application state unchanged after each test.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 68983369-d78c-4077-887c-29ccf731e64e

📥 Commits

Reviewing files that changed from the base of the PR and between ed9a644 and 2aed4c2.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (36)
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/packages/dev-console/integration-tests/support/constants/webTerminal.ts
  • frontend/packages/dev-console/integration-tests/support/pageObjects/webterminal-po.ts
  • frontend/packages/dev-console/integration-tests/support/pages/functions/addTerminalTabs.ts
  • frontend/packages/dev-console/integration-tests/support/pages/functions/checkTerminalIcon.ts
  • frontend/packages/webterminal-plugin/integration-tests/.eslintrc
  • frontend/packages/webterminal-plugin/integration-tests/cypress.config.js
  • frontend/packages/webterminal-plugin/integration-tests/features/customization/customization-of-web-terminal.feature
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-teminal-basic.feature
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-terminal-adminuser.feature
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-terminal-devuser.feature
  • frontend/packages/webterminal-plugin/integration-tests/package.json
  • frontend/packages/webterminal-plugin/integration-tests/reporter-config.json
  • frontend/packages/webterminal-plugin/integration-tests/support/commands/hooks.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/commands/index.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/pages/functions/webTerminalConfiguration.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/common/webTerminal.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/customization/customization-of-web-terminal.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/pages/web-terminal/initTerminal-page.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/pages/web-terminal/webTerminal-page.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-Terminal-devuser.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-teminal-basic.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-terminal-adminuser.ts
  • frontend/packages/webterminal-plugin/integration-tests/testData/devworkspaceOperatorSubscription.yaml
  • frontend/packages/webterminal-plugin/integration-tests/testData/webterminalOperatorSubscription.yaml
  • frontend/packages/webterminal-plugin/integration-tests/tsconfig.json
  • frontend/playwright.config.ts
  • frontend/public/components/namespace-bar.tsx
  • frontend/public/components/utils/headings.tsx
  • frontend/public/components/utils/number-spinner.tsx
💤 Files with no reviewable changes (25)
  • frontend/packages/webterminal-plugin/integration-tests/features/customization/customization-of-web-terminal.feature
  • frontend/packages/webterminal-plugin/integration-tests/support/commands/hooks.ts
  • frontend/packages/webterminal-plugin/integration-tests/reporter-config.json
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-terminal-adminuser.feature
  • frontend/packages/webterminal-plugin/integration-tests/tsconfig.json
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/pages/web-terminal/webTerminal-page.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-teminal-basic.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/customization/customization-of-web-terminal.ts
  • frontend/packages/webterminal-plugin/integration-tests/testData/webterminalOperatorSubscription.yaml
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-terminal-devuser.feature
  • frontend/packages/dev-console/integration-tests/support/pageObjects/webterminal-po.ts
  • frontend/packages/dev-console/integration-tests/support/constants/webTerminal.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/pages/functions/webTerminalConfiguration.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/commands/index.ts
  • frontend/packages/dev-console/integration-tests/support/pages/functions/checkTerminalIcon.ts
  • frontend/packages/dev-console/integration-tests/support/pages/functions/addTerminalTabs.ts
  • frontend/packages/webterminal-plugin/integration-tests/cypress.config.js
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/common/webTerminal.ts
  • frontend/packages/webterminal-plugin/integration-tests/package.json
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/pages/web-terminal/initTerminal-page.ts
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-terminal-adminuser.ts
  • frontend/packages/webterminal-plugin/integration-tests/.eslintrc
  • frontend/packages/webterminal-plugin/integration-tests/support/step-definitions/web-terminal/web-Terminal-devuser.ts
  • frontend/packages/webterminal-plugin/integration-tests/features/web-terminal/web-teminal-basic.feature
  • frontend/packages/webterminal-plugin/integration-tests/testData/devworkspaceOperatorSubscription.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
frontend/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/**/*.{ts,tsx,js,jsx}: Never import from package index files (e.g., @console/shared) in new code, as they can create circular dependencies and slow builds. Import from specific file paths instead.
Do not use backticks in t() calls for i18n strings, as the i18n parser cannot extract keys from template literals. Use single or double quotes instead.

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import from deprecated packages or use code with the @deprecated TSdoc tag in new code.

**/*.{ts,tsx}: Use React functional components with hooks instead of class components
State Management: Use React hooks and Context API (migrating away from legacy Redux/Immutable.js)
Hooks: Use existing hooks from console-shared when possible (useK8sWatchResource, useUserSettings, etc.)
API calls: Use k8s resource hooks for data fetching, consoleFetchJSON for HTTP requests
Extensions: Use console extension points for plugin integration
Types: Check existing types in console-shared before creating new ones
Dynamic Plugins: Use console extension points for plugin integration
Styling: Use SCSS modules co-located with components, PatternFly design system components, avoid any SCSS/CSS if possible
Accessibility: Follow WCAG 2.1 AA standards, use semantic HTML, ARIA labels where needed, ensure keyboard navigation, test with screen readers
i18n: Use useTranslation('namespace') hook with key format for translation keys
Error Handling: Use ErrorBoundary components and graceful degradation patterns
Optimize re-renders: Use useCallback for memoized callbacks to avoid function recreation every render
Optimize re-renders: Use useMemo for expensive computations to avoid recalculating on every render
Lazy loading: Use React.lazy() to lazy load heavy components
TypeScript type safety: Avoid using any type; suggest proper type definitions and verify null/undefined are handled properly
Type component props properly: Reuse existing component prop types instead of duplicating type definitions
Use proper hooks: Use specialized hooks like usePluginInfo for plugin data instead of generic data fetching patterns
Avoid deprecated components: Check for JSDoc @deprecated tags, import paths containing /deprecated, and DEPRECATED_ file name prefix before using components
Importing from barrel files and circular dependencies: Import directly from specific files instead...

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
frontend/**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Never use absolute URLs or paths in the console code. The console runs behind a proxy under an arbitrary path.

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When writing code for static plugins, ensure that all $codeRef reference the corresponding extension type from the @console/dynamic-plugin-sdk package.

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (TESTING.md)

**/*.{tsx,ts}: Always use page.getByTestId('x') for Playwright selectors which queries [data-test="x"]. If a React element only has a legacy test attribute, add data-test to the element. Never remove legacy attributes
Prefer data-test attributes in Cypress selectors (e.g., cy.get('[data-test="create-deployment"]')) over brittle CSS/ARIA selectors

File Naming: PascalCase for components, kebab-case for utilities, *.spec.ts(x) for tests

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.{go,ts,tsx,js,jsx}

📄 CodeRabbit inference engine (STYLEGUIDE.md)

Use lowercase dash-separated names for all files to avoid git issues with case-insensitive file systems

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (STYLEGUIDE.md)

**/*.{ts,tsx,js,jsx}: New code MUST be written in TypeScript, not JavaScript
Prefer functional programming patterns and immutable data structures
Run the linter and follow all rules defined in .eslintrc
Never use absolute paths in code - the app should be able to run behind a proxy under an arbitrary path

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
frontend/**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (README.md)

frontend/**/*.{js,ts,tsx}: Support only the latest versions of Edge, Chrome, Safari, and Firefox browsers; IE 11 and earlier are not supported
CSP violations should be automatically reported to telemetry by parsing dynamic plugin names from securitypolicyviolation events, with throttling to prevent duplicate reports within a day

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (INTERNATIONALIZATION.md)

For dynamic translation keys that cannot be parsed by i18next-parser (t(key), t('key' + id), t(key${id})), specify possible static values in comments for the parser to extract

Files:

  • frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx
  • frontend/public/components/utils/number-spinner.tsx
  • frontend/public/components/namespace-bar.tsx
  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/public/components/utils/headings.tsx
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
**/*.ts

📄 CodeRabbit inference engine (STYLEGUIDE.md)

Plugin SDK Changes: Any updates to console-dynamic-plugin-sdk should aim to maintain backward compatibility as it's a public API - use the plugin-api-review skill to vet changes for public API impact and ensure proper documentation updates

Files:

  • frontend/playwright.config.ts
  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
frontend/e2e/tests/**/*.spec.ts

📄 CodeRabbit inference engine (TESTING.md)

frontend/e2e/tests/**/*.spec.ts: E2E tests must validate full user workflows against a real OpenShift cluster using Playwright, located in frontend/e2e/tests/<package>/
Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture
Use KubernetesClient for cluster interactions in Playwright tests - never use shell commands in tests
Import test and expect from e2e/fixtures, not from @playwright/test. Custom fixtures provide cleanup, testConfig, and k8sClient

Files:

  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (STYLEGUIDE.md)

Tests should follow a similar 'test tables' convention as used in Go where applicable

Files:

  • frontend/e2e/tests/webterminal/web-terminal-config.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts
  • frontend/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts
  • frontend/e2e/tests/webterminal/web-terminal-admin.spec.ts
frontend/e2e/pages/**/*.ts

📄 CodeRabbit inference engine (TESTING.md)

Extend BasePage in Playwright page objects which provides robustClick(), waitForLoadingComplete(), and goTo(). Locators are private readonly properties; actions are async methods

Files:

  • frontend/e2e/pages/web-terminal-config-page.ts
  • frontend/e2e/pages/web-terminal-page.ts
🔀 Multi-repo context openshift/console-operator

[::openshift/console-operator::] manifests/03-rbac-role-cluster.yaml:210 — Role rules include the string "web-terminal" (RBAC permission entry/reference). (Search for webterminal/web-terminal returned only this match.)

🔇 Additional comments (7)
frontend/packages/console-shared/src/components/actions/menu/ActionMenuToggle.tsx (1)

89-89: LGTM!

frontend/public/components/namespace-bar.tsx (2)

94-98: LGTM!


146-150: LGTM!

frontend/public/components/utils/headings.tsx (1)

177-181: LGTM!

frontend/public/components/utils/number-spinner.tsx (1)

29-31: LGTM!

frontend/playwright.config.ts (1)

30-30: LGTM!

frontend/e2e/pages/web-terminal-page.ts (1)

106-107: ⚡ Quick win

The selector at line 106 is correct as written.

Playwright's locator() method automatically constrains chained selectors to the parent locator's subtree. When you call this.tabsList.locator('> *'), the > * combinator correctly targets all direct children of the matched <ul> element. The :scope pseudo-class is unnecessary here because Playwright implicitly handles scoping in chained locators and would be redundant. This is valid and safe usage per Playwright's documented behavior.

			> Likely an incorrect or invalid review comment.

Comment on lines +7 to +10
private readonly incrementButton = this.page.locator(
'[data-test="Increment"], [data-test-id="Increment"]',
);
private readonly selectToggle = this.page.getByTestId('console-select-menu-toggle');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Prefer getByTestId over CSS selectors for test hooks.

These segments still use CSS selectors for test attributes where Playwright test-id selectors should be used for consistency and stability.

As per coding guidelines, "Always use page.getByTestId('x') for Playwright selectors which queries [data-test=\"x\"]."

Also applies to: 28-33, 40-43, 49-50

🤖 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/e2e/pages/web-terminal-config-page.ts` around lines 7 - 10, Replace
CSS-based test selectors with Playwright test-id queries: change the locator for
incrementButton (currently using '[data-test="Increment"],
[data-test-id="Increment"]') to use this.page.getByTestId('Increment') and
ensure similar replacements for any other locators in this file that query
[data-test="..."] (including the ones around the other occurrences you noted);
keep selectToggle as getByTestId('console-select-menu-toggle') and ensure all
test-hook selectors consistently use page.getByTestId('<name>') instead of CSS
selectors.

Comment thread frontend/e2e/pages/web-terminal-config-page.ts
Comment thread frontend/e2e/pages/web-terminal-config-page.ts
Comment on lines +11 to +26
private readonly addTabButton = this.page.locator(
'[data-test="multi-tab-terminal"] [aria-label="Add new tab"]',
);
private readonly closeTabButtons = this.page.locator('[aria-label="Close terminal tab"]');
private readonly tabsList = this.page.locator('[data-test="multi-tab-terminal"] ul');
private readonly drawerCloseButton = this.page.getByTestId('cloudshell-drawer-close-button');
private readonly loadingBox = this.page.getByTestId('loading-box');
private readonly timeoutLink = this.page.getByText('Timeout', { exact: true });
private readonly incrementButton = this.page.locator(
'[data-test="Increment"], [data-test-id="Increment"]',
);
private readonly timeoutInput = this.page.locator('input[aria-label="Input"]');
private readonly startButton = this.page.locator('[data-test-id="submit-button"]');
private readonly resourceTitle = this.page.locator(
'[data-test="resource-title"], [data-test-id="resource-title"]',
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Standardize Playwright selectors on getByTestId for data-test.

Several new selectors still use raw CSS against data-test/data-test-id instead of getByTestId(...). Converging these makes selectors less brittle and aligns with the e2e selector contract.

As per coding guidelines, "Always use page.getByTestId('x') for Playwright selectors which queries [data-test=\"x\"]."

Also applies to: 136-157, 189-193

🤖 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/e2e/pages/web-terminal-page.ts` around lines 11 - 26, Several
locators still use raw CSS selectors against data-test/data-test-id; update them
to use Playwright's getByTestId pattern to follow the e2e selector contract.
Replace usages in the class fields like addTabButton, closeTabButtons, tabsList,
incrementButton, startButton, and resourceTitle so they call
page.getByTestId('...') for the element that has the data-test/data-test-id and
then chain more specific queries (getByLabel, getByRole, locator on that test-id
root) as needed instead of '[data-test="..."]' or '[data-test-id="..."]'; apply
the same refactor to the other occurrences mentioned (around the blocks
referenced in the review: the later selectors at lines ~136-157 and ~189-193) so
all selectors that target data-test use getByTestId consistently.

Comment on lines +28 to +29
private readonly openInNewTabLink = this.page.locator("a[href='/terminal']");
private readonly closeTerminalButton = this.page.locator(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid root-absolute routes/selectors in page object navigation.

Line [28], Line [38], and Lines [169-175] hardcode /... paths. These can break when Console is served under a proxy subpath. Route construction here should stay base-path aware.

As per coding guidelines, "Never use absolute URLs or paths in the console code. The console runs behind a proxy under an arbitrary path."

Also applies to: 38-38, 169-175

🤖 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/e2e/pages/web-terminal-page.ts` around lines 28 - 29, The locators
and navigation use root-absolute paths (e.g. "a[href='/terminal']") which will
break when the app is served under a proxy subpath; update the selectors and any
navigation logic that reference "/terminal" (and the other terminal-related
locators/methods around openInNewTabLink, closeTerminalButton and the terminal
open/close helpers) to build hrefs using the app's base path helper or runtime
basePath variable (e.g., compose `${basePath}/terminal`) or switch to
path-agnostic locators (by role/text) so they remain correct behind a proxy;
locate and replace all hardcoded "/..." usages in the web-terminal-page's
locators and navigation methods accordingly.

Comment on lines +6 to +12
const TEST_NAMESPACE = 'aut-terminal-basic';

test.describe('Web Terminal basic user', { tag: ['@web-terminal', '@regression'] }, () => {
test.beforeEach(async ({ k8sClient, cleanup }) => {
await k8sClient.createNamespace(TEST_NAMESPACE);
cleanup.trackNamespace(TEST_NAMESPACE);
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use unique namespace names per test run.

Lines [6-12] reuse a fixed namespace across all tests. That tends to flake on retries or overlapping runs when deletion lags. Generate a per-test suffix (e.g., with test.info().parallelIndex/timestamp/random) before createNamespace.

🤖 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/e2e/tests/webterminal/developer/web-terminal-basic.spec.ts` around
lines 6 - 12, TEST_NAMESPACE is a fixed value causing flakiness; modify the
test.beforeEach block to generate a unique namespace per run (e.g., append
test.info().parallelIndex, timestamp or random suffix) and use that generated
name when calling k8sClient.createNamespace and cleanup.trackNamespace instead
of the constant TEST_NAMESPACE so each test invocation creates and tracks its
own unique namespace.

Comment on lines +56 to +64
const devWorkspaces = await k8sClient.listCustomResources(
DEVWORKSPACE_GROUP,
DEVWORKSPACE_VERSION,
NEW_PROJECT,
DEVWORKSPACE_PLURAL,
);
expect(devWorkspaces.length).toBeGreaterThan(0);
const phase = (devWorkspaces[0] as any).status?.phase;
expect(phase).toBe('Running');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Assert phase on the matched DevWorkspace, not the first list item.

Lines [56-64] and [93-101] use devWorkspaces[0], which can point to the wrong resource if multiple DevWorkspaces exist. Resolve by metadata.name (from the selected row/resource title) before asserting status.phase.

Also applies to: 93-101

🤖 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/e2e/tests/webterminal/developer/web-terminal-devuser.spec.ts` around
lines 56 - 64, The test currently asserts status.phase on devWorkspaces[0],
which can be the wrong resource; instead, get the selected resource name (the UI
row/title variable used when selecting the DevWorkspace), then find the matching
item in the list by comparing metadata.name and assert that its status?.phase is
'Running'. Update both places that use devWorkspaces[0] (the code calling
listCustomResources with
DEVWORKSPACE_GROUP/DEVWORKSPACE_VERSION/NEW_PROJECT/DEVWORKSPACE_PLURAL and the
later duplicate) to locate the correct item via metadata.name before checking
status.phase.

Comment on lines +10 to +29
test.beforeEach(async ({ k8sClient }) => {
const devWorkspaces = await k8sClient.listCustomResources(
DEVWORKSPACE_GROUP,
DEVWORKSPACE_VERSION,
TERMINAL_NAMESPACE,
DEVWORKSPACE_PLURAL,
);
for (const dw of devWorkspaces) {
const name = (dw as any).metadata?.name;
if (name) {
await k8sClient.deleteCustomResource(
DEVWORKSPACE_GROUP,
DEVWORKSPACE_VERSION,
TERMINAL_NAMESPACE,
DEVWORKSPACE_PLURAL,
name,
);
}
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Replace global pre-clean with per-test cleanup tracking.

Lines [10-29] delete all DevWorkspaces in a shared namespace on every test start. That is risky for concurrent suites and still doesn’t guarantee post-test cleanup. Prefer per-test resource tracking/teardown via fixtures (or targeted afterEach cleanup for created resources only).

As per coding guidelines, "Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture."

🤖 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/e2e/tests/webterminal/web-terminal-admin.spec.ts` around lines 10 -
29, The current test.beforeEach loop enumerates and deletes all DevWorkspaces
via k8sClient.listCustomResources and deleteCustomResource (using
DEVWORKSPACE_GROUP/DEVWORKSPACE_VERSION/TERMINAL_NAMESPACE/DEVWORKSPACE_PLURAL),
which is global and unsafe; remove that global pre-clean and instead track
resources created by each test and register them with the existing cleanup
fixture (or perform a targeted afterEach that only deletes names recorded by the
test). Update tests that create DevWorkspaces to call the cleanup fixture (or
push created resource names to a per-test array) so teardown only touches those
specific resources rather than listing/deleting everything in the namespace.

Comment on lines +89 to +100
const devWorkspaces = await k8sClient.listCustomResources(
DEVWORKSPACE_GROUP,
DEVWORKSPACE_VERSION,
TERMINAL_NAMESPACE,
DEVWORKSPACE_PLURAL,
);
expect(devWorkspaces.length).toBeGreaterThan(0);
const uid = (devWorkspaces[0] as any).metadata?.uid;
expect(uid).toBeTruthy();

await webTerminal.navigateToDevWorkspaceYaml(TERMINAL_NAMESPACE, devWsName);
await expect(webTerminal.getMonacoEditor()).toContainText(uid, { timeout: 30_000 });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Correlate UID checks with the opened DevWorkspace name.

Lines [89-100] and [134-145] fetch all DevWorkspaces, then read devWorkspaces[0].metadata.uid. If list ordering differs, UID assertion can mismatch the YAML page opened by devWsName. Select the object by metadata.name === devWsName before asserting UID.

Also applies to: 134-145

🤖 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/e2e/tests/webterminal/web-terminal-admin.spec.ts` around lines 89 -
100, The test currently grabs devWorkspaces[0].metadata.uid which can mismatch
the YAML opened for devWsName; instead, after calling
k8sClient.listCustomResources (used in this block), find the DevWorkspace whose
metadata.name === devWsName and use that object's metadata.uid for the
assertion; update both occurrences (the block with k8sClient.listCustomResources
and the later similar block) so
webTerminal.navigateToDevWorkspaceYaml(TERMINAL_NAMESPACE, devWsName) is
validated against the uid of the DevWorkspace found by name, not index 0, using
the same variable names (devWorkspaces, devWsName, uid,
webTerminal.getMonacoEditor).

Comment on lines +29 to +140
test(
'WT-02-TC02: change timeout and image with persist checkboxes',
{ tag: ['@smoke'] },
async ({ page }) => {
const configPage = new WebTerminalConfigPage(page);

await test.step('Navigate to Web Terminal Configuration', async () => {
await configPage.navigateToWebTerminalConfig();
});

await test.step('Set timeout to Minutes and enter image', async () => {
await configPage.incrementTimeout();
await configPage.selectTimeoutUnit('Minutes');
await configPage.setImageValue(TEST_IMAGE_805);
});

await test.step('Check persist checkboxes and save', async () => {
await configPage.checkPersistCheckboxes();
await configPage.clickSaveButton();
});

await test.step('Verify success alert', async () => {
await expect(configPage.getSuccessAlert()).toBeVisible();
});
},
);

test(
'WT-02-TC03: change timeout to Hours and verify values persist after tab switch',
{ tag: ['@smoke'] },
async ({ page }) => {
const configPage = new WebTerminalConfigPage(page);

await test.step('Navigate to Web Terminal Configuration', async () => {
await configPage.navigateToWebTerminalConfig();
});

await test.step('Set timeout to Hours, enter image, check persist, and save', async () => {
await configPage.incrementTimeout();
await configPage.selectTimeoutUnit('Hours');
await configPage.setImageValue(TEST_IMAGE_806);
await configPage.checkPersistCheckboxes();
await configPage.clickSaveButton();
});

await test.step('Switch to Developer tab and back to Web Terminal', async () => {
await configPage.clickDeveloperTab();
await configPage.clickWebTerminalTab();
});

await test.step('Verify saved values persist', async () => {
await expect(configPage.getImageInput()).toHaveValue(TEST_IMAGE_806);
await expect(configPage.getSelectToggle()).toContainText('Hours');
await expect(configPage.getTimeoutCheckbox()).toBeChecked();
await expect(configPage.getImageCheckbox()).toBeChecked();
});
},
);

test(
'WT-02-TC04: save without persist checkboxes',
{ tag: ['@regression'] },
async ({ page }) => {
const configPage = new WebTerminalConfigPage(page);

await test.step('Navigate to Web Terminal Configuration', async () => {
await configPage.navigateToWebTerminalConfig();
});

await test.step('Set timeout, image, uncheck persist, and save', async () => {
await configPage.incrementTimeout();
await configPage.selectTimeoutUnit('Hours');
await configPage.setImageValue(TEST_IMAGE_806);
await configPage.uncheckPersistCheckboxes();
await configPage.clickSaveButton();
});

await test.step('Verify success alert', async () => {
await expect(configPage.getSuccessAlert()).toBeVisible();
});
},
);

test(
'WT-02-TC05: verify unchecked checkboxes persist after tab switch',
{ tag: ['@regression'] },
async ({ page }) => {
const configPage = new WebTerminalConfigPage(page);

await test.step('Navigate to Web Terminal Configuration', async () => {
await configPage.navigateToWebTerminalConfig();
});

await test.step('Set values, uncheck persist, and save', async () => {
await configPage.incrementTimeout();
await configPage.selectTimeoutUnit('Hours');
await configPage.setImageValue(TEST_IMAGE_806);
await configPage.uncheckPersistCheckboxes();
await configPage.clickSaveButton();
});

await test.step('Switch to Developer tab and back to Web Terminal', async () => {
await configPage.clickDeveloperTab();
await configPage.clickWebTerminalTab();
});

await test.step('Verify checkboxes are unchecked', async () => {
await expect(configPage.getTimeoutCheckbox()).not.toBeChecked();
await expect(configPage.getImageCheckbox()).not.toBeChecked();
});
},
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add rollback for cluster config mutations in this suite.

These tests persist changes to Console Web Terminal config but never restore the original values. That creates order-dependent failures and leaks state into other suites. Capture baseline config in setup and restore it in teardown.

As per coding guidelines, "Each Playwright E2E test block must create its own resources, assert independently, and clean up via the cleanup fixture."

🤖 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/e2e/tests/webterminal/web-terminal-config.spec.ts` around lines 29 -
140, The suite mutates Console Web Terminal config but never restores it; add
capture-and-rollback using the cleanup fixture: in this test file call into
WebTerminalConfigPage to snapshot current settings at setup (e.g., add/ use a
method WebTerminalConfigPage.getCurrentConfig()) and register a restore rollback
with cleanup in teardown (e.g., cleanup.add(async () => await
configPage.restoreConfig(originalConfig)) using a new
WebTerminalConfigPage.restoreConfig(config) method), then run the tests as-is;
this ensures navigateToWebTerminalConfig(), clickSaveButton(), setImageValue(),
selectTimeoutUnit(), checkPersistCheckboxes()/uncheckPersistCheckboxes(), etc.
leave the application state unchanged after each test.

@fsgreco fsgreco marked this pull request as draft May 19, 2026 14:58
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 19, 2026
Migrates all 15 webterminal E2E test scenarios (13 automated + 2 manual
skip) from Cypress/Cucumber to Playwright, following Console's layered
architecture.

Tests migrated:
- 3 admin tests (multi-tab, DevWorkspace verification with UID match)
- 5 config tests (timeout/image settings, persist checkboxes)
- 3 basic user tests (terminal open/close, new tab, inactivity timeout)
- 2 developer user tests (create/select project, DevWorkspace status)
- 2 manual scenarios preserved as test.skip

Selector migration:
- Added data-test alongside data-test-id in NumberSpinner, PageHeading,
  NamespaceBar, and ActionMenuToggle for Playwright getByTestId() usage
- Legacy data-test-id attributes preserved for backward compatibility

Note to @BabbarPB08: the webterminal Cypress tests from openshift#16269 should
now use the Playwright infrastructure under e2e/tests/webterminal/.
See e2e/pages/web-terminal-page.ts for the shared page object.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fsgreco fsgreco force-pushed the CONSOLE-5244-e2e-migration-webterminal--main branch from 2aed4c2 to 99e5669 Compare May 19, 2026 15:52
@fsgreco
Copy link
Copy Markdown
Contributor Author

fsgreco commented May 19, 2026

/retest

@fsgreco
Copy link
Copy Markdown
Contributor Author

fsgreco commented May 19, 2026

/retest
error not related to frontend changes.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 19, 2026

@fsgreco: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-console 99e5669 link true /test e2e-gcp-console
ci/prow/backend 99e5669 link true /test backend
ci/prow/e2e-playwright 99e5669 link false /test e2e-playwright

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/core Related to console core functionality component/dev-console Related to dev-console component/shared Related to console-shared do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants