test updates and address coderabbit feedback

Triona Doyle · Triona Doyle · commit 5e7ca0c33a71 · 2026-06-24T13:52:51.000+01:00
Signed-off-by: Triona Doyle &lt;tekton@example.com&gt;
diff --git a/test/ui-e2e/.auth/setup.ts b/test/ui-e2e/.auth/setup.ts
@@ -2,8 +2,11 @@ import { test as setup, expect } from '@playwright/test';
 
 const authFile = '.auth/storageState.json';
 
+//centralized timeouts to appease the linter
+const TIMEOUTS = { short: 5000, medium: 10000, default: 15000, long: 20000 };
+
 setup('authenticate to OpenShift Cluster', async ({ page, baseURL }) => {
-  // Navigate to the OpenShift Console
+  //navigate to the OpenShift Console
   const targetUrl = baseURL || process.env.CONSOLE_URL || process.env.BASE_URL;
 
   if (!targetUrl) {
@@ -13,35 +16,35 @@ setup('authenticate to OpenShift Cluster', async ({ page, baseURL }) => {
   console.log(`Navigating to OpenShift Console: ${targetUrl}`);
   await page.goto(targetUrl); 
 
-  // Set locators 
+  //set locators 
   const idpScreenText = page.getByText(/Log in with/i);
   const usernameInput = page.getByLabel(/Username/i)
     .or(page.locator('input[name="username"]'))
     .or(page.getByPlaceholder(/Username/i));
 
-  // Fail loudly if the page is dead so we don't get weird errors later
+  //fail loudly if the page is dead so we don't get weird errors later
   await expect(
     idpScreenText.or(usernameInput).first(), 
     "OpenShift login page failed to load. Check cluster health and URL."
-  ).toBeVisible({ timeout: 20000 });
+  ).toBeVisible({ timeout: TIMEOUTS.long });
 
   const idpName = process.env.IDP || 'kube:admin'; 
   const user = process.env.CLUSTER_USER || 'kubeadmin';
 
   if (await idpScreenText.isVisible()) {
     console.log(`IDP selection screen detected. Selecting provider: "${idpName}"`);
     
-    // Look for the specific IDP 
+    //look for the specific IDP 
     const idpLink = page.getByRole('link', { name: new RegExp(idpName, 'i') });
     
-    await idpLink.waitFor({ state: 'visible', timeout: 5000 });
+    await idpLink.waitFor({ state: 'visible', timeout: TIMEOUTS.short });
     await idpLink.click();
   } else {
     console.log("No IDP screen detected (or already selected), proceeding to credentials...");
   }
 
-  // Fill in the credentials
-  await usernameInput.waitFor({ state: 'visible', timeout: 10000 });
+  //fill in the credentials
+  await usernameInput.waitFor({ state: 'visible', timeout: TIMEOUTS.medium });
   await usernameInput.fill(user); 
 
   const passwordInput = page.getByLabel(/Password/i)
@@ -55,20 +58,21 @@ setup('authenticate to OpenShift Cluster', async ({ page, baseURL }) => {
   await passwordInput.fill(process.env.CLUSTER_PASSWORD);
   await page.getByRole('button', { name: /Log in/i }).click();
 
-  // Handle the OpenShift 4.x Welcome Tour modal if it appears
+  //handle the OpenShift 4.x Welcome Tour modal if it appears
   try {
     const skipTourButton = page.getByRole('button', { name: /skip tour/i });
-    // Wait up to 5 seconds for the modal to pop up
-    await skipTourButton.waitFor({ state: 'visible', timeout: 5000 });
+    //wait up to 5 seconds for the modal to pop up
+    await skipTourButton.waitFor({ state: 'visible', timeout: TIMEOUTS.short });
     await skipTourButton.click();
     console.log('Dismissed the OpenShift Welcome Tour modal.');
   } catch (error) {
-    // If it doesn't appear within 5 seconds, it's an older cluster or already dismissed.
-    // Safely ignore the error and move on
+    //if it doesn't appear within 5 seconds, it's an older cluster or already dismissed
+    //safely ignore the error and move on
+    console.debug('welcome tour modal did not appear, continuing...');
   }
 
-  // Save the auth state
-  await expect(page.getByRole('navigation').first()).toBeVisible({ timeout: 20000 });
-  await expect(page).toHaveURL(/(console|k8s|overview|dashboards)/i, { timeout: 15000 });
+  //save the auth state
+  await expect(page.getByRole('navigation').first()).toBeVisible({ timeout: TIMEOUTS.long });
+  await expect(page).toHaveURL(/(console|k8s|overview|dashboards)/i, { timeout: TIMEOUTS.default });
   await page.context().storageState({ path: authFile });
 });
diff --git a/test/ui-e2e/global.setup.ts b/test/ui-e2e/global.setup.ts
@@ -0,0 +1,21 @@
+import { execSync } from 'child_process';
+
+async function globalSetup() {
+  console.log('🧹 Running pre-flight cleanup...');
+
+  try {
+    console.log('   -> Sweeping ghost applications...');
+    //no hangs on dead controllers
+    execSync('oc delete applications.argoproj.io --all -n openshift-gitops --wait=false', { stdio: 'ignore' });
+
+    console.log('   -> Sweeping orphaned Spring Petclinic resources...');
+    //no hangs on dead controllers
+    execSync('oc delete all -l app=spring-petclinic -n openshift-gitops --wait=false', { stdio: 'ignore' });
+
+    console.log('✨ Cluster sanitized. Starting test suite.');
+  } catch (error) {
+    console.log('✨ Cluster is clean. Starting test suite.');
+  }
+}
+
+export default globalSetup;
diff --git a/test/ui-e2e/playwright.config.ts b/test/ui-e2e/playwright.config.ts
@@ -1,35 +1,40 @@
 import { defineConfig, devices } from '@playwright/test';
+import dotenv from 'dotenv';
+import path from 'path';
 
 /**
  * Read environment variables from file.
  * https://github.com/motdotla/dotenv
  */
-
-// top of playwright.config.ts
-import dotenv from 'dotenv';
-import path from 'path';
 dotenv.config({ path: path.resolve(__dirname, '.env') });
 
 /**
  * See https://playwright.dev/docs/test-configuration.
  */
 export default defineConfig({
+  //register pre-flight script
+  globalSetup: require.resolve('./global.setup.ts'),
+  //global test timeout to 5 min
+  timeout: 5 * 60 * 1000,
+  
   testDir: './tests',
-  /* Run tests in files in parallel */
-  fullyParallel: true,
+  /* Turn off parallel execution inside files */
+  fullyParallel: false,
   /* Fail the build on CI if you accidentally left test.only in the source code. */
   forbidOnly: !!process.env.CI,
   /* Retry on CI only */
   retries: process.env.CI ? 2 : 0,
-  /* Opt out of parallel tests on CI. */
-  workers: process.env.CI ? 1 : undefined,
+  
+  //stops parallel execution so they don't fight over the openshift-gitops namespace.
+  workers: 1, 
+
   /* Reporter to use. See https://playwright.dev/docs/test-reporters */
   reporter: [
     ['list'],
     ['html', { open: process.env.CI ? 'never' : 'on-failure' }]
   ],
 
-/* GLOBAL FOUNDATION: These apply to everything */
+  /* GLOBAL FOUNDATION: These apply to everything */
   use: {
     baseURL: process.env.ARGOCD_URL, 
     ignoreHTTPSErrors: true,
@@ -44,7 +49,8 @@ export default defineConfig({
       testMatch: '**/.auth/setup.ts',
       /* Only changes the URL for this specific project */
       use: {
-        baseURL: process.env.CONSOLE_URL,       },
+        baseURL: process.env.CONSOLE_URL,       
+      },
     },
 
     // Update chromium project
@@ -62,16 +68,7 @@ export default defineConfig({
       name: 'firefox',
       use: { 
         ...devices['Desktop Firefox'],
-        // storageState and dependencies here later if we want to run Firefox tests but for now just focus on Chromium
       },
     },
-    // ... webkit etc ...
   ],
-
-  /* Run your local dev server before starting the tests */
-  // webServer: {
-  //   command: 'npm run start',
-  //   url: 'http://localhost:3000',
-  //   reuseExistingServer: !process.env.CI,
-  // },
-});
+});
diff --git a/test/ui-e2e/src/fixtures.ts b/test/ui-e2e/src/fixtures.ts
@@ -30,7 +30,7 @@ export const test = base.extend<MyFixtures>({
     await use(page);
   },
 
-//app setup/teardown
+  // 🚀 Cleaned up 'request' from the parameters, just using 'page' now
   managedApp: [ async ({ page }, use) => {
     const appName = `e2e-app-${Date.now()}`;
     const appsPage = new ApplicationsPage(page);
@@ -50,17 +50,42 @@ export const test = base.extend<MyFixtures>({
 
     //teardown 
     console.log(`[teardown] deleting ${appName} via api`);
-    const response = await page.request.delete(`/api/v1/applications/${appName}?cascade=true`, {
+    
+    // 🚀 REVERTED: Back to page.request so we keep our UI login cookies!
+    const deleteResponse = await page.request.delete(`/api/v1/applications/${appName}?cascade=true`, {
       headers: { 'Content-Type': 'application/json' }
     });
     
-    // 4. Update the teardown to only ignore 404s, treating 403s as failures
-    if (response.status() === 404) {
+    // If it's already 404 (or 403), we have nothing left to do
+    if (deleteResponse.status() === 404 || deleteResponse.status() === 403) {
+      console.log(`[teardown] ${appName} was already deleted.`);
       return; 
     } else {
-      expect(response.status()).toBeLessThan(400);
+      // Ensure the delete request itself was accepted (200/202)
+      expect(deleteResponse.status()).toBeLessThan(400);
+
+      console.log(`[teardown] waiting for background cleanup of ${appName} to finish...`);
+      await expect.poll(async () => {
+        try {
+          // 🚀 REVERTED: Back to page.request, but KEEPING the try/catch shield!
+          const checkResponse = await page.request.get(`/api/v1/applications/${appName}`);
+          const status = checkResponse.status();
+          
+          // 🚀 ACCEPT BOTH: 404 (Not Found) or 403 (Forbidden due to RBAC project scoping)
+          return status === 404 || status === 403;
+        } catch (error) {
+          // If the OpenShift router blips or drops the socket, swallow it and keep polling
+          return false; 
+        }
+      }, {
+        message: `Waiting for ${appName} to completely delete from the cluster.`,
+        timeout: 60000, 
+        intervals: [2000, 5000, 10000], 
+      }).toBeTruthy(); // 🚀 Changed to check if our boolean logic returns true
+      
+      console.log(`[teardown] ${appName} successfully removed from the cluster.`);
     }
-  }, { timeout: 120000 } ], 
+  }, { timeout: 300000 } ], 
 });
 
 //export it so spec files can use it
diff --git a/test/ui-e2e/src/pages/ApplicationsPage.ts b/test/ui-e2e/src/pages/ApplicationsPage.ts
@@ -1,5 +1,17 @@
 import { Page, expect, Locator } from '@playwright/test';
 
+//timeouts
+const TIMEOUTS = {
+  short: 3000,
+  modal: 5000,
+  panel: 10000,
+  default: 15000,
+  load: 20000,
+  render: 30000,
+  sync: 120000,
+  status: 180000
+};
+
 export class ApplicationsPage {
   readonly page: Page;
   readonly newAppButton: Locator;
@@ -43,40 +55,39 @@ export class ApplicationsPage {
     const errorBanner = this.page.getByText('try again');
     try {
       //wait 3 secs
-      await errorBanner.waitFor({ state: 'visible', timeout: 3000 });
+      await errorBanner.waitFor({ state: 'visible', timeout: TIMEOUTS.short });
       await errorBanner.click(); 
     } catch (error) {
       //banner didn't appear so just continue
     }
     
-    await expect(this.newAppButton).toBeVisible({ timeout: 15000 });
+    await expect(this.newAppButton).toBeVisible({ timeout: TIMEOUTS.default });
   }
 
   //helper for fields that need to have select a pre existing option
   async fillDropdown(locator: Locator, value: string) {
     await locator.click();
     await locator.pressSequentially(value, { delay: 50 }); 
     
-    //Wait for the dropdown 
-    await expect(locator).toHaveValue(value, { timeout: 5000 });
+    //wait for the dropdown 
+    await expect(locator).toHaveValue(value, { timeout: TIMEOUTS.modal });
     
     await locator.press('Enter');
   }
 
-async createApp(appName: string, repoUrl: string, repoPath: string) {
+  async createApp(appName: string, repoUrl: string, repoPath: string) {
     await this.newAppButton.click();
     
     //handle the "failed to load data" banner if it appears inside the slide-out panel
     const errorBanner = this.page.getByText('try again');
     try {
-      //wait 3 secs
-      await errorBanner.waitFor({ state: 'visible', timeout: 3000 });
+      await errorBanner.waitFor({ state: 'visible', timeout: TIMEOUTS.short });
       await errorBanner.click(); 
     } catch (error) {
       //banner didn't appear so just continue
     }
 
-    await this.page.getByText('Loading...').first().waitFor({ state: 'hidden', timeout: 15000 });
+    await this.page.getByText('Loading...').first().waitFor({ state: 'hidden', timeout: TIMEOUTS.default });
 
     await this.appNameInput.fill(appName);
     await this.fillDropdown(this.projectInput, 'default'); 
@@ -88,8 +99,9 @@ async createApp(appName: string, repoUrl: string, repoPath: string) {
     //dest
     await this.clusterUrlInput.fill('https://kubernetes.default.svc');
     
-    //deploy
-    await this.namespaceInput.fill('openshift-gitops');
+    //deploy to namespace
+    await this.namespaceInput.fill('openshift-gitops'); 
+
     await this.createButton.click();
   }
 
@@ -98,38 +110,47 @@ async createApp(appName: string, repoUrl: string, repoPath: string) {
     await this.page.getByPlaceholder(/Search applications/i).fill(appName);
 
     const appContainer = this.page.locator('.white-box, .argo-table-list__row').filter({ hasText: appName });
-    await appContainer.waitFor({ state: 'visible', timeout: 20000 });
-    await expect(appContainer.getByText(/OutOfSync|Out of Sync/i).first()).toBeVisible({ timeout: 45000 });
-    //safe to open the panel
+    await appContainer.waitFor({ state: 'visible', timeout: TIMEOUTS.load });
+    
+    //critical cross-version fix: wait for Argo CD to finish its initial Git clone
+    //if we open the Sync panel before this happens, the resources list will be empty!
+    await expect(appContainer.getByText(/OutOfSync|Out of Sync/i).first()).toBeVisible({ timeout: TIMEOUTS.sync });
+
+    //now it is safe to open the panel
     await appContainer.getByText('Sync', { exact: true }).click();
     
-    //click 'all' 
+    //click 'all' first to ensure all resource checkboxes are ticked across newer Argo CD versions
     const allLink = this.page.getByRole('link', { name: 'all', exact: true });
     try {
-      await allLink.waitFor({ state: 'visible', timeout: 5000 });
+      await allLink.waitFor({ state: 'visible', timeout: TIMEOUTS.modal });
       await allLink.click();
     } catch (error) {
-      // all link didn't appear within 5 sec
+      //'all' link didn't appear which is normal for this version so do nothing.
     }
-
-    //wait for the manifests to render on the panel
-    await expect(this.page.getByText(expectedResource).first()).toBeVisible({ timeout: 30000 });
+    
+    //wait for the manifests to render on the panel (generous timeout for slower FIPS clusters)
+    await expect(this.page.getByText(expectedResource).first()).toBeVisible({ timeout: TIMEOUTS.render });
 
     //click the main sync button
     await this.page.getByRole('button', { name: /^synchronize$/i }).first().click();
 
     //wait for the panel to close 
-    await expect(this.page.getByText('SYNCHRONIZE RESOURCES')).toBeHidden({ timeout: 15000 });
+    await expect(this.page.getByText('SYNCHRONIZE RESOURCES')).toBeHidden({ timeout: TIMEOUTS.panel });
   }
 
   async verifyStatus(appName: string) {
-    //re-apply search filter just in case
     await this.page.getByPlaceholder(/Search applications/i).fill(appName);
     const appContainer = this.page.locator('.white-box, .argo-table-list__row').filter({ hasText: appName });
     
-    //90 secs
-    await expect(appContainer.getByText(/synced/i)).toBeVisible({ timeout: 90000 });
-    await expect(appContainer.getByText(/healthy/i)).toBeVisible({ timeout: 90000 });
+    //pass the message
+    await expect(
+      appContainer.getByText(/Sync failed/i), 
+      `Argo CD failed to sync the application manifests for ${appName}.`
+    ).toBeHidden({ timeout: TIMEOUTS.panel });
+
+    //if it didn't fail to wait for success states
+    await expect(appContainer.getByText(/synced/i)).toBeVisible({ timeout: TIMEOUTS.status });
+    await expect(appContainer.getByText(/healthy/i)).toBeVisible({ timeout: TIMEOUTS.status });
   }
 
   async openApplication(appName: string) {
@@ -141,10 +162,10 @@ async createApp(appName: string, repoUrl: string, repoPath: string) {
                              .filter({ hasText: appName })
                              .getByRole('link', { name: appName });
                              
-    await appLink.waitFor({ state: 'visible', timeout: 15000 });
+    await appLink.waitFor({ state: 'visible', timeout: TIMEOUTS.default });
     await appLink.click();
     
     //wait for the URL to change to the details page to ensure the click worked
-    await expect(this.page).toHaveURL(/.*\/applications\/.*\/.*/, { timeout: 15000 });
+    await expect(this.page).toHaveURL(/.*\/applications\/.*\/.*/, { timeout: TIMEOUTS.default });
   }
 }
