Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ cypress-a11y-report.json
/dynamic-demo-plugin/**/dist
**/.claude/settings.local.json
**/chartstore-*/
.playwright-mcp/
4 changes: 4 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ These files are the single source of truth for architecture, coding standards, a
- [CONTRIBUTING.md](CONTRIBUTING.md) - contribution workflow and commit message conventions.
- [README.md](README.md) - project setup, build instructions, and architecture overview.

## Playwright migration

We are migrating Cypress e2e tests to Playwright. Use `/migrate-cypress` to convert test files and `/debug-test` to fix failing tests. Shared migration context (translation tables, structural rules, checklist) is in `.claude/migration-context.md`.

### Dynamic plugin SDK

- [Dynamic Plugin SDK documentation](frontend/packages/console-dynamic-plugin-sdk/README.md) - architecture, design principles, and development guidelines. Consult before modifying SDK code.
Expand Down
1 change: 1 addition & 0 deletions frontend/.eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ Godeps
dynamic-demo-plugin
.eslintrc.js
tsconfig.json
e2e/.eslintrc.json
e2e/tsconfig.json
5 changes: 5 additions & 0 deletions frontend/e2e/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"testing-library/prefer-screen-queries": "off"
}
}
67 changes: 67 additions & 0 deletions frontend/e2e/pages/details-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { expect } from '@playwright/test';
import type { Locator } from '@playwright/test';

import BasePage from './base-page';

export class DetailsPage extends BasePage {
private readonly pageHeading = this.page.locator('[data-test="page-heading"]');
private readonly resourceTitle = this.page.locator('[data-test-id="resource-title"]');
private readonly skeletonView = this.page.getByTestId('skeleton-detail-view');
private readonly actionsMenuButton = this.page.locator('[data-test-id="actions-menu-button"]');
readonly breadcrumbLink0 = this.page.locator('[data-test-id="breadcrumb-link-0"]');
readonly statusPopoverButton = this.page.getByTestId('popover-status-button');
readonly enableAutoscaleButton = this.page.getByTestId('enable-autoscale');
readonly xtermViewport = this.page.locator('.xterm-viewport');
readonly resourcesSuccessMessage = this.page.getByTestId('resources-successfully-created');
readonly eventTotals = this.page.getByTestId('event-totals');
admissionWarning(testId: string): Locator {
return this.page.getByTestId(testId);
}

debugContainerLink(containerName?: string): Locator {
const testId = containerName
? `popup-debug-container-link-${containerName}`
: 'debug-container-link';
return this.page.getByTestId(testId);
}

async titleShouldContain(title: string): Promise<void> {
await this.pageHeading.waitFor({ state: 'visible', timeout: 30_000 });
await expect(this.pageHeading).toContainText(title, { timeout: 30_000 });
}

async sectionHeaderShouldExist(sectionHeading: string): Promise<void> {
await expect(
this.page.locator(`[data-test-section-heading="${sectionHeading}"]`),
).toBeVisible();
}

async isLoaded(): Promise<void> {
await expect(this.skeletonView).toBeHidden({ timeout: 30_000 });
await this.resourceTitle.waitFor({ state: 'visible', timeout: 30_000 });
await expect(this.resourceTitle).not.toBeEmpty();
}

async selectTab(name: string): Promise<void> {
const tab = this.page.locator(`[data-test-id="horizontal-link-${name}"]`);
await this.robustClick(tab);
}

async clickPageActionFromDropdown(actionID: string): Promise<void> {
await this.robustClick(this.actionsMenuButton);
const action = this.page.locator(`[data-test-action="${actionID}"]:not([disabled])`);
await this.robustClick(action);
}

async clickBreadcrumb(): Promise<void> {
await this.robustClick(this.breadcrumbLink0);
}

async clickStatusPopover(): Promise<void> {
await this.robustClick(this.statusPopoverButton, { timeout: 60_000 });
}

async clickDebugContainerLink(containerName?: string): Promise<void> {
await this.robustClick(this.debugContainerLink(containerName));
}
}
161 changes: 161 additions & 0 deletions frontend/e2e/pages/list-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import { expect } from '@playwright/test';
import type { Locator } from '@playwright/test';

import BasePage from './base-page';

export class ListPage extends BasePage {
private readonly heading = this.page.locator('[data-test="page-heading"] h1');

async titleShouldHaveText(title: string): Promise<void> {
await expect(this.heading).toContainText(title);
}

// --- Resource row helpers (older VirtualizedTable) ---

async rowsShouldExist(resourceName: string): Promise<void> {
await expect(
this.page.locator('[data-test-rows="resource-row"]').filter({ hasText: resourceName }),
).toBeVisible({ timeout: 60_000 });
}

async rowsShouldNotExist(resourceName: string): Promise<void> {
await expect(this.page.locator(`[data-test-id="${resourceName}"]`)).toBeHidden({
timeout: 90_000,
});
}
Comment on lines +15 to +25
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

Inconsistent selector strategy between existence checks.

rowsShouldExist filters by data-test-rows="resource-row" + text match, while rowsShouldNotExist uses data-test-id="${resourceName}". This mismatch means you're checking different DOM elements for presence vs. absence, which could produce false positives if the row exists but the data-test-id attribute is missing or named differently.

Align both methods to use the same locator strategy. If data-test-id is more reliable, use it for both; otherwise use the data-test-rows + text filter for both.

🔧 Proposed fix to align selector strategies
 async rowsShouldExist(resourceName: string): Promise<void> {
   await expect(
-    this.page.locator('[data-test-rows="resource-row"]').filter({ hasText: resourceName }),
+    this.page.locator(`[data-test-id="${resourceName}"]`),
   ).toBeVisible({ timeout: 60_000 });
 }
📝 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.

Suggested change
async rowsShouldExist(resourceName: string): Promise<void> {
await expect(
this.page.locator('[data-test-rows="resource-row"]').filter({ hasText: resourceName }),
).toBeVisible({ timeout: 60_000 });
}
async rowsShouldNotExist(resourceName: string): Promise<void> {
await expect(this.page.locator(`[data-test-id="${resourceName}"]`)).toBeHidden({
timeout: 90_000,
});
}
async rowsShouldExist(resourceName: string): Promise<void> {
await expect(
this.page.locator(`[data-test-id="${resourceName}"]`),
).toBeVisible({ timeout: 60_000 });
}
async rowsShouldNotExist(resourceName: string): Promise<void> {
await expect(this.page.locator(`[data-test-id="${resourceName}"]`)).toBeHidden({
timeout: 90_000,
});
}
🤖 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/list-page.ts` around lines 15 - 25, The two helpers
rowsShouldExist and rowsShouldNotExist use different selectors which can cause
false results; update rowsShouldNotExist to use the same locator strategy as
rowsShouldExist (i.e.,
this.page.locator('[data-test-rows="resource-row"]').filter({ hasText:
resourceName })) and assert it is hidden/not visible with the same timeout, or
alternatively change both to use data-test-id consistently—pick one strategy and
make both functions use that same locator (refer to rowsShouldExist and
rowsShouldNotExist to locate where to change).


async rowsClickKebabAction(resourceName: string, actionName: string): Promise<void> {
const row = this.page
.locator('[data-test-rows="resource-row"]')
.filter({ hasText: resourceName });
const kebab = row.locator('[data-test-id="kebab-button"]');
await this.robustClick(kebab);
const action = this.page.locator(`[data-test-action="${actionName}"]:not([disabled])`);
await this.robustClick(action);
}

async rowsClickStatusButton(resourceName: string): Promise<void> {
const row = this.page
.locator('[data-test-rows="resource-row"]')
.filter({ hasText: resourceName });
const statusButton = row.getByTestId('popover-status-button');
await this.robustClick(statusButton, { timeout: 60_000 });
}

async filterByStatus(status: string): Promise<void> {
const filterToggle = this.page.locator('[data-ouia-component-id="DataViewCheckboxFilter"]');
if (await filterToggle.isVisible().catch(() => false)) {
await this.robustClick(filterToggle);
const filterItem = this.page.locator(
`[data-ouia-component-id="DataViewCheckboxFilter-filter-item-${status}"]`,
);
await this.robustClick(filterItem);
await this.robustClick(filterToggle);
} else {
const filterDropdownToggle = this.page.locator(
'[data-test-id="filter-dropdown-toggle"] button',
);
if (await filterDropdownToggle.isVisible().catch(() => false)) {
await this.robustClick(filterDropdownToggle);
await this.page.locator(`#${status}`).click();
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 | 🟡 Minor | ⚡ Quick win

Unvalidated CSS ID selector construction.

Constructing a CSS ID selector with #${status} assumes status is a valid CSS identifier. If status contains special characters (spaces, colons, etc.), this selector will fail silently.

🛡️ Proposed fix using attribute selector
-        await this.page.locator(`#${status}`).click();
+        await this.page.locator(`[id="${status}"]`).click();
📝 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.

Suggested change
await this.page.locator(`#${status}`).click();
await this.page.locator(`[id="${status}"]`).click();
🤖 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/list-page.ts` at line 60, The selector built with
`#${status}` assumes `status` is a valid CSS identifier and will break for
values with spaces or special chars; replace it with an attribute selector that
matches the id value (e.g., use this.page.locator(`[id="${status}"]`).click())
or otherwise escape `status` before interpolation; update the occurrence of
`this.page.locator(`#${status}`).click();` to use the safe attribute form (or a
proper escaping helper) so `status` values containing special characters are
handled correctly.

await this.robustClick(filterDropdownToggle);
}
}
}

// --- DataView row helpers (ConsoleDataView) ---
// These use generic table locators that work even if data-test attributes
// are not forwarded to the DOM by PatternFly DataView components.

private dvCell(resourceName: string, cellName = 'name'): Locator {
return this.page.locator(`[data-test="data-view-cell-${resourceName}-${cellName}"]`);
}

private dvRow(resourceName: string): Locator {
return this.page.locator('table tbody tr').filter({
has: this.page.getByRole('link', { name: resourceName, exact: true }),
});
}

async dvRowsShouldBeLoaded(): Promise<void> {
await expect(this.page.getByTestId('data-view-table')).toBeVisible({ timeout: 60_000 });
}

private async resolveRow(resourceName: string): Promise<Locator> {
const cell = this.dvCell(resourceName);
if (await cell.isVisible({ timeout: 5_000 }).catch(() => false)) {
return cell.locator('xpath=ancestor::tr');
}
return this.dvRow(resourceName);
}

async dvRowsShouldExist(resourceName: string, cellName = 'name'): Promise<void> {
const cell = this.dvCell(resourceName, cellName);
const row = this.dvRow(resourceName);
try {
await expect(cell).toBeVisible({ timeout: 30_000 });
} catch {
await this.page.reload({ waitUntil: 'domcontentloaded' });
try {
await expect(cell).toBeVisible({ timeout: 30_000 });
} catch {
await expect(row).toBeVisible({ timeout: 30_000 });
}
}
}
Comment on lines +92 to +105
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

Excessive retry logic with page reload masks underlying issues.

Triple-nested try-catch with a full page reload is a strong signal that the underlying selectors or timing assumptions are fragile. This pattern will make test failures non-deterministic and harder to diagnose.

Consider:

  1. Root-cause why data-test attributes are sometimes missing and fix at the source (ensure PatternFly DataView forwards them).
  2. Use explicit waitFor with a single fallback rather than nested try-catch.
  3. If reload is truly necessary, add a comment explaining why and under what conditions it occurs.
🤖 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/list-page.ts` around lines 92 - 105, The dvRowsShouldExist
method contains fragile triple-nested try/catch with a full page.reload that
masks selector/timing bugs; replace the nested retries with a single explicit
wait pattern (use a single expect/waitFor with a clear timeout and one fallback)
and remove the automatic full-page reload unless you document why it’s required;
investigate and fix the root cause for missing data-test attributes (ensure
PatternFly DataView forwards them) rather than retrying, and when keeping a
reload keep a short comment in dvRowsShouldExist referencing dvCell and dvRow
explaining the exact condition that warrants reload and the observable you
expect after reload.


async dvRowsShouldNotExist(resourceName: string): Promise<void> {
const cell = this.dvCell(resourceName);
await expect(cell).toBeHidden({ timeout: 90_000 });
}

async dvRowsCountShouldBe(count: number): Promise<void> {
await expect(this.page.locator('table tbody tr')).toHaveCount(count, { timeout: 60_000 });
}

async dvRowsClickKebabAction(resourceName: string, actionName: string): Promise<void> {
const row = await this.resolveRow(resourceName);
const kebab = row.locator('[data-test-id="kebab-button"]');
await this.robustClick(kebab);
const action = this.page.locator(`[data-test-action="${actionName}"]:not([disabled])`);
await this.robustClick(action);
}

async dvRowsClickStatusButton(resourceName: string): Promise<void> {
const row = await this.resolveRow(resourceName);
const statusButton = row.getByTestId('popover-status-button');
await this.robustClick(statusButton, { timeout: 60_000 });
}

async dvFilterByName(name: string): Promise<void> {
const filters = this.page.locator('[data-ouia-component-id="DataViewFilters"]');
await this.robustClick(filters.locator('.pf-v6-c-menu-toggle').first());
await this.robustClick(
this.page.locator('.pf-v6-c-menu__list-item').filter({ hasText: 'Name' }),
);
const input = this.page.locator('[aria-label="Filter by name"]');
await input.clear();
await input.fill(name);
}

async dvFilterBy(filterName: string, checkboxLabel: string): Promise<void> {
await this.dvRowsShouldBeLoaded();
const filters = this.page.locator('[data-ouia-component-id="DataViewFilters"]');
await this.robustClick(filters.locator('.pf-v6-c-menu-toggle').first());
await this.robustClick(
this.page.locator('.pf-v6-c-menu__list-item').filter({ hasText: filterName }),
);
await this.robustClick(this.page.locator('[data-ouia-component-id="DataViewCheckboxFilter"]'));
const filterItem = this.page.locator(
`[data-ouia-component-id="DataViewCheckboxFilter-filter-item-${checkboxLabel}"]`,
);
await expect(filterItem).toBeVisible();
await this.robustClick(filterItem);
await expect(this.page).toHaveURL(new RegExp(`=${checkboxLabel}`), { timeout: 10_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 | 🔴 Critical | ⚡ Quick win

Regular expression constructed from user input enables ReDoS.

Constructing new RegExp(\=${checkboxLabel}`)without escapingcheckboxLabel` allows an attacker-controlled string (or a malicious test fixture) to inject a catastrophic backtracking pattern. While this is test code, malicious fixtures or compromised dependencies could exploit this to hang the test suite.

Escape the input or use a static pattern with string matching.

🔒 Proposed fix to escape regex input
+  // Helper to escape regex special characters
+  private escapeRegex(str: string): string {
+    return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  }

   async dvFilterBy(filterName: string, checkboxLabel: string): Promise<void> {
     // ... existing code ...
-    await expect(this.page).toHaveURL(new RegExp(`=${checkboxLabel}`), { timeout: 10_000 });
+    await expect(this.page).toHaveURL(new RegExp(`=${this.escapeRegex(checkboxLabel)}`), { timeout: 10_000 });
     await this.robustClick(this.page.locator('[data-ouia-component-id="DataViewCheckboxFilter"]'));
   }
📝 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.

Suggested change
await expect(this.page).toHaveURL(new RegExp(`=${checkboxLabel}`), { timeout: 10_000 });
private escapeRegex(str: string): string {
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
async dvFilterBy(filterName: string, checkboxLabel: string): Promise<void> {
// ... existing code ...
await expect(this.page).toHaveURL(new RegExp(`=${this.escapeRegex(checkboxLabel)}`), { timeout: 10_000 });
await this.robustClick(this.page.locator('[data-ouia-component-id="DataViewCheckboxFilter"]'));
}
🤖 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/list-page.ts` at line 154, The test constructs a RegExp
from unchecked user input (`checkboxLabel`) which enables ReDoS; update the
assertion in the method using this.page and toHaveURL so the input is escaped or
avoided: either call toHaveURL with a plain string/substring match (e.g., assert
the URL contains `=${checkboxLabel}`) or sanitize the value by passing
`checkboxLabel` through a safe escape routine (e.g., escapeRegExp) before
building new RegExp(`=${escapedLabel}`); ensure the escape helper is used
wherever `checkboxLabel` is interpolated into a RegExp.

await this.robustClick(this.page.locator('[data-ouia-component-id="DataViewCheckboxFilter"]'));
}

get clickCreateYAMLButton(): Locator {
return this.page.getByTestId('item-create');
}
Comment on lines +158 to +160
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 | ⚡ Quick win

Misleading getter name implies action but returns locator.

The getter is named clickCreateYAMLButton but returns a Locator rather than performing a click. This violates the principle of least surprise—callers expect click* methods to execute the action.

Rename to createYAMLButton or getCreateYAMLButton, or convert to an async method that performs the click.

♻️ Proposed rename
-  get clickCreateYAMLButton(): Locator {
+  get createYAMLButton(): Locator {
     return this.page.getByTestId('item-create');
   }
📝 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.

Suggested change
get clickCreateYAMLButton(): Locator {
return this.page.getByTestId('item-create');
}
get createYAMLButton(): Locator {
return this.page.getByTestId('item-create');
}
🤖 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/list-page.ts` around lines 158 - 160, The getter
clickCreateYAMLButton is misleading because it returns a Locator instead of
performing a click; either rename it to createYAMLButton or getCreateYAMLButton,
or change it into an async method that performs the click (e.g., async
clickCreateYAMLButton() { await this.page.getByTestId('item-create').click();
}). Update all call sites to match the chosen change and keep the unique symbol
clickCreateYAMLButton (if converting) or use the new getter name where
referenced.

}
44 changes: 44 additions & 0 deletions frontend/e2e/pages/login-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import BasePage from './base-page';

export class LoginPage extends BasePage {
private readonly loginButton = this.page.locator('[data-test-id="login"]');
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 | 🔴 Critical | ⚡ Quick win

Test-id attribute mismatch will break selector.

Line 4 uses data-test-id="login" but global.setup.ts:9 configured Playwright to use data-test attribute for all getByTestId calls. This inconsistency means:

  • Line 8's getByTestId('user-dropdown-toggle') looks for data-test="user-dropdown-toggle"
  • Line 4's manual locator looks for data-test-id="login"

If the HTML uses only data-test attributes (standard after the global setup), this selector will fail.

🔧 Proposed fix
-  private readonly loginButton = this.page.locator('[data-test-id="login"]');
+  private readonly loginButton = this.page.getByTestId('login');

This ensures consistent use of the configured test-id attribute.

📝 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.

Suggested change
private readonly loginButton = this.page.locator('[data-test-id="login"]');
private readonly loginButton = this.page.getByTestId('login');
🤖 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/login-page.ts` at line 4, The locator for loginButton uses
the wrong attribute (data-test-id) causing it to miss selectors configured to
use data-test; update the loginButton definition (symbol: loginButton) to use
the same test-id strategy as the rest of the tests — either switch the manual
locator to target data-test="login" or replace it with page.getByTestId('login')
so it aligns with global setup and the getByTestId('user-dropdown-toggle')
usage.

private readonly usernameInput = this.page.locator('#inputUsername');
private readonly passwordInput = this.page.locator('#inputPassword');
private readonly submitButton = this.page.locator('button[type="submit"]');
private readonly userDropdownToggle = this.page.getByTestId('user-dropdown-toggle');

providerButton(provider: string) {
return this.page.getByText(provider, { exact: true });
}

async loginAs(provider: string, username: string, password: string): Promise<boolean> {
const baseURL = process.env.WEB_CONSOLE_URL || 'http://localhost:9000';
await this.page.goto(baseURL, { timeout: 90_000, waitUntil: 'domcontentloaded' });

const authDisabled = await this.page
.evaluate(() => (window as any).SERVER_FLAGS?.authDisabled)
.catch(() => false);

if (authDisabled) {
return false;
}

const providerBtn = this.providerButton(provider);
await this.loginButton
.or(this.usernameInput)
.or(providerBtn)
.first()
.waitFor({ state: 'visible', timeout: 30_000 });

if ((await providerBtn.count()) > 0 && (await providerBtn.isVisible())) {
await providerBtn.click();
await this.usernameInput.waitFor({ state: 'visible', timeout: 30_000 });
}

await this.usernameInput.fill(username);
await this.passwordInput.fill(password);
await this.submitButton.click();
await this.userDropdownToggle.waitFor({ state: 'visible', timeout: 60_000 });
return true;
}
}
30 changes: 30 additions & 0 deletions frontend/e2e/pages/machine-config-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { expect } from '@playwright/test';
import type { Locator } from '@playwright/test';

import BasePage from './base-page';

export class MachineConfigPage extends BasePage {
readonly configFilePath = this.page.getByTestId('config-file-path-0');
readonly copyToClipboard = this.page.locator('.co-copy-to-clipboard__text');

sectionHeading(heading: string): Locator {
return this.page.locator(`[data-test-section-heading="${heading}"]`);
}

errorHeading(text: string): Locator {
return this.page.getByText(text);
}

async checkConfigFileDetails(mode: number, overwrite: boolean, content: string): Promise<void> {
await this.configFilePath.scrollIntoViewIfNeeded();
await this.page.locator('button[aria-label="Info"]').first().click();
const descriptionList = this.page.locator('[class*="description-list"]');
await expect(descriptionList.getByText(String(mode), { exact: true })).toBeVisible();
await expect(descriptionList.getByText(String(overwrite), { exact: true })).toBeVisible();
const decoded = decodeURIComponent(content)
.replace(/^(data:,)/, '')
.slice(0, 30);
const codeBlock = this.page.locator('code').first();
await expect(codeBlock).toContainText(decoded);
}
Comment on lines +18 to +29
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

Multiple selector fragilities and magic numbers.

  • Line 20: .first() assumes the Info button context but could match any "Info" button on the page.
  • Line 21: [class*="description-list"] couples to CSS implementation and breaks during refactoring.
  • Lines 22-23: String(overwrite) converts boolean to "true"/"false", but the UI might render "Yes"/"No" or localized text—this assertion will fail silently or incorrectly.
  • Line 26: .slice(0, 30) is a magic number with no explanation. Why 30 characters?
  • Line 27: locator('code').first() could match any code block on the page, not necessarily the config content.
🛡️ Recommended improvements
  async checkConfigFileDetails(mode: number, overwrite: boolean, content: string): Promise<void> {
    await this.configFilePath.scrollIntoViewIfNeeded();
-    await this.page.locator('button[aria-label="Info"]').first().click();
+    // Scope to the config file section to avoid ambiguity
+    await this.configFilePath.locator('..').getByRole('button', { name: 'Info' }).click();
-    const descriptionList = this.page.locator('[class*="description-list"]');
+    const descriptionList = this.page.getByTestId('config-file-description-list');
-    await expect(descriptionList.getByText(String(mode), { exact: true })).toBeVisible();
-    await expect(descriptionList.getByText(String(overwrite), { exact: true })).toBeVisible();
+    await expect(descriptionList.getByTestId('config-file-mode')).toHaveText(String(mode));
+    await expect(descriptionList.getByTestId('config-file-overwrite')).toHaveText(String(overwrite));
    const decoded = decodeURIComponent(content)
      .replace(/^(data:,)/, '')
-      .slice(0, 30);
-    const codeBlock = this.page.locator('code').first();
+      .slice(0, 30); // Truncate to avoid full-content comparison; adjust if needed
+    const codeBlock = this.configFilePath.locator('..').locator('code').first();
    await expect(codeBlock).toContainText(decoded);
  }

Note: Requires adding data-test attributes to the UI components.

🤖 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/machine-config-page.ts` around lines 18 - 29, The
checkConfigFileDetails method uses fragile selectors and magic values; update it
to click the Info button and scope subsequent lookups to the same info panel
(avoid .first()) by targeting dedicated test attributes (e.g.,
data-test="config-info-btn" for the button and data-test="config-info-panel" for
the panel) so locators like description list and code block are looked up within
that panel; replace the brittle class selector '[class*="description-list"]'
with a data-test attribute (e.g., data-test="config-description-list") and find
displayed values for mode and overwrite using a tolerant matcher that accepts
expected renderings for booleans (e.g., map overwrite to
["true","false","Yes","No"] or allow substring/regex matches) instead of
String(overwrite); eliminate the magic .slice(0, 30) by either deriving the
expected preview length from a named constant or by asserting that the decoded
content startsWith the expected snippet (or parameterize the expected preview
length) and scope the code block lookup to the info panel (e.g.,
data-test="config-code") so locator('code').first() is no longer used.

}
13 changes: 13 additions & 0 deletions frontend/e2e/pages/masthead-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { expect } from '@playwright/test';

import BasePage from './base-page';

export class MastheadPage extends BasePage {
readonly loadingIndicator = this.page.getByTestId('loading-indicator');
readonly globalNotifications = this.page.getByTestId('global-notifications');

async usernameShouldHaveText(text: string): Promise<void> {
const toggle = this.page.getByTestId('user-dropdown-toggle');
await expect(toggle).toHaveText(text);
}
}
38 changes: 38 additions & 0 deletions frontend/e2e/pages/modal-page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import BasePage from './base-page';

export class ModalPage extends BasePage {
private get cancelButton() {
return this.page.locator('[data-test-id="modal-cancel-action"]');
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 | 🔴 Critical | ⚡ Quick win

Test-id attribute mismatch will break selector.

Line 7 uses data-test-id="modal-cancel-action" but global.setup.ts:9 configured Playwright to use the data-test attribute for test-id selectors. This inconsistency will cause the locator to fail if the HTML uses data-test attributes (which is the convention after the global setup).

🔧 Proposed fix
-  private get cancelButton() {
-    return this.page.locator('[data-test-id="modal-cancel-action"]');
-  }
+  private get cancelButton() {
+    return this.page.getByTestId('modal-cancel-action');
+  }

This ensures consistent use of the configured test-id attribute.

📝 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.

Suggested change
return this.page.locator('[data-test-id="modal-cancel-action"]');
private get cancelButton() {
return this.page.getByTestId('modal-cancel-action');
}
🤖 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/modal-page.ts` at line 7, The locator in modal-page.ts
uses the wrong attribute name: update the return statement that calls
this.page.locator(...) for the modal cancel button so it targets the configured
data-test attribute instead of data-test-id (or, alternatively, use Playwright's
getByTestId('modal-cancel-action')). Change the selector that references
"modal-cancel-action" to match the project's configured test-id attribute
(data-test) so the locator resolves correctly.

}

private get submitButton() {
return this.page.locator('button[type=submit]');
}

async shouldBeOpened(): Promise<void> {
await this.cancelButton.scrollIntoViewIfNeeded();
await expect(this.cancelButton).toBeVisible({ timeout: 20_000 });
}

async shouldBeClosed(): Promise<void> {
await expect(this.cancelButton).toBeHidden();
}

async submit(): Promise<void> {
await this.submitButton.click();
}

async cancel(): Promise<void> {
await this.cancelButton.click();
}

async submitShouldBeDisabled(): Promise<void> {
await expect(this.submitButton).toBeDisabled();
}

async submitShouldBeEnabled(): Promise<void> {
await expect(this.submitButton).toBeEnabled();
}
}
Loading