test: Addressing review feedback

Author: jjaquish

playwright/RUNNING_TESTS.md

@@ -1,8 +1,8 @@
 # Running Playwright Tests
 
-The Playwright tests are configured to run against the **Red Hat stage environment** (`https://console.stage.redhat.com`).
+The Playwright tests are configured by default to run against the **local development proxy** (`https://stage.foo.redhat.com:1337`).
 
-This matches the pattern used by other HCC frontend E2E tests (e.g., rbac-ui) and is suitable for both local development and CI/CD (Konflux).
+This requires running the local dev server (`npm start`) and proper `/etc/hosts` configuration. For remote testing without a local proxy, you can override the base URL to `https://console.stage.redhat.com`.
 
 ## Prerequisites
 
@@ -24,47 +24,46 @@ E2E_PASSWORD=your-redhat-password
 
 ## Running Tests
 
-### Direct Against Stage (Recommended)
+### Against Local Dev Proxy (Default)
 
-Tests run directly against `https://console.stage.redhat.com` by default:
+Tests run against `https://stage.foo.redhat.com:1337` by default (requires `npm start`):
 
 ```bash
 # Run all migrated tests
-npx playwright test integrations.spec.ts notifications.spec.ts
+npx playwright test integrations-ui.spec.ts notifications-ui.spec.ts
 
 # Run only integration tests
-npx playwright test integrations.spec.ts
+npx playwright test integrations-ui.spec.ts
 
 # Run only notification tests
-npx playwright test notifications.spec.ts
+npx playwright test notifications-ui.spec.ts
 
 # Run in UI mode (interactive debugging)
 npx playwright test --ui
 
 # Run specific test suite
-npx playwright test integrations.spec.ts -g "Webhook Integration Creation"
+npx playwright test integrations-ui.spec.ts -g "Webhook Integration"
 
 # Run with headed browser (see what's happening)
 npx playwright test --headed
 
 # Debug a specific test
-npx playwright test integrations.spec.ts --debug
+npx playwright test integrations-ui.spec.ts --debug
 ```
 
-## Running Against Local Dev Server (Optional)
+## Running Against Remote Stage (Optional)
 
-If you want to test against a local development proxy instead:
+If you want to test against remote stage without a local proxy:
 
 ```bash
 # Set environment variable to override the default
-export PLAYWRIGHT_BASE_URL="https://stage.foo.redhat.com:1337"
+export PLAYWRIGHT_BASE_URL="https://console.stage.redhat.com"
 
 # Run tests
 npx playwright test
 ```
 
-**Note:** This requires `/etc/hosts` configuration and a running local dev server (`npm start`).
-Most developers and CI/CD should use the default stage environment instead.
+**Note:** Most local development uses the default local proxy (`stage.foo.redhat.com:1337`).
 
 ## Troubleshooting
 
@@ -154,8 +153,8 @@ In CI (Konflux), tests run with:
 export E2E_USER="your-redhat-username"
 export E2E_PASSWORD="your-redhat-password"
 
-# Run tests directly against stage
-npx playwright test integrations.spec.ts --headed
+# Run tests against local dev proxy
+npx playwright test integrations-ui.spec.ts --headed
 
 # Watch the tests run in the browser!
 ```

playwright/global-setup.ts

@@ -26,7 +26,7 @@ async function login(page: Page, user: string, password: string) {
     throw new Error('Proxy config incorrect - Lockdown page detected');
   }
 
-  console.log(`Logging in as ${user}...`);
+  console.log('Logging in...');
 
   // Fill username
   const usernameInput = page

playwright/integrations-ui.spec.ts

@@ -1,7 +1,8 @@
 import { expect, test } from '@playwright/test';
-import { INTEGRATIONS_PATH } from './test-utils';
+import { INTEGRATIONS_PATH, dismissCookieConsent } from './test-utils';
 import { generateCommunicationPayload, generateWebhookPayload } from './utils/data-generators';
 import {
+  deleteIntegration,
   fillCommunicationForm,
   fillWebhookForm,
   waitForSuccessNotification,
@@ -23,37 +24,37 @@ import {
 test.describe('Integrations Navigation', () => {
   test('should navigate across all tabs', async ({ page }) => {
     await page.goto(INTEGRATIONS_PATH);
-    await page.waitForLoadState('networkidle');
+    await page.waitForLoadState('domcontentloaded');
     await expect(page).toHaveURL(/settings\/integrations/);
 
     await test.step('Communications tab loads', async () => {
       const communicationsTab = page
         .locator('button:has-text("Communications"), a:has-text("Communications")')
         .first();
-      if (await communicationsTab.isVisible()) {
-        await communicationsTab.click();
-        await page.waitForLoadState('networkidle');
-      }
+      await expect(communicationsTab).toBeVisible({ timeout: 5000 });
+      await communicationsTab.click();
+      await page.waitForLoadState('domcontentloaded');
+      await expect(page).toHaveURL(/communications/);
     });
 
     await test.step('Reporting tab loads', async () => {
       const reportingTab = page
         .locator('button:has-text("Reporting"), a:has-text("Reporting")')
         .first();
-      if (await reportingTab.isVisible()) {
-        await reportingTab.click();
-        await page.waitForLoadState('networkidle');
-      }
+      await expect(reportingTab).toBeVisible({ timeout: 5000 });
+      await reportingTab.click();
+      await page.waitForLoadState('domcontentloaded');
+      await expect(page).toHaveURL(/reporting/);
     });
 
     await test.step('Webhooks tab loads', async () => {
       const webhooksTab = page
         .locator('button:has-text("Webhooks"), a:has-text("Webhooks")')
         .first();
-      if (await webhooksTab.isVisible()) {
-        await webhooksTab.click();
-        await page.waitForLoadState('networkidle');
-      }
+      await expect(webhooksTab).toBeVisible({ timeout: 5000 });
+      await webhooksTab.click();
+      await page.waitForLoadState('domcontentloaded');
+      await expect(page).toHaveURL(/webhooks/);
     });
   });
 });
@@ -68,16 +69,9 @@ test.describe('Webhook Integration Lifecycle', () => {
 
     // Navigate to integrations page
     await page.goto(INTEGRATIONS_PATH);
-    await page.waitForLoadState('networkidle');
+    await page.waitForLoadState('domcontentloaded');
 
-    // Dismiss cookie consent if it appears
-    const acceptCookies = page
-      .getByRole('button', { name: 'Accept all' })
-      .or(page.getByRole('button', { name: 'Accept' }));
-    if (await acceptCookies.isVisible({ timeout: 2000 }).catch(() => false)) {
-      await acceptCookies.click();
-      await page.waitForTimeout(500);
-    }
+    await dismissCookieConsent(page);
 
     // Click "Create Integration" dropdown button (use .first() as there may be multiple)
     const createButton = page.getByRole('button', { name: 'Create Integration' }).first();
@@ -91,58 +85,30 @@ test.describe('Webhook Integration Lifecycle', () => {
     await webhooksMenuItem.waitFor({ state: 'visible', timeout: 5000 });
     await webhooksMenuItem.click();
 
-    // Now the wizard dialog should open
-    await page.waitForTimeout(1000);
+    // Wait for wizard dialog to open
+    await page.waitForSelector('[role="dialog"]', { state: 'visible', timeout: 10000 });
 
     // Fill the webhook form
     await fillWebhookForm(page, webhookPayload);
     await waitForSuccessNotification(page);
 
-    // Modal closes automatically - wait for it
-    await page.waitForTimeout(2000);
+    // Wait for modal to close
+    await page.waitForSelector('[role="dialog"]', { state: 'hidden', timeout: 10000 });
 
     // Navigate to Webhooks tab to verify the webhook appears
     const webhooksTab = page.locator('button:has-text("Webhooks"), a:has-text("Webhooks")').first();
     await webhooksTab.click();
-    await page.waitForLoadState('networkidle');
-    await page.waitForTimeout(1000);
+    await page.waitForLoadState('domcontentloaded');
 
     // Verify webhook appears in the list
     const webhookElement = page.locator(`text="${webhookPayload.name}"`).first();
     await expect(webhookElement).toBeVisible({ timeout: 10000 });
 
     // Delete the webhook
-    const container = page
-      .locator('[role="row"], [class*="card"]', {
-        has: page.locator(`text="${webhookPayload.name}"`),
-      })
-      .first();
-
-    const deleteButton = container
-      .locator('button[aria-label*="Delete"], button:has-text("Delete")')
-      .first();
-    if (await deleteButton.isVisible({ timeout: 3000 }).catch(() => false)) {
-      await deleteButton.click();
-    } else {
-      const kebabButton = container
-        .locator('button[aria-label*="Actions"], button[aria-label*="Kebab"]')
-        .first();
-      await kebabButton.click();
-      await page.waitForTimeout(500);
-      const deleteMenuItem = page.locator('button:has-text("Delete")').first();
-      await deleteMenuItem.click();
-    }
-
-    // Confirm deletion
-    const confirmButton = page
-      .locator('button:has-text("Delete"), button:has-text("Confirm")')
-      .first();
-    await confirmButton.waitFor({ state: 'visible', timeout: 5000 });
-    await confirmButton.click({ force: true });
+    await deleteIntegration(page, webhookPayload.name);
 
-    await page.waitForTimeout(2000);
-    const webhookGone = await webhookElement.isVisible({ timeout: 2000 }).catch(() => false);
-    expect(webhookGone).toBe(false);
+    // Verify it's gone
+    await expect(webhookElement).not.toBeVisible({ timeout: 10000 });
   });
 });
 
@@ -156,16 +122,9 @@ test.describe('Communication Integration Lifecycle', () => {
 
     // Navigate to integrations page
     await page.goto(INTEGRATIONS_PATH);
-    await page.waitForLoadState('networkidle');
+    await page.waitForLoadState('domcontentloaded');
 
-    // Dismiss cookie consent if it appears
-    const acceptCookies = page
-      .getByRole('button', { name: 'Accept all' })
-      .or(page.getByRole('button', { name: 'Accept' }));
-    if (await acceptCookies.isVisible({ timeout: 2000 }).catch(() => false)) {
-      await acceptCookies.click();
-      await page.waitForTimeout(500);
-    }
+    await dismissCookieConsent(page);
 
     // Click "Create Integration" dropdown button (same pattern as webhook)
     const createButton = page.getByRole('button', { name: 'Create Integration' }).first();
@@ -179,61 +138,31 @@ test.describe('Communication Integration Lifecycle', () => {
     await communicationsMenuItem.waitFor({ state: 'visible', timeout: 5000 });
     await communicationsMenuItem.click();
 
-    // Wait for wizard to appear
-    await page.waitForTimeout(1000);
+    // Wait for wizard dialog to open
+    await page.waitForSelector('[role="dialog"]', { state: 'visible', timeout: 10000 });
 
-    // Fill the communication form (wizard should be open now)
+    // Fill the communication form
     await fillCommunicationForm(page, payload);
     await waitForSuccessNotification(page);
 
-    // Modal closes automatically - wait for it
-    await page.waitForTimeout(2000);
+    // Wait for modal to close
+    await page.waitForSelector('[role="dialog"]', { state: 'hidden', timeout: 10000 });
 
     // Navigate to Communications tab to verify the integration appears
     const communicationsTab = page
       .locator('button:has-text("Communications"), a:has-text("Communications")')
       .first();
     await communicationsTab.click();
-    await page.waitForLoadState('networkidle');
-    await page.waitForTimeout(1000);
+    await page.waitForLoadState('domcontentloaded');
 
     // Verify integration appears in the list
     const integrationElement = page.locator(`text="${payload.name}"`).first();
     await expect(integrationElement).toBeVisible({ timeout: 10000 });
 
     // Delete the integration
-    const container = page
-      .locator('[role="row"], [class*="card"]', {
-        has: page.locator(`text="${payload.name}"`),
-      })
-      .first();
+    await deleteIntegration(page, payload.name);
 
-    const deleteButton = container
-      .locator('button[aria-label*="Delete"], button:has-text("Delete")')
-      .first();
-    if (await deleteButton.isVisible({ timeout: 3000 }).catch(() => false)) {
-      await deleteButton.click();
-    } else {
-      const kebabButton = container
-        .locator('button[aria-label*="Actions"], button[aria-label*="Kebab"]')
-        .first();
-      await kebabButton.click();
-      await page.waitForTimeout(500);
-      const deleteMenuItem = page.locator('button:has-text("Delete")').first();
-      await deleteMenuItem.click();
-    }
-
-    // Confirm deletion
-    const confirmButton = page
-      .locator('button:has-text("Delete"), button:has-text("Confirm")')
-      .first();
-    await confirmButton.waitFor({ state: 'visible', timeout: 5000 });
-    await confirmButton.click({ force: true });
-
-    await page.waitForTimeout(2000);
-    const integrationGone = await integrationElement
-      .isVisible({ timeout: 2000 })
-      .catch(() => false);
-    expect(integrationGone).toBe(false);
+    // Verify it's gone
+    await expect(integrationElement).not.toBeVisible({ timeout: 10000 });
   });
 });