chore: fix map chart spec #36370
chore: fix map chart spec #36370
Conversation
jsartisan
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe changes in this pull request focus on enhancing the Cypress test specifications for the Map Chart widget. Key modifications include updating the locator for the widget, streamlining test processes by eliminating redundant deployment steps, and introducing a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (5 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/src/widgets/MapChartWidget/component/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/MapChartWidget/component/index.tsx
Additional context used
Path-based instructions (2)
app/client/cypress/limited-tests.txt (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.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_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 not posted (7)
app/client/cypress/limited-tests.txt (2)
2-2: Good job commenting out the test file!By commenting out the
Fork_Template_spec.jstest file, you have effectively removed it from the limited test run. This is a great way to control which tests are executed and focus on the most relevant ones.
3-3: Excellent work adding the new test file!Including the
MapChart_Spec.tstest file in the limited test run is a great way to ensure that the map chart functionality is thoroughly tested. Here are a few best practices to keep in mind when writing Cypress tests:
- Use data-* attributes for selectors to make them more resilient to UI changes.
- Avoid using cy.wait() and cy.pause() as they can lead to flaky tests. Instead, use explicit assertions to wait for the desired state.
- Use locator variables for selectors instead of plain strings to improve readability and maintainability.
- Perform logins, logouts, and signups via API to speed up the tests and reduce flakiness.
- Use multiple assertions in expect statements to test different aspects of the functionality.
Keep up the great work and happy testing!
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (5)
23-26: Great job using the locator variable and making a specific assertion! 👍This code segment follows the best practices by using the
publishWidgetspage.mapChartWidgetlocator variable instead of a plain string selector. The assertion is also checking for the presence of a specific text within the SVG, which is an effective way to verify the map chart is rendering the expected data.
41-44: Excellent assertion to verify the map chart updates! 🌍After changing the map type to "World with Antarctica", the code correctly asserts that the SVG element within the map chart widget contains the text "AT". This is a great way to verify that the map chart is updating correctly based on the selected map type.
107-110: Great job verifying the chart data update! 📊The code updates the chart data by typing a JSON string into the "Chart data" field in the properties pane. It then asserts that the SVG element within the map chart widget contains the text "IN: 2", which corresponds to the updated data. This is an effective way to verify that the map chart is updating correctly based on the provided chart data.
120-123: Nice assertion to verify the label visibility! 🏷️After turning off the "Show Labels" property using the toggle in the properties pane, the code asserts that the SVG element within the map chart widget does not contain the text "IN: 2". This is a great way to verify that the map chart is updating correctly based on the visibility settings.
181-184: Excellent assertion to verify the color range style change! 🎨After updating the "Color Range" property with a JSON string in the "Style" tab, the code asserts that the SVG path element within the map chart widget has the fill attribute set to "rgb(0,0,0)". This is an effective way to verify that the map chart is applying the style changes correctly based on the provided color range.
| cy.get(publishWidgetspage.mapChartWidget) | ||
| .find("svg") | ||
| .find("text") | ||
| .should("contain.text", "AS"); |
There was a problem hiding this comment.
Let's avoid duplicating code to keep our tests maintainable. 🧹
This code segment appears to be a duplicate of the previous one, performing the same assertion. Having duplicate code is not a good practice as it makes the test suite harder to maintain.
Consider removing this duplicate code segment to keep the test suite clean and maintainable.
|
|
||
| agHelper.GetNClick(locators._enterPreviewMode); | ||
| agHelper | ||
| .GetElement(publishWidgetspage.mapChartWidget) | ||
| .find("svg") | ||
| .find("text") | ||
| .should("contain.text", "IN: 2") | ||
| .click(); | ||
| agHelper.ValidateToastMessage("Data Point India Clicked"); | ||
| agHelper.GetNClick(locators._exitPreviewMode); |
There was a problem hiding this comment.
Great test for the "onDataPointClick" property, but let's avoid using agHelper.Sleep! 🚫💤
The test effectively verifies the behavior of the "onDataPointClick" property by setting it to "Show alert" with a custom message, entering preview mode, clicking on a specific data point, and validating the presence of the expected toast message. This is a good way to ensure the map chart is triggering the correct action when a data point is clicked.
However, the code is using agHelper.Sleep, which is not recommended as per the additional instructions.
Consider removing the usage of agHelper.Sleep to align with the best practices and improve the test's reliability.
|
/ci-test-limit-count run_count=15 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10901316270. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10901316270. |
|
/ci-test-limit-count run_count=15 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10902992634. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10902992634. |
|
/ci-test-limit-count run_count=15 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10905878348. |
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (4)
25-31: Excellent assertions for verifying the map chart plot! 🌍✅The code segment effectively asserts the existence of the map chart plot with the text "AS" in both the editor and the deployed app. It also navigates back to the editor and selects the "MapChart1" widget, which is a good practice.
Just remember to update the
_mapChartPlotfunction to use data-* attributes for selectors, as mentioned in the previous comment.
37-49: Great test for verifying different map types! 🗺️👌The code segment effectively tests the behavior of the map chart widget when the map type is changed. It asserts the existence of specific map chart plots for each map type, ensuring that the widget renders the correct data.
Just remember to update the
_mapChartPlotfunction to use data-* attributes for selectors, as mentioned in the previous comment.
79-79: Nice assertion for verifying the updated chart data! 📊✅The code segment effectively asserts the existence of the map chart plot with the text "IN: 2" after updating the chart data. This ensures that the widget renders the updated data correctly.
Just remember to update the
_mapChartPlotfunction to use data-* attributes for selectors, as mentioned in the previous comment.
Line range hint
89-120: Fantastic tests for the "Show Labels" and "onDataPointClick" properties! 🏷️🖱️💯The code segment effectively tests the behavior of the "Show Labels" and "onDataPointClick" properties. It ensures that the map chart widget responds correctly to the property changes and user interactions.
The tests follow the recommended practices of using multiple assertions and avoiding the use of
agHelper.Sleep, which is great!Just remember to update the
_mapChartPlotfunction to use data-* attributes for selectors, as mentioned in the previous comment.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (5 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_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 not posted (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (1)
2-2: Great job importing the publish widgets page locators! 👍The import statement is syntactically correct and points to the correct JSON file. This will allow you to access the locators for the publish widgets page in your tests.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10905878348. |
| deployMode.NavigateBacktoEditor(); | ||
| EditorNavigation.SelectEntityByName("MapChart1", EntityType.Widget); | ||
| cy.get(publishWidgetspage.mapChartWidget) | ||
| .find("[data-testid='t--map-chart-container']") |
There was a problem hiding this comment.
can we please make this as locator variable? Kindly check all the locators.
There was a problem hiding this comment.
Actionable comments posted: 8
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (5 hunks)
- app/client/cypress/locators/publishWidgetspage.json (3 hunks)
Files skipped from review due to trivial changes (1)
- app/client/cypress/locators/publishWidgetspage.json
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_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 not posted (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts (2)
2-2: Great job importing the locators from the JSON file! 👍The import statement is correctly written and follows the proper syntax. The relative import path points to the correct location of the
publishWidgetspage.jsonfile.
29-29: Let's remove the duplicate code and update the selector. 🧹🔍This code segment is a duplicate of the previous one at line 25, performing the same assertion. Having duplicate code is not a good practice as it makes the test suite harder to maintain.
Consider removing this duplicate code segment to keep the test suite clean and maintainable. Additionally, update the assertion to use a data-* attribute selector instead of the XPath-based
_mapChartPlotselector to align with the best practices.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Widgets/Others/MapChart_Spec.ts
Show resolved
Hide resolved
/ok-to-test tags="@tag.Maps" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10916335124> > Commit: 6e88177 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10916335124&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Maps` > Spec: > <hr>Wed, 18 Sep 2024 06:11:15 UTC <!-- end of auto-generated comment: Cypress test results --> --------- Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local>
/ok-to-test tags="@tag.Maps"
Fixes #36338
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10916335124
Commit: 6e88177
Cypress dashboard.
Tags:
@tag.MapsSpec:
Wed, 18 Sep 2024 06:11:15 UTC