Skip to content

[RFR] Revert PR 1679 #1682

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

Merged
merged 1 commit into from
Nov 3, 2025
Merged

[RFR] Revert PR 1679 #1682

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
5 changes: 4 additions & 1 deletion cypress/e2e/models/migration/applicationinventory/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ export class Application {
const itemsPerPage = 100;
if (forceReload) {
cy.visit(Application.fullUrl, { timeout: 35 * SEC }).then((_) => {
cy.get("h1", { timeout: 10 * SEC }).should("contain", applicationInventory);
// Bug MTA-3812 Application Inventory page takes long to load
// TODO: Wait of 10s to be reduced after above bug is fixed
cy.wait(10 * SEC);
cy.get("h1").should("contain", applicationInventory);
Comment on lines +162 to +165
Copy link

@coderabbitai coderabbitai bot Sep 23, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid fixed waits; keep an explicit timeout on the header to reduce flakiness

A hard 10s wait slows tests and the removal of the explicit timeout on the header weakens resilience. Default Cypress timeout (~4s) may still flake after the 10s wait. Keep the temporary wait if needed for MTA-3812, but reintroduce a longer explicit timeout consistent with the non-reload path.

Apply this diff:

-                cy.wait(10 * SEC);
-                cy.get("h1").should("contain", applicationInventory);
+                // Temporary wait for MTA-3812; keep explicit timeout to reduce flakiness
+                cy.wait(10 * SEC);
+                cy.get("h1", { timeout: 60 * SEC }).should("contain", applicationInventory);

Optional (preferable longer-term): replace the fixed wait with an event-based wait (e.g., intercept the apps API or wait for a stable UI control like items-per-page toggle to be enabled) before asserting the header.

📝 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
// Bug MTA-3812 Application Inventory page takes long to load
// TODO: Wait of 10s to be reduced after above bug is fixed
cy.wait(10 * SEC);
cy.get("h1").should("contain", applicationInventory);
// Bug MTA-3812 Application Inventory page takes long to load
// TODO: Wait of 10s to be reduced after above bug is fixed
// Temporary wait for MTA-3812; keep explicit timeout to reduce flakiness
cy.wait(10 * SEC);
cy.get("h1", { timeout: 60 * SEC }).should("contain", applicationInventory);
🤖 Prompt for AI Agents 
In cypress/e2e/models/migration/applicationinventory/application.ts around lines
162 to 165, remove the flaky pattern of a fixed wait without an explicit
assertion timeout: keep the temporary cy.wait(10 * SEC) if MTA-3812 requires it,
but change the header assertion to include a longer explicit timeout (e.g.,
cy.get("h1", { timeout: 15000 }).should("contain", applicationInventory)) so the
check is resilient; optionally replace the fixed wait with an event-based wait
(intercept the apps API or wait for a specific UI control to become enabled) as
a longer-term improvement.

selectItemsPerPage(itemsPerPage);
});
return;
Expand Down