NO-JIRA: Remove redundant waitFor calls in Playwright e2e tests

.claude/migration-context.md

@@ -97,9 +97,11 @@ When migrating a Cypress test that uses `cy.get('[data-test-id="x"]')` or `cy.by
 
 | Cypress                                                  | Playwright                                                                                                                                                |
 | -------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `cy.wait(3000)`                                          | **AVOID.** Use `await expect(locator).toBeVisible()` or condition-based waits. Only use `page.waitForTimeout()` as absolute last resort during debugging. |
-| `cy.get(s, { timeout: 60000 })`                          | `await this.page.locator(s).waitFor({ state: 'visible', timeout: 60000 })`                                                                                |
-| `cy.contains(text, { timeout })`                         | `await this.page.getByText(text).waitFor({ state: 'visible', timeout })`                                                                                  |
+| `cy.wait(3000)`                                          |  Use web assertions e.g. `await expect(locator).toBeVisible()` or condition-based waits. Only use `page.waitForTimeout()` as absolute last resort during debugging. |
+| `cy.get(s, { timeout }).click()`                         | `await locator.click({ timeout })` — pass timeout to the action, not a separate `waitFor()`. All Playwright actions accept a `timeout` option             |
+| `cy.get(s, { timeout }).should('be.visible')`            | `await expect(locator).toBeVisible({ timeout })` — pass timeout to the assertion                                                                          |
+| `cy.get(s, { timeout })` (no action, just waiting)       | `await locator.waitFor({ state: 'visible', timeout })` — only when no action or assertion follows                                                         |
+| `cy.contains(text, { timeout })`                         | `await expect(page.getByText(text)).toBeVisible({ timeout })` or `await page.getByText(text).click({ timeout })` depending on what follows                |
 | `cy.intercept('GET', url).as('req')` + `cy.wait('@req')` | `await this.page.waitForResponse(url)` or `page.waitForResponse(resp => resp.url().includes(url))`                                                        |
 
 ### Resource Lifecycle
@@ -439,11 +441,96 @@ The MCP's tracked page stays on `about:blank` — use the `p` reference from the
 
 ---
 
+## Playwright Auto-Awaiting
+
+Playwright action methods (`fill()`, `click()`, `check()`, `uncheck()`, `selectOption()`, `type()`, `press()`) **auto-wait for the element to be actionable** (visible, enabled, stable). You do NOT need an explicit `waitFor()` before calling these actions. This includes `robustClick()` in page objects — it also auto-waits.
+
+> **ESLint enforcement:** The `no-restricted-syntax` rule in `e2e/.eslintrc.cjs` warns on all `.waitFor()` calls. Legitimate uses must have `// eslint-disable-next-line no-restricted-syntax`. This catches redundant `waitFor()` at lint time — `yarn eslint` will flag new violations.
+
+```typescript
+// WRONG — redundant waitFor before an action
+await input.waitFor({ state: 'visible' });
+await input.fill('text');
+
+// WRONG — redundant waitFor before robustClick
+await action.waitFor({ state: 'visible', timeout: 10_000 });
+await this.robustClick(action);
+
+// RIGHT — actions auto-wait for actionability
+await input.fill('text');
+await this.robustClick(action);
+
+// RIGHT — if you need a custom timeout, pass it to the action
+await input.fill('text', { timeout: 10_000 });
+await action.click({ timeout: 10_000 });
+```
+
+Only use explicit `waitFor()` when you need to wait for an element **without acting on it** — e.g., confirming navigation completed, or waiting for loading indicators to disappear:
+
+```typescript
+// OK — waiting for a state transition, not an action
+await page.getByTestId('loading-indicator').waitFor({ state: 'detached' });
+
+// OK — confirming the editor loaded before reading its content (not an action on the element)
+await page.getByTestId('code-editor').waitFor({ state: 'visible' });
+```
+
+Similarly, `waitForLoadingComplete()` should not be called at the end of page object methods like `selectProject()`. The caller's next action will auto-wait for whatever element it needs.
+
+---
+
+## Adding `data-test` Attributes
+
+When migrating selectors, **always check the React source** for existing `data-test` attributes before creating locators:
+
+1. **If `data-test` already exists** on the element → use `getByTestId('value')` directly.
+2. **If only a legacy attribute exists** (`data-test-id`, `data-test-rows`, `data-test-dropdown-menu`, `data-test-action`, etc.) → add `data-test="value"` to the React component source alongside the existing legacy attribute, then use `getByTestId('value')`.
+3. **Never use legacy attribute selectors** like `page.locator('[data-test-rows="..."]')` or `page.locator('[data-test-dropdown-menu="..."]')` when `data-test` exists or can be added.
+
+```typescript
+// WRONG — using legacy selector directly
+private readonly resourceRows = this.page.locator('[data-test-rows="resource-row"]');
+
+// RIGHT — data-test="resource-row" already exists on the same element
+private readonly resourceRows = this.page.getByTestId('resource-row');
+```
+
+---
+
+## k8sClient Cleanup
+
+`KubernetesClient.deleteNamespace()` and `KubernetesClient.deleteCustomResource()` catch errors and call `isNotFound(err)` to silently swallow 404 "not found" responses. Do NOT wrap these cleanup calls in try/catch blocks. Note: `deleteClusterCustomResource` is not implemented in `KubernetesClient` — do not reference it.
+
+```typescript
+// WRONG — unnecessary error handling
+test.afterAll(async ({ k8sClient }) => {
+  try { await k8sClient.deleteNamespace(namespace); } catch { /* may already be deleted */ }
+});
+
+// RIGHT — k8sClient handles 404 silently
+test.afterAll(async ({ k8sClient }) => {
+  await k8sClient.deleteNamespace(namespace);
+});
+```
+
+---
+
+## Page Object Naming
+
+- Do NOT prefix methods or locators with `legacy`. If a locator targets an older DOM structure that will be replaced, name it for what it does, not its age (e.g., `filterByNameInput` not `legacyFilterByName`).
+- Common actions (navigate to form, click create dropdown item, filter + select) should be page object methods, not inline locator chains in spec files.
+
+---
+
 ## Things to NEVER Do
 
 - **Never import `test` or `expect` from `@playwright/test`** — import from `e2e/fixtures`
 - **Never transliterate** — `cy.get(x).click()` → `page.locator(x).click()` is not a migration. Understand intent, use idiomatic Playwright APIs
 - **Never use `page.waitForTimeout()`** as a replacement for `cy.wait()`. Find the condition to wait for
+- **Never add `waitFor()` before an action** — `fill()`, `click()`, `check()`, etc. already auto-wait for actionability
+- **Never use legacy test attribute selectors** (`[data-test-rows="..."]`, `[data-test-id="..."]`, `[data-test-dropdown-menu="..."]`) — add `data-test` to the React source and use `getByTestId()`
+- **Never wrap k8sClient cleanup in try/catch** — `deleteNamespace` and `deleteCustomResource` already swallow 404s
+- **Never prefix methods with `legacy`** — name for what it does, not its age
 - **Never put locators in spec files** when a page object exists or should exist
 - **Never rely on test order** — each `test()` must work independently.
 - **Never skip cleanup** — every created resource must be tracked with `cleanup.track*()`

.claude/skills/migrate-cypress/SKILL.md

@@ -79,8 +79,10 @@ If MCP is unavailable or no cluster is reachable, log a warning: "Playwright MCP
 1. Create/extend page objects with locators and interaction methods. Follow the established pattern:
    - Import `Locator` type from `@playwright/test` and default-import `BasePage`
    - Use `getByTestId()` for `data-test` attributes, `locator()` for other selectors
+   - If the React component only has a legacy test attribute (`data-test-id`, `data-test-rows`, `data-test-dropdown-menu`, etc.) but no `data-test`, **add `data-test` to the React component source** and use `getByTestId()` — never use legacy attribute selectors directly
    - Expose locators via getter methods (`getX(): Locator`), keep locator properties `private readonly`
-   - Use `robustClick()` inside page objects; specs use plain `.click()`.  
+   - Use `robustClick()` inside page objects; specs use plain `.click()`
+   - Do NOT name methods or locators with a `legacy` prefix — name for what they do
 
 Example:
    ```typescript
@@ -145,8 +147,10 @@ Example:
 - **Self-contained tests** — merge sequential `it` blocks into one `test()` with `test.step()`
 - **No fixed waits** — replace `cy.wait(ms)` with condition-based waits or assertion timeouts
 - **No shell commands** — replace `cy.exec('oc ...')` with `KubernetesClient`
+- **No try/catch in cleanup** — `k8sClient.deleteNamespace()` and `deleteCustomResource()` already swallow 404 errors
+- **Add `data-test` to React source** — when the component only has legacy test attributes (`data-test-id`, `data-test-rows`, etc.), add `data-test` alongside and use `getByTestId()`
 - **Framework-first** — use existing page objects before creating new ones
-- **Correct layer** — locators in page objects, test scenarios in specs
+- **Correct layer** — locators in page objects, test scenarios in specs; common multi-step interactions belong in page object methods, not inline in specs
 
 ## Troubleshooting
 

frontend/e2e/.eslintrc.cjs

@@ -9,6 +9,17 @@ module.exports = {
   rules: {
     'no-console': 'off',
     'no-empty-pattern': 'off',
+    'no-restricted-syntax': [
+      'warn',
+      {
+        selector: 'CallExpression[callee.property.name="waitFor"]',
+        message:
+          'Playwright actions (click, fill, check, clear) auto-wait for actionability. ' +
+          'Do not call waitFor() before an action on the same locator. ' +
+          'If this waitFor() is intentional (waiting for state without a subsequent action), ' +
+          'add // eslint-disable-next-line no-restricted-syntax',
+      },
+    ],
     'playwright/no-conditional-in-test': 'off',
     'playwright/no-skipped-test': ['warn', { allowConditional: true }],
   },

frontend/e2e/pages/alertmanager-page.ts

@@ -24,18 +24,17 @@ export class AlertmanagerPage extends BasePage {
 
   async navigateToAlertmanager(): Promise<void> {
     await this.goTo('/settings/cluster/alertmanagerconfig');
-    await this.createReceiverButton.waitFor({ state: 'visible' });
+    await expect(this.createReceiverButton).toBeVisible();
   }
 
   async navigateToYAMLPage(): Promise<void> {
     await this.goTo('/settings/cluster/alertmanageryaml');
-    // Wait for editor toolbar to load (indicates editor is ready)
-    await this.page.getByRole('button', { name: 'Copy code to clipboard' }).waitFor();
+    await expect(this.page.getByRole('button', { name: 'Copy code to clipboard' })).toBeVisible();
   }
 
   async navigateToEditReceiver(receiverName: string): Promise<void> {
     await this.goTo(`/settings/cluster/alertmanagerconfig/receivers/${receiverName}/edit`);
-    await this.saveChangesButton.waitFor({ state: 'visible' });
+    await expect(this.saveChangesButton).toBeVisible();
   }
 
   async createReceiver(receiverName: string, receiverTypeConfig: string): Promise<void> {
@@ -51,7 +50,7 @@ export class AlertmanagerPage extends BasePage {
   async save(): Promise<void> {
     await expect(this.saveChangesButton).toBeEnabled();
     await this.robustClick(this.saveChangesButton);
-    await this.createReceiverButton.waitFor({ state: 'visible', timeout: 60_000 });
+    await expect(this.createReceiverButton).toBeVisible({ timeout: 60_000 });
   }
 
   async showAdvancedConfiguration(): Promise<void> {
@@ -60,7 +59,7 @@ export class AlertmanagerPage extends BasePage {
 
     const button = this.advancedConfigButton.locator('button');
     await this.robustClick(button);
-    await sendResolved.waitFor({ state: 'visible', timeout: 15_000 });
+    await expect(sendResolved).toBeVisible({ timeout: 15_000 });
   }
 
   async getYAMLContent(): Promise<string> {

frontend/e2e/pages/base-page.ts

@@ -21,6 +21,7 @@ export default abstract class BasePage {
     try {
       const count = await loadingElements.count().catch(() => 0);
       if (count > 0) {
+        // eslint-disable-next-line no-restricted-syntax
         await loadingElements.first().waitFor({ state: 'hidden', timeout: timeoutMs });
       }
     } catch {
@@ -61,6 +62,7 @@ export default abstract class BasePage {
     for (let attempt = 1; attempt <= retries; attempt++) {
       try {
         await this.waitForLoadingComplete(Math.min(attemptTimeout / 4, 3_000));
+        // eslint-disable-next-line no-restricted-syntax
         await locator.waitFor({ state: 'visible', timeout: attemptTimeout });
         await locator.scrollIntoViewIfNeeded({ timeout: attemptTimeout / 3 });
 

frontend/e2e/pages/cluster-dashboard-page.ts

@@ -1,4 +1,5 @@
 import type { Locator } from '@playwright/test';
+import { expect } from '@playwright/test';
 
 import BasePage from './base-page';
 
@@ -18,7 +19,8 @@ export class ClusterDashboardPage extends BasePage {
   }
 
   async waitForStatusCardLoaded(): Promise<void> {
-    await this.statusCard.waitFor({ state: 'visible', timeout: 30_000 });
+    await expect(this.statusCard).toBeVisible({ timeout: 30_000 });
+    // eslint-disable-next-line no-restricted-syntax
     await this.statusCard.locator('.skeleton-health').waitFor({ state: 'hidden', timeout: 30_000 }).catch(() => {
       // Skeletons may have already disappeared
     });
@@ -38,6 +40,6 @@ export class ClusterDashboardPage extends BasePage {
 
   async openInsightsPopup(): Promise<void> {
     await this.robustClick(this.insightsButton);
-    await this.popover.waitFor({ state: 'visible', timeout: 10_000 });
+    await expect(this.popover).toBeVisible({ timeout: 10_000 });
   }
 }

frontend/e2e/pages/cluster-settings-page.ts

@@ -1,4 +1,5 @@
 import type { Locator } from '@playwright/test';
+import { expect } from '@playwright/test';
 
 import BasePage from './base-page';
 
@@ -29,7 +30,7 @@ export class ClusterSettingsPage extends BasePage {
    */
   async navigateToDetails(): Promise<void> {
     await this.goTo('/settings/cluster');
-    await this.detailsTab.waitFor({ state: 'visible' });
+    await expect(this.detailsTab).toBeVisible();
   }
 
   /**
@@ -43,9 +44,8 @@ export class ClusterSettingsPage extends BasePage {
    * Click the current channel update link to open the modal
    */
   async openChannelModal(): Promise<void> {
-    await this.currentChannelUpdateLink.waitFor({ state: 'visible' });
     await this.currentChannelUpdateLink.click();
-    await this.modalTitle.waitFor({ state: 'visible', timeout: 30_000 });
+    await expect(this.modalTitle).toBeVisible({ timeout: 30_000 });
   }
 
   /**
@@ -59,7 +59,6 @@ export class ClusterSettingsPage extends BasePage {
    * Type a channel name into the input field (for "Input channel" modal)
    */
   async inputChannelName(channelName: string): Promise<void> {
-    await this.channelModalInput.waitFor({ state: 'visible' });
     await this.channelModalInput.clear();
     await this.channelModalInput.fill(channelName);
   }
@@ -81,6 +80,7 @@ export class ClusterSettingsPage extends BasePage {
   async confirmAction(): Promise<void> {
     await this.robustClick(this.confirmActionButton);
     // Wait for modal to close
+    // eslint-disable-next-line no-restricted-syntax
     await this.modalTitle.waitFor({ state: 'hidden', timeout: 10_000 }).catch(() => {
       // Modal may close quickly, ignore timeout
     });
@@ -91,11 +91,9 @@ export class ClusterSettingsPage extends BasePage {
    */
   async navigateToConfigurationTab(): Promise<void> {
     await this.navigateToTab(this.configurationTab);
-    // Wait for loading to complete
-    await this.page
-      .locator('.loading-box__loaded')
-      .first()
-      .waitFor({ state: 'visible', timeout: 30_000 });
+    await expect(this.page.locator('.loading-box__loaded').first()).toBeVisible({
+      timeout: 30_000,
+    });
   }
 
   /**
@@ -169,21 +167,11 @@ export class ClusterSettingsPage extends BasePage {
    */
   async openUpdateModal(): Promise<void> {
     const updateButton = this.page.getByTestId('cv-update-button');
-    await updateButton.waitFor({ state: 'visible' });
     await this.robustClick(updateButton);
 
     // Wait for modal to appear
-    await this.modalTitle.waitFor({ state: 'visible', timeout: 10_000 });
-    await this.page.waitForFunction(
-      () => {
-        const title = document.querySelector('[data-test="modal-title"]');
-        return title?.textContent?.includes('Update cluster');
-      },
-      { timeout: 10_000 },
-    );
-
-    const modal = this.page.getByTestId('update-cluster-modal');
-    await modal.waitFor({ state: 'visible' });
+    await expect(this.modalTitle).toContainText('Update cluster', { timeout: 10_000 });
+    await expect(this.page.getByTestId('update-cluster-modal')).toBeVisible();
   }
 
   /**
@@ -193,9 +181,6 @@ export class ClusterSettingsPage extends BasePage {
     const modal = this.page.getByTestId('update-cluster-modal');
     const dropdownToggle = modal.getByTestId('dropdown-with-switch-toggle');
 
-    await dropdownToggle.waitFor({ state: 'visible' });
-    await dropdownToggle.waitFor({ state: 'attached' });
-
     await this.robustClick(dropdownToggle);
   }
 }

frontend/e2e/pages/details-page.ts

@@ -4,22 +4,6 @@ import BasePage from './base-page';
 
 export class DetailsPage extends BasePage {
   private readonly pageHeading = this.page.getByTestId('page-heading');
-  private readonly resourceTitle = this.page.getByTestId('resource-title');
-  private readonly skeletonLoader = this.page.getByTestId('skeleton-detail-view');
-
-  /**
-   * Wait for the details page to load
-   */
-  async waitForPageLoad(): Promise<void> {
-    await this.skeletonLoader.waitFor({ state: 'detached', timeout: 30_000 }).catch(() => {
-      // Skeleton may not appear for fast loads
-    });
-    // Wait for either resource title or page heading to be visible
-    await Promise.race([
-      this.resourceTitle.waitFor({ state: 'visible', timeout: 30_000 }),
-      this.pageHeading.waitFor({ state: 'visible', timeout: 30_000 }),
-    ]);
-  }
 
   /**
    * Get the page heading locator
@@ -42,7 +26,6 @@ export class DetailsPage extends BasePage {
    */
   async clickKebabAction(actionId: string): Promise<void> {
     const action = this.page.locator(`[data-test-action="${actionId}"]`);
-    await action.waitFor({ state: 'visible', timeout: 10_000 });
     await this.robustClick(action);
   }
 
@@ -64,15 +47,10 @@ export class DetailsPage extends BasePage {
 
   /**
    * Click a resource row to navigate to its details
-   * Set waitForLoad=false to skip waiting for page load (useful for in-page navigation)
    */
-  async clickResourceRow(resourceId: string, waitForLoad = true): Promise<void> {
+  async clickResourceRow(resourceId: string): Promise<void> {
     const row = this.getResourceRow(resourceId);
-    await row.waitFor({ state: 'visible', timeout: 30_000 });
     await this.robustClick(row);
-    if (waitForLoad) {
-      await this.waitForPageLoad();
-    }
   }
 
   /**

frontend/e2e/pages/navigation.ts

@@ -15,8 +15,6 @@ export class Navigation {
     // Navigate to home first to ensure app is loaded
     await this.page.goto('/');
     const sectionButton = this.page.getByRole('button', { name: section });
-    await sectionButton.waitFor({ state: 'visible' });
-
     await sectionButton.click();
     await this.page.getByRole('link', { name: link }).click();
     await this.page.waitForLoadState('domcontentloaded');