Skip to content

fix(test): resolve flaky DSCrudAndBindings_Spec after-hook cleanup failure#41830

Open
subrata71 wants to merge 1 commit into
releasefrom
fix/app-15242-flaky-dscrudbindings-spec
Open

fix(test): resolve flaky DSCrudAndBindings_Spec after-hook cleanup failure#41830
subrata71 wants to merge 1 commit into
releasefrom
fix/app-15242-flaky-dscrudbindings-spec

Conversation

@subrata71
Copy link
Copy Markdown
Collaborator

@subrata71 subrata71 commented May 20, 2026

Description

The Cypress spec DSCrudAndBindings_Spec.ts (Git > ExistingApps > v1.9.24) fails deterministically in EE CI with a 30s timeout in the after() hook when DeleteApplication cannot find the app card on the home page.

Root cause: After returning from deployed mode via NavigateToHomeDirectly(), the correct workspace is not selected on the home page. DeleteApplication assumes the app card is already visible, but it isn't — unlike DeleteWorkspace, which calls SelectWorkspace internally.

Three fixes applied:

  1. after() hook — Added homePage.SelectWorkspace(workspaceName) before DeleteApplication to ensure the correct workspace is visible
  2. Merged two it() blocks — The two tests shared deployed-mode state with no navigation reset between them. Merging eliminates the inter-test state dependency that caused the after all hook to fire between tests
  3. Slider focus assertions — Added .should("be.visible") and .should("be.focused") to the slider interaction to guard against undetected focus failures

Evidence:

Fixes https://linear.app/appsmith/issue/APP-15242/fix-flaky-cypress-spec-dscrudandbindings-spects-after-hook-cleanup

Automation

/ok-to-test tags="@tag.Git"

🔍 Cypress test results

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/26168633701
Commit: 2466c67
Cypress dashboard.
Tags: @tag.Git
Spec:


Wed, 20 May 2026 14:43:45 UTC

…ilure

The after() hook failed deterministically because DeleteApplication could
not find the app card after NavigateToHomeDirectly — the correct workspace
was not selected. Also merged two it() blocks that shared deployed-mode
state without a reset, and added focus assertions on the slider interaction.

Fixes https://linear.app/appsmith/issue/APP-15242/fix-flaky-cypress-spec-dscrudandbindings-spects-after-hook-cleanup
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Walkthrough

A Cypress Git regression test consolidates two test cases into one flow, removes redundant comments, adds explicit visibility and focus assertions to slider interactions, and changes cleanup navigation to workspace selection instead of home navigation.

Changes

Git Regression Test Restructuring

Layer / File(s) Summary
Test case consolidation and flow merge
app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts
First test case renamed to reflect that it now covers CRUD pages plus widgets/bindings validation in a single flow; second test case removed and its validation merged as an inline section marker within the first test. "Assert table data" comment removed at the CRUD boundary.
Slider interaction assertion enhancements
app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts
Slider step assertions add explicit visibility check before focus, then assert the focused state after the focus action.
Cleanup navigation update
app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts
Cleanup logic changes from NavigateToHomeDirectly() to SelectWorkspace() before deleting the application and workspace.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

Two tests became one, a streamlined flow ✨
With sliders now seeing what they should know
And cleanup that picks the workspace with care
Git regression's better, refactored with flair 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing a flaky Cypress test's after-hook cleanup issue, which is the primary purpose of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description is comprehensive, well-structured, and follows the template with all required sections including issue reference, detailed root cause analysis, specific fixes applied, and evidence of the problem.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/app-15242-flaky-dscrudbindings-spec

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@subrata71 subrata71 self-assigned this May 20, 2026
@subrata71 subrata71 added the ok-to-test Required label for CI label May 20, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts (1)

242-249: ⚖️ Poor tradeoff

Consider refactoring cleanup logic to avoid after() hook.

The coding guidelines specify "Avoid using after and aftereach in test cases." While this cleanup logic is necessary and the current fix (line 246) correctly addresses the immediate flaky test issue, consider moving cleanup operations to a different pattern in future refactoring (e.g., explicit cleanup within the test or using a custom command).

Note: This is pre-existing code; the current PR appropriately focuses on the minimal fix for the flaky test. As per coding guidelines, after and afterEach hooks should be avoided in test cases.

🤖 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
`@app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts`
around lines 242 - 249, The test uses an after() hook for cleanup
(gitSync.DeleteDeployKey, agHelper.WaitUntilAllToastsDisappear,
deployMode.NavigateToHomeDirectly, homePage.SelectWorkspace,
homePage.DeleteApplication, homePage.DeleteWorkspace); refactor by removing the
after() block and moving these cleanup steps into the test flow or a reusable
custom Cypress command (e.g., cy.cleanupAppRepo) so cleanup is invoked
explicitly at the end of the test where needed; ensure the new pattern calls the
same methods in the same order and preserves error handling/awaiting of toasts.
🤖 Prompt for all review comments with 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.

Nitpick comments:
In
`@app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts`:
- Around line 242-249: The test uses an after() hook for cleanup
(gitSync.DeleteDeployKey, agHelper.WaitUntilAllToastsDisappear,
deployMode.NavigateToHomeDirectly, homePage.SelectWorkspace,
homePage.DeleteApplication, homePage.DeleteWorkspace); refactor by removing the
after() block and moving these cleanup steps into the test flow or a reusable
custom Cypress command (e.g., cy.cleanupAppRepo) so cleanup is invoked
explicitly at the end of the test where needed; ensure the new pattern calls the
same methods in the same order and preserves error handling/awaiting of toasts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d6ab97b4-6f23-4f74-8ac6-eeda53b3779d

📥 Commits

Reviewing files that changed from the base of the PR and between 89a4240 and 2466c67.

📒 Files selected for processing (1)
  • app/client/cypress/e2e/Regression/ClientSide/Git/ExistingApps/v1.9.24/DSCrudAndBindings_Spec.ts

@subrata71 subrata71 requested a review from sondermanish May 20, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants