Skip to content

OCPBUGS-82512: Fix knative e2e test failures caused by createRoot timing #16289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sg00dwin wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −4
Open

OCPBUGS-82512: Fix knative e2e test failures caused by createRoot timing #16289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion frontend/packages/dev-console/integration-tests/support/pages/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,11 @@ export const navigateTo = (opt: devNavigationMenu) => {

export const projectNameSpace = {
clickProjectDropdown: () => {
cy.byLegacyTestID('namespace-bar-dropdown').find('button').first().click();
cy.byLegacyTestID('namespace-bar-dropdown', { timeout: 30000 })
.should('exist')
.find('button')
.first()
.click();
},
selectCreateProjectOption: () => {
cy.document().then((doc) => {
Expand Down
3 changes: 1 addition & 2 deletions frontend/packages/knative-plugin/integration-tests/features/e2e/knative-ci.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Disabled due to createRoot concurrent rendering failures (OCPBUGS-82512)
@knative @smoke @to-do
@knative @smoke
Feature: Perform actions on knative service and revision
As a user, I want to perform edit or delete operations on knative revision in topology page

Expand Down
5 changes: 4 additions & 1 deletion frontend/packages/topology/integration-tests/support/pages/topology/topology-page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,10 @@ export const topologyPage = {
cy.get(`[data-test-action="${action}"] button`).click();
},
getNode: (nodeName: string) => {
return cy.get(topologyPO.graph.nodeLabel).should('be.visible').contains(nodeName);
return cy
.get(topologyPO.graph.nodeLabel, { timeout: 30000 })
.should('be.visible')
.contains(nodeName);
Comment on lines +292 to +295
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Cypress, for cy.get(selector, { timeout: 30000 }).contains(text), does .contains(text)inherit the 30s timeout fromget, or does it use defaultCommandTimeout unless timeout is explicitly passed?

💡 Result:

No, .contains(text) does not inherit the 30s timeout from cy.get(selector, { timeout: 30000 }). It uses defaultCommandTimeout unless a timeout is explicitly passed to .contains. Both cy.get and .contains are queries that retry until they resolve or timeout. The timeout option set on cy.get applies specifically to waiting for the selector elements to appear in the DOM (up to 30s). Once cy.get resolves (yields elements), .contains then searches within those yielded elements for the text. Since .contains has no explicit timeout, it uses the default defaultCommandTimeout (typically 4s) for its own retry logic—retrying the text search within the subject until found or that default timeout expires. This is evidenced by official Cypress documentation: .contains explicitly states its timeout defaults to defaultCommandTimeout [6,7,10]. Query chains retry from the top using the initiating command's timeout for the entire chain's assertions, but each query command (like .contains) has its own timeout for its resolution [11,12,24]. Examples consistently show setting timeout on the first query affects chained assertions, but child queries like .contains require their own timeout option for extended waiting [16].

Citations:

🏁 Script executed:

#!/bin/bash
# Verify the file exists and check the actual code at the specified lines
if [ -f "frontend/packages/topology/integration-tests/support/pages/topology/topology-page.ts" ]; then
  echo "=== File exists, showing context around lines 292-295 ==="
  sed -n '288,300p' "frontend/packages/topology/integration-tests/support/pages/topology/topology-page.ts" | cat -n
else
  echo "File not found. Searching for topology-page.ts..."
  find . -name "topology-page.ts" -type f
fi

Repository: openshift/console

Length of output: 675

🌐 Web query:

Cypress cy.contains(selector, text, options) - Does this syntax exist and does the timeout option work on it?

💡 Result:

Yes, the syntax Cypress cy.contains(selector, text, options) exists, and the timeout option works on it. The official Cypress documentation explicitly lists this syntax and confirms that the options object includes a 'timeout' property, which overrides the defaultCommandTimeout to specify how long to wait for cy.contains to resolve before timing out.

Citations:

Timeout on text match must be explicit to prevent flakiness.

The 30-second timeout on cy.get() applies only to finding label elements; once any label appears, contains(nodeName) runs with the default 4-second timeout for text matching. This leaves a window where the correct node name may not be found before timeout, defeating the purpose of the 30-second window.

Apply the timeout directly to the text-matching operation:

Proposed fix 
   return cy
-    .get(topologyPO.graph.nodeLabel, { timeout: 30000 })
-    .should('be.visible')
-    .contains(nodeName);
+    .contains(topologyPO.graph.nodeLabel, nodeName, { timeout: 30000 })
+    .should('be.visible');
📝 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
return cy
.get(topologyPO.graph.nodeLabel, { timeout: 30000 })
.should('be.visible')
.contains(nodeName);
return cy
.contains(topologyPO.graph.nodeLabel, nodeName, { timeout: 30000 })
.should('be.visible');
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@frontend/packages/topology/integration-tests/support/pages/topology/topology-page.ts`
around lines 292 - 295, The current chain uses a 30s timeout on
cy.get(topologyPO.graph.nodeLabel) but not on the subsequent text match, so
change the text match to use the explicit timeout; update the call that
currently does .contains(nodeName) to .contains(nodeName, { timeout: 30000 })
(or alternatively use cy.contains(topologyPO.graph.nodeLabel, nodeName, {
timeout: 30000 })) so the node label text matching shares the same 30s window.

},
getNodeLabel: (nodeName: string) => {
return cy.get(topologyPO.graph.selectNodeLabel).should('be.visible').contains(nodeName);
Expand Down