Skip to content

fix: stabilize flaky TreeSelect onOptionChange cypress test#41709

Open
subrata71 wants to merge 2 commits intoreleasefrom
subrata71/fix-tree-select-flaky
Open

fix: stabilize flaky TreeSelect onOptionChange cypress test#41709
subrata71 wants to merge 2 commits intoreleasefrom
subrata71/fix-tree-select-flaky

Conversation

@subrata71
Copy link
Copy Markdown
Collaborator

@subrata71 subrata71 commented Apr 5, 2026

Description

Stabilizes test case #6 ("Verify onOptionChange with query") in Tree_Select_2_spec.ts, which was failing intermittently in CI with:

AssertionError: Timed out retrying after 30000ms: expected '<span.rc-tree-select-tree-title>' to be 'visible'

The dropdown's parent container had rc-tree-select-dropdown-hidden with display: none, meaning the click on the TreeSelect selector didn't open the dropdown.

Root cause: Two independent issues compounding:

  1. Action selector UI race condition — The onOptionChange handler was configured via 7 sequential clicks through cascading action selector dropdowns. Each dropdown transition is async; force: true on some clicks was an earlier attempt to work around this, but it only bypasses actionability checks, not content readiness. When any step failed silently, the handler was misconfigured.

  2. Dropdown click swallowed during widget remount — The TreeSelect component uses key={Math.random()} that changes when expandAll evaluates. During deploy-mode page hydration, the evaluation engine asynchronously dispatches updated props, triggering a full remount. A click landing during this remount window reaches a DOM element whose React fiber tree is being torn down, so the event goes unprocessed.

Fix:

  • Replace the fragile 7-step visual action selector setup with deterministic JS mode (ToggleJSMode + UpdatePropertyFieldValue), matching the pattern used by tests 7–12 in the same spec
  • After clicking the TreeSelect in deploy mode, verify the dropdown actually opened; if not, retry the click once (by then the remount has completed)

Automation

/ok-to-test tags="@tag.Widget, @tag.Select"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/24092241506
Commit: a64edc6
Cypress dashboard.
Tags: @tag.Widget, @tag.Select
Spec:


Tue, 07 Apr 2026 17:43:08 UTC

Communication

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

  • Yes
  • No

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Updated TreeSelect widget E2E test case. Enhanced onOptionChange callback test setup with refined configuration approach. Modified interaction logic to implement conditional retry mechanism based on dropdown visibility. Streamlined validation flow for option selection and alert messaging.

Replace fragile multi-step action selector UI setup with deterministic
JS mode configuration, and add a conditional retry for the deployed
TreeSelect dropdown click to handle widget remount timing during page
hydration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the Bug Something isn't working label Apr 5, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 5, 2026

Walkthrough

A Cypress E2E test for TreeSelect's onOptionChange callback was updated to use inline JavaScript mode instead of property-pane configuration, and the widget interaction logic was refactored to include conditional retry logic with visibility checks.

Changes

Cohort / File(s) Summary
TreeSelect Test Updates
app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_2_spec.ts
Modified "onOptionChange with query" test case: changed callback setup to use JS mode with {{Query1.run().then(...)}}; refactored widget interaction to wait for element appearance, click once, then conditionally retry based on dropdown visibility before asserting final state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

Bug, ok-to-test

Suggested reviewers

  • sondermanish

Poem

🌳 A TreeSelect test takes flight,
JS mode shines oh so bright,
Retry logic, clean and sound,
Dropdown visibility—now found! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: stabilizing a flaky TreeSelect test through deterministic configuration and retry logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description comprehensively covers the issue, root causes, fix details, and includes Cypress test results confirming all tests passed.

✏️ 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 subrata71/fix-tree-select-flaky

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 added the ok-to-test Required label for CI label Apr 5, 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_2_spec.ts`:
- Around line 232-236: Replace the hardcoded CSS string inside the
cy.get("body").then(...) retry check (currently using
".rc-tree-select-dropdown:not(.rc-tree-select-dropdown-hidden)") with a
locator-backed selector variable (preferably a data-* attribute), e.g., create
or reuse a constant like TREE_SELECT_DROPDOWN and use that in the
$body.find(...) condition; update the test to reference that locator variable
instead of the raw CSS string so the check follows the repo selector rules.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 56b6498e-96e4-497e-a36f-c01b35439a3e

📥 Commits

Reviewing files that changed from the base of the PR and between 7f10885 and 43efa85.

📒 Files selected for processing (1)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_2_spec.ts

Comment on lines +232 to +236
cy.get("body").then(($body) => {
if (
$body.find(
".rc-tree-select-dropdown:not(.rc-tree-select-dropdown-hidden)",
).length === 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.

⚠️ Potential issue | 🟡 Minor

Use locator-backed selector instead of raw dropdown CSS chain.

Line 235 uses a hardcoded CSS selector inside find(...), which is brittle and against the repo selector rules. Please use a locator variable (preferably backed by data-*) for this retry condition.

♻️ Suggested change in this file
       cy.get("body").then(($body) => {
-        if (
-          $body.find(
-            ".rc-tree-select-dropdown:not(.rc-tree-select-dropdown-hidden)",
-          ).length === 0
-        ) {
+        if ($body.find(locators._treeSelectTitle).length === 0) {
           agHelper.GetNClick(treeSelectInDeploy);
         }
       });

As per coding guidelines: "Use locator variables for locators and do not use plain strings." and "Use data-* attributes for selectors."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/client/cypress/e2e/Regression/ClientSide/Widgets/TreeSelect/Tree_Select_2_spec.ts`
around lines 232 - 236, Replace the hardcoded CSS string inside the
cy.get("body").then(...) retry check (currently using
".rc-tree-select-dropdown:not(.rc-tree-select-dropdown-hidden)") with a
locator-backed selector variable (preferably a data-* attribute), e.g., create
or reuse a constant like TREE_SELECT_DROPDOWN and use that in the
$body.find(...) condition; update the test to reference that locator variable
instead of the raw CSS string so the check follows the repo selector rules.

@subrata71 subrata71 added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant