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
81 changes: 78 additions & 3 deletions cypress.config.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,79 @@
import { defineConfig } from 'cypress';
import { plugin as cypressGrepPlugin } from '@cypress/grep/plugin';
import { execFileSync } from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import * as console from 'console';

const ARTIFACTS_DIR = './gui_test_screenshots/artifacts';
const OLS_NAMESPACE = 'openshift-lightspeed';
const OC_TIMEOUT = 30000;

const CLUSTER_RESOURCES = [
'pods',
'services',
'deployments',
'replicasets',
'routes',
'rolebindings',
'serviceaccounts',
'olsconfig',
'clusterserviceversion',
'installplan',
'configmap',
];

function runOC(args: string[], kubeconfigPath: string): string | null {
const argv = kubeconfigPath ? [...args, '--kubeconfig', kubeconfigPath] : args;
try {
return execFileSync('oc', argv, {
encoding: 'utf-8',
timeout: OC_TIMEOUT,
});
} catch (e: unknown) {
console.error(`oc ${args.slice(0, 3).join(' ')} failed: ${e}`);
return null;
}
}

function gatherClusterArtifacts(kubeconfigPath: string) {
const clusterDir = path.join(ARTIFACTS_DIR, 'cluster');
const podLogsDir = path.join(clusterDir, 'podlogs');
fs.mkdirSync(podLogsDir, { recursive: true });

for (const resource of CLUSTER_RESOURCES) {
const output = runOC(['get', resource, '-n', OLS_NAMESPACE, '-o', 'yaml'], kubeconfigPath);
if (output) {
fs.writeFileSync(path.join(clusterDir, `${resource}.yaml`), output);
}
}

// Pod logs
const podsJson = runOC(['get', 'pods', '-n', OLS_NAMESPACE, '-o', 'json'], kubeconfigPath);
if (podsJson) {
try {
const pods = JSON.parse(podsJson);
for (const pod of pods.items || []) {
const podName = pod.metadata?.name;
const containers = (pod.spec?.containers || []).map((c: { name: string }) => c.name);
for (const container of containers) {
const logs = runOC(
['logs', `pod/${podName}`, '-c', container, '-n', OLS_NAMESPACE],
kubeconfigPath,
);
if (logs) {
fs.writeFileSync(path.join(podLogsDir, `${podName}-${container}.log`), logs);
}
}
}
} catch (e: unknown) {
console.error(`Failed to parse pod JSON: ${e}`);
}
}

console.log(`Cluster artifacts gathered in ${clusterDir}`);
}

export default defineConfig({
screenshotsFolder: './gui_test_screenshots/cypress/screenshots',
screenshotOnRunFailure: true,
Expand Down Expand Up @@ -72,13 +143,17 @@ export default defineConfig({
console.table(data);
return null;
},
gatherClusterArtifacts() {
gatherClusterArtifacts(config.env.KUBECONFIG_PATH || '');
return null;
Comment on lines +146 to +148
Copy link
Copy Markdown

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

Avoid collecting artifacts from an unintended cluster context.

At Line 147, falling back to '' means oc can use ambient/default kubeconfig, which may export data from the wrong cluster. Skip collection (or fail fast) when KUBECONFIG_PATH is not set.

Proposed patch
         gatherClusterArtifacts() {
-          gatherClusterArtifacts(config.env.KUBECONFIG_PATH || '');
+          const kubeconfigPath = config.env.KUBECONFIG_PATH;
+          if (!kubeconfigPath) {
+            console.warn('Skipping cluster artifact gathering: KUBECONFIG_PATH is not set');
+            return null;
+          }
+          gatherClusterArtifacts(kubeconfigPath);
           return null;
         },
📝 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
gatherClusterArtifacts() {
gatherClusterArtifacts(config.env.KUBECONFIG_PATH || '');
return null;
gatherClusterArtifacts() {
const kubeconfigPath = config.env.KUBECONFIG_PATH;
if (!kubeconfigPath) {
console.warn('Skipping cluster artifact gathering: KUBECONFIG_PATH is not set');
return null;
}
gatherClusterArtifacts(kubeconfigPath);
return null;
},
🤖 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 `@cypress.config.ts` around lines 146 - 148, The current gatherClusterArtifacts
wrapper calls gatherClusterArtifacts(config.env.KUBECONFIG_PATH || '') which
passes an empty string and lets oc use the ambient kubeconfig; change it to
require a non-empty config.env.KUBECONFIG_PATH: check config.env.KUBECONFIG_PATH
inside the gatherClusterArtifacts wrapper and if it's falsy either skip
collection by returning null (and log a warning) or fail fast by throwing a
clear error; only call the inner gatherClusterArtifacts(path) when
config.env.KUBECONFIG_PATH is present to avoid using the default cluster
context.

},
});
cypressGrepPlugin(config);
on('after:spec', (spec: Cypress.Spec, results: CypressCommandLine.RunResult) => {
on('after:spec', (_spec: Cypress.Spec, results: CypressCommandLine.RunResult) => {
if (results && results.video) {
// Do we have failures for any retry attempts?
const failures = results.tests.some((test) =>
test.attempts.some((attempt) => attempt.state === 'failed'),
const failures = results.tests?.some((test) =>
test.attempts?.some((attempt) => attempt.state === 'failed'),
);
if (!failures && fs.existsSync(results.video)) {
Comment on lines +155 to 158
Copy link
Copy Markdown

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

🧩 Analysis chain

🏁 Script executed:

fd -t f "cypress.config.ts" | head -5

Repository: openshift/lightspeed-console

Length of output: 90


🏁 Script executed:

cat -n cypress.config.ts | sed -n '150,165p'

Repository: openshift/lightspeed-console

Length of output: 790


Default to "keep video" when test outcome is unknown to prevent unintended deletion of failure videos.

At line 155-157, results.tests?.some(...) can return undefined when results.tests is null or undefined. Since !undefined evaluates to true, the video deletion logic executes unintentionally in these cases. Update the assignment to use the nullish coalescing operator to treat unknown outcomes as "failures exist":

Proposed patch
-          const failures = results.tests?.some((test) =>
+          const failures = results.tests?.some((test) =>
             test.attempts?.some((attempt) => attempt.state === 'failed'),
-          );
-          if (!failures && fs.existsSync(results.video)) {
+          ) ?? true;
+          if (!failures && fs.existsSync(results.video)) {
             // Delete the video if the spec passed and no tests retried
             fs.unlinkSync(results.video);
           }
📝 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
const failures = results.tests?.some((test) =>
test.attempts?.some((attempt) => attempt.state === 'failed'),
);
if (!failures && fs.existsSync(results.video)) {
const failures = results.tests?.some((test) =>
test.attempts?.some((attempt) => attempt.state === 'failed'),
) ?? true;
if (!failures && fs.existsSync(results.video)) {
// Delete the video if the spec passed and no tests retried
fs.unlinkSync(results.video);
}
🤖 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 `@cypress.config.ts` around lines 155 - 158, The variable failures can be
undefined when results.tests is nullish, causing !failures to be true and
triggering video deletion unintentionally; update the assignment of failures
(the const named failures that uses results.tests?.some(...)) to default unknown
outcomes to "failures exist" (true) using the nullish coalescing operator so the
subsequent check if (!failures && fs.existsSync(results.video)) only deletes
videos when we know tests passed and not when test results are unknown.

// Delete the video if the spec passed and no tests retried
Expand Down
10 changes: 10 additions & 0 deletions cypress/support/e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ import { register } from '@cypress/grep';

register();

Cypress.on('uncaught:exception', (err) => {
// The console defines __load_plugin_entry__ via webpack module federation
// before loading plugin bundles, but race conditions during page load can
// cause the plugin script to execute before the global is set.
// This is a console-internal timing issue, not a test failure.
if (err.message?.includes('__load_plugin_entry__')) {
return false;
}
});
Comment on lines +7 to +15
Copy link
Copy Markdown

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

🧩 Analysis chain

🌐 Web query:

In Cypress, what are the recommended best practices for Cypress.on('uncaught:exception') handlers so specific known errors are suppressed without masking unrelated test failures?

💡 Result:

The recommended best practices for Cypress.on('uncaught:exception') handlers, as per official documentation, focus on selectively suppressing known errors while allowing unrelated failures to propagate [1][2]. 1. Use conditional logic based on error.message to ignore only specific known errors. Return false only for matching errors; otherwise, let the test fail naturally [1]. Example for global handler (e.g., in cypress/support/e2e.js): Cypress.on('uncaught:exception', (err, runnable) => { if (err.message.includes('list not defined')) { // known 3rd-party error return false; } // Acts as no-op for other errors, failing the test }); [1][3][4] 2. Prefer cy.on inside specific tests for scoped handling, avoiding global suppression. This automatically unbinds after the test [1][3]. Example: it('handles known error', () => { cy.on('uncaught:exception', (e, runnable) => { if (e.message.includes('Things went bad')) { return false; } }); // test code that triggers the error }); [1][3] 3. For one-time suppression, use cy.once [5]. 4. Avoid blanket return false, as it masks all errors [1][6]. Log errors for debugging [3]. 5. Place global handlers in support files; test-specific in 'it' blocks [1][4]. Note: Recent Cypress versions (e.g., 13+) may wrap app errors; check err.message for app details or err.name/err.cause [7][8]. Fix root causes in the app when possible [2].

Citations:


Narrow the exception filter to add specificity and logging.

At line 12, suppressing exceptions by message substring alone is a valid but minimal approach. While message-only filtering is standard in Cypress examples for known errors, adding a stack trace check makes the filter more specific and reduces the risk of accidentally masking unrelated failures. Additionally, logging when suppression occurs is an explicit best practice for debugging.

Proposed patch
 Cypress.on('uncaught:exception', (err) => {
-  if (err.message?.includes('__load_plugin_entry__')) {
+  const isPluginBootstrapRace =
+    err.message?.includes('__load_plugin_entry__') &&
+    /lightspeed-console-plugin|module federation|webpack/i.test(err.stack ?? '');
+
+  if (isPluginBootstrapRace) {
+    Cypress.log({
+      name: 'uncaught:exception:suppressed',
+      message: err.message,
+    });
     return false;
   }
 });
🤖 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 `@cypress/support/e2e.js` around lines 7 - 15, Update the
Cypress.on('uncaught:exception', ...) handler to narrow suppression by requiring
the plugin marker to appear in the error stack as well as the message, and emit
a log when you suppress it; specifically, in the callback for uncaught:exception
check both err.message?.includes('__load_plugin_entry__') and
err.stack?.includes('__load_plugin_entry__'), and when both match call
Cypress.log (or console.warn) with a short message and include the err object
before returning false to suppress.


// Collect browser console errors and warnings for output after each test
const browserLogs = [];

Expand Down
3 changes: 3 additions & 0 deletions tests/tests/lightspeed-install.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ describe('OLS UI', () => {
});

after(() => {
// Gather cluster artifacts before cleanup so resources still exist
cy.task('gatherClusterArtifacts');

if (Cypress.env('SKIP_OLS_SETUP')) {
cy.task('log', 'Skip OLS uninstall because CYPRESS_SKIP_OLS_SETUP is true');
} else {
Expand Down