-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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.
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
fix:Widget Column test case fix #38045
Conversation
/ci-test-limit-count run_count=20 runId=12232063661 |
WalkthroughThe pull request modifies the Cypress test cases for widget property navigation, specifically updating the properties being tested in the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/12233426820. |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts (1)
Line range hint
175-179
: Use constants for assertion messages.Store the column name "imdb_id" in a constant to maintain consistency and make updates easier.
+const COLUMN_NAME = "imdb_id"; // ... -_.agHelper.GetNAssertContains(_.propPane._paneTitle, "imdb_id"); +_.agHelper.GetNAssertContains(_.propPane._paneTitle, COLUMN_NAME);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts
(1 hunks)app/client/cypress/limited-tests.txt
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/cypress/limited-tests.txt
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts (1)
Pattern app/client/cypress/**/**.*
: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts (2)
167-171
: 🛠️ Refactor suggestion
Avoid using hardcoded waits.
The test uses WaitUntilTableLoad
before interacting with the table. Consider using Cypress's built-in retry-ability with assertions instead.
-_.table.WaitUntilTableLoad(0, 0, "v2");
+cy.get('[data-cy="table-widget"]')
+ .should('be.visible')
+ .and('have.length.gt', 0);
Likely invalid or redundant comment.
Line range hint 1-182
: Verify test coverage for column name changes.
The test case focuses on validating the regex functionality but should also verify the column name change impact.
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts
Outdated
Show resolved
Hide resolved
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12233426820.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
## Description Column name updates. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Widget" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12235074532> > Commit: 245d0f2 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12235074532&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` > Spec: > <hr>Mon, 09 Dec 2024 13:04:29 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Updated widget property navigation test cases to reflect changes in widget properties. - **Chores** - Modified the list of limited tests to replace an outdated test entry with a new focus on widget property navigation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Column name updates.
Fixes #
Issue Number
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Widget"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12235074532
Commit: 245d0f2
Cypress dashboard.
Tags:
@tag.Widget
Spec:
Mon, 09 Dec 2024 13:04:29 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
Chores