Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 3 additions & 1 deletion frontend/e2e/pages/yaml-editor-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export class YamlEditorPage extends BasePage {
}

async waitForEditorReady(): Promise<void> {
await expect(this.codeEditor).toBeVisible({ timeout: 30_000 });
const mounting = this.page.getByTestId('code-editor-mounting');
await expect(mounting.or(this.codeEditor)).toBeVisible({ timeout: 60_000 });
await expect(this.codeEditor).toBeVisible({ timeout: 60_000 });
}

async waitForSidebarLoaded(): Promise<void> {
Expand Down
153 changes: 102 additions & 51 deletions frontend/e2e/tests/console/app/poll-console-updates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,113 +19,164 @@ const PLUGIN_MANIFEST_DEFAULT = { name: PLUGIN_NAME, version: '0.0.0' };
const PLUGIN_MANIFEST_DEFAULT2 = { name: PLUGIN_NAME2, version: '0.0.0' };
const PLUGIN_MANIFEST_NEW_VERSION = { name: PLUGIN_NAME, version: '1.0.0' };

// The component needs two poll cycles to initialize its prev/current refs before
// it can detect changes. Tests wait for two check-updates responses (baselineReady)
// before switching the mocked payload.
test.describe('PollConsoleUpdates', { tag: ['@admin'] }, () => {
test('triggers the console update toast when consoleCommit changes', async ({ page }) => {
let resolveFirst: () => void;
const firstIntercepted = new Promise<void>((r) => {
resolveFirst = r;
let payload = UPDATES_DEFAULT;

await page.route(CHECK_UPDATES_URL, (route) => route.fulfill({ json: payload }));

// Start listening for responses BEFORE navigating so we don't miss the first poll.
let responseCount = 0;
const baselineReady = new Promise<void>((resolve) => {
page.on('response', (resp) => {
if (resp.url().includes('/api/check-updates') && resp.status() === 200) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same comment re. import type at the top instead of inline type imports

responseCount++;
if (responseCount >= 2) {
resolve();
}
}
});
});

await page.route(CHECK_UPDATES_URL, async (route) => {
await route.fulfill({ json: UPDATES_DEFAULT });
resolveFirst();
});
await page.goto('/');
await firstIntercepted;
await baselineReady;

await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_NEW_COMMIT }),
);
// Switch the payload — the next poll will see a different consoleCommit.
payload = UPDATES_NEW_COMMIT;

await expect(page.getByTestId('refresh-web-console')).toBeVisible({ timeout: 300_000 });
});

test('triggers the console update toast when a plugin is added', async ({ page }) => {
let resolveDefault: () => void;
const defaultIntercepted = new Promise<void>((r) => {
resolveDefault = r;
let updatesPayload = UPDATES_DEFAULT;
let manifestAbort = true;

await page.route(CHECK_UPDATES_URL, (route) => route.fulfill({ json: updatesPayload }));
await page.route(PLUGIN_MANIFEST_URL, (route) => {
if (manifestAbort) {
return route.abort();
}
return route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT });
});

await page.route(CHECK_UPDATES_URL, async (route) => {
await route.fulfill({ json: UPDATES_DEFAULT });
resolveDefault();
let responseCount = 0;
const baselineReady = new Promise<void>((resolve) => {
page.on('response', (resp) => {
if (resp.url().includes('/api/check-updates') && resp.status() === 200) {
responseCount++;
if (responseCount >= 2) {
resolve();
}
}
});
});

await page.goto('/');
await defaultIntercepted;
await baselineReady;

await page.route(PLUGIN_MANIFEST_URL, (route) => route.abort());
await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_NEW_PLUGIN }),
);
// Add a plugin whose manifest endpoint is erroring — toast should NOT appear yet.
updatesPayload = UPDATES_NEW_PLUGIN;

await expect(page.getByTestId('refresh-web-console')).not.toBeAttached({
timeout: 10_000,
});

await page.route(PLUGIN_MANIFEST_URL, (route) =>
route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT }),
);
// Make the manifest endpoint succeed — toast should now appear.
manifestAbort = false;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

await expect(page.getByTestId('refresh-web-console')).toBeVisible({ timeout: 300_000 });
});

test('triggers the console update toast when a plugin is added and a different plugin endpoint is erroring', async ({
page,
}) => {
await page.route(PLUGIN_MANIFEST_URL, (route) => route.abort());
await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_NEW_PLUGIN }),
);
await page.goto('/');
let updatesPayload = UPDATES_NEW_PLUGIN;
let manifest1Abort = true;
let manifest2Abort = true;

// Wait for the first check-updates poll to establish baseline state
await page.waitForResponse((resp) => resp.url().includes('/api/check-updates'));
await page.route(CHECK_UPDATES_URL, (route) => route.fulfill({ json: updatesPayload }));
await page.route(PLUGIN_MANIFEST_URL, (route) => {
if (manifest1Abort) {
return route.abort();
}
return route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT });
});
await page.route(PLUGIN_MANIFEST_URL2, (route) => {
if (manifest2Abort) {
return route.abort();
}
return route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT2 });
});

let responseCount = 0;
const baselineReady = new Promise<void>((resolve) => {
page.on('response', (resp) => {
if (resp.url().includes('/api/check-updates') && resp.status() === 200) {
responseCount++;
if (responseCount >= 2) {
resolve();
}
}
});
});

await page.goto('/');
await baselineReady;

await expect(page.getByTestId('refresh-web-console')).not.toBeAttached({
timeout: 10_000,
});

// Now introduce a second plugin — plugin1 manifest still errors, plugin2 manifest also errors
await page.route(PLUGIN_MANIFEST_URL2, (route) => route.abort());
await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_NEW_PLUGIN2 }),
);
// Introduce a second plugin — both manifest endpoints are still erroring.
updatesPayload = UPDATES_NEW_PLUGIN2;

// Wait for the app to poll and see the new plugin list
// Wait for the app to poll and see the new plugin list.
await page.waitForResponse((resp) => resp.url().includes('/api/check-updates'));

await expect(page.getByTestId('refresh-web-console')).not.toBeAttached({
timeout: 10_000,
});

// Make plugin2 manifest succeed — toast should appear
await page.route(PLUGIN_MANIFEST_URL2, (route) =>
route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT2 }),
);
// Make plugin2 manifest succeed — toast should appear.
manifest2Abort = false;

await expect(page.getByTestId('refresh-web-console')).toBeVisible({ timeout: 300_000 });
});

test('triggers the console update toast when a plugin is removed', async ({ page }) => {
await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_NEW_PLUGIN }),
);
let updatesPayload = UPDATES_NEW_PLUGIN;

await page.route(CHECK_UPDATES_URL, (route) => route.fulfill({ json: updatesPayload }));
await page.route(PLUGIN_MANIFEST_URL, (route) =>
route.fulfill({ json: PLUGIN_MANIFEST_DEFAULT }),
);
await page.goto('/');

await page.waitForResponse((resp) => resp.url().includes('/api/check-updates'));
let responseCount = 0;
const baselineReady = new Promise<void>((resolve) => {
page.on('response', (resp) => {
if (resp.url().includes('/api/check-updates') && resp.status() === 200) {
responseCount++;
if (responseCount >= 2) {
resolve();
}
}
});
});

await page.route(CHECK_UPDATES_URL, (route) =>
route.fulfill({ json: UPDATES_DEFAULT }),
);
await page.goto('/');
await baselineReady;

// Remove the plugin from the list.
updatesPayload = UPDATES_DEFAULT;

await expect(page.getByTestId('refresh-web-console')).toBeVisible({ timeout: 300_000 });
});

test('triggers the console update toast when a plugin version changes', async ({ page }) => {
// Serve the old version for the first 2 manifest fetches, then switch to the new version.
// Serve the old version for the first few manifest fetches, then switch to the new version.
// The component needs at least one render cycle with the old version recorded as
// prevPluginManifestsData before it can detect the version change.
let manifestFetchCount = 0;
Expand Down
2 changes: 1 addition & 1 deletion frontend/e2e/tests/console/crud/other-routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const routes: RouteConfig[] = [
{
path: '/k8s/all-namespaces/events',
assertLoaded: async (page) => {
await expect(page.getByRole('row').first()).toBeVisible();
await expect(page.getByTestId('event-totals')).toBeVisible();
},
},
{
Expand Down
16 changes: 14 additions & 2 deletions frontend/e2e/tests/console/favorites/favorites.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@ import { test, expect } from '../../../fixtures';
import { warmupSPA } from '../../../pages/base-page';

test.describe('Favorites', { tag: ['@admin'] }, () => {
test('adds, displays, removes, and limits favorites', async ({ page }) => {
test('adds, displays, removes, and limits favorites', async ({ page, k8sClient }) => {
const sidebar = page.locator('#page-sidebar');

await test.step('Clear any stale favorites from prior runs', async () => {
try {
await k8sClient.patchConfigMap(
'user-settings-kubeadmin',
'openshift-console-user-settings',
{ 'console.favorites': '[]' },
);
} catch {
// ConfigMap may not exist yet
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});

await warmupSPA(page);

await test.step('Verify no favorites message when none are added', async () => {
await sidebar.getByRole('button', { name: 'Favorites' }).click();
await expect(page.getByTestId('no-favorites-message')).toBeVisible();
await expect(page.getByTestId('no-favorites-message')).toBeVisible({ timeout: 30_000 });
});

await test.step('Open Add to Favorites modal', async () => {
Expand Down
4 changes: 3 additions & 1 deletion frontend/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const chromeArgs = [
'--no-sandbox',
'--disable-setuid-sandbox',
'--disable-background-networking',
'--disable-background-timer-throttling',
'--disable-client-side-phishing-detection',
'--disable-default-apps',
'--disable-extensions',
Expand All @@ -49,6 +50,7 @@ export default defineConfig({
testDir: './e2e/tests',
testMatch: '**/*.spec.ts',
forbidOnly: isCI,
globalTimeout: parseInt(process.env.GLOBAL_TIMEOUT_MS || '0', 10) || (isCI ? 110 * 60 * 1000 : 0),

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant config file with line numbers.
sed -n '1,160p' frontend/playwright.config.ts | cat -n

Repository: openshift/console

Length of output: 5460


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,160p' frontend/playwright.config.ts | cat -n

Repository: openshift/console

Length of output: 5460


🏁 Script executed:

#!/bin/bash
set -euo pipefail
pwd
ls

Repository: openshift/console

Length of output: 844


🌐 Web query:

Playwright globalTimeout 0 meaning disable timeout documentation

💡 Result:

In Playwright, setting globalTimeout to 0 means that the timeout behavior is disabled, allowing the test suite to run for an unlimited amount of time [1][2]. The globalTimeout configuration option defines the maximum time in milliseconds that the entire test suite is allowed to run [1]. By default, there is no global timeout (i.e., it is unlimited) [3][4]. When you explicitly set globalTimeout to 0, you are effectively choosing to disable any automatic termination of the test run based on duration, which can be useful if you do not want Playwright to stop the process prematurely [1][2]. This is distinct from other timeout settings in Playwright, where specific values are used to enforce constraints on individual tests, actions, or hooks to prevent resource wastage [3][4].

Citations:


Honor an explicit zero timeout override.

GLOBAL_TIMEOUT_MS=0 is treated as unset in CI because of ||, so the 110-minute default still applies. Check for an explicit env value before falling back.

🤖 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/playwright.config.ts` at line 52, Update the globalTimeout
configuration expression to distinguish an explicitly provided GLOBAL_TIMEOUT_MS
value of "0" from an unset or invalid value. Use the parsed environment value
whenever GLOBAL_TIMEOUT_MS is explicitly present, and only apply the isCI-based
110-minute default when the variable is absent.

Source: Path instructions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the ternary/or combination is somewhat complex, maybe GLOBAL_TIMEOUT_MS can be set in the test-prow-playwright-e2e script instead?

retries: isCI ? 1 : 0,
timeout: 120_000,
reporter: isCI
Expand Down Expand Up @@ -84,7 +86,7 @@ export default defineConfig({
},
},

workers: process.env.WORKERS ? parseInt(process.env.WORKERS, 10) : isCI ? 1 : undefined,
workers: process.env.WORKERS ? parseInt(process.env.WORKERS, 10) : undefined,

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.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
rg -n 'WORKERS|workers' frontend test-prow-playwright-e2e.sh
rg -n '"`@playwright/test`"|`@playwright/test`@' package.json yarn.lock frontend/package.json frontend/yarn.lock 2>/dev/null || true

Repository: openshift/console

Length of output: 694


Preserve WORKERS syntax instead of coercing it with parseInt.
parseInt accepts malformed values like 2foo and strips supported Playwright percentage forms such as 50%. Validate the env var first, or pass through a valid worker string unchanged.

🤖 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/playwright.config.ts` at line 88, Update the workers configuration
to preserve valid WORKERS values, including Playwright percentage strings,
instead of coercing them with parseInt. Validate the environment value before
use and pass the original string through unchanged when valid, while retaining
undefined when WORKERS is unset and rejecting malformed values such as “2foo”.

Source: Path instructions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

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.

nit: since we are not supporting percentage values we can be stricter and use Number() that is stricter than parseInt()

Suggested change
workers: process.env.WORKERS ? parseInt(process.env.WORKERS, 10) : undefined,
workers: process.env.WORKERS ? Number(process.env.WORKERS) || undefined : undefined,


projects: [
{
Expand Down
2 changes: 2 additions & 0 deletions test-prow-playwright-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export BRIDGE_BASE_ADDRESS="$(oc get consoles.config.openshift.io cluster -o jso

./contrib/create-user.sh

export WORKERS="${WORKERS:-2}"

pushd frontend

SCENARIO="${1:-e2e}"
Expand Down