test: running to test if they are flaky #36732
Conversation
NandanAnantharamu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe changes in this pull request involve updates to the test suites for the MultiSelect and Tabs widgets within the Cypress end-to-end testing framework. Key modifications include unskipping and enhancing test cases, improving validation checks, and updating the list of limited tests to include the newly active test files. These adjustments aim to ensure comprehensive coverage and accurate validation of widget functionalities. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
|
/ci-test-limit |
github-actions
bot
added
skip-changelog
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11229052462. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect5_spec.ts (1)
Line range hint
312-362: Good job on unskipping the test case, class! Let's review your work.I'm pleased to see that you've followed many of our best practices:
- You've avoided using cy.wait, cy.pause, and agHelper.sleep(). Well done!
- You're using locator variables instead of plain strings. That's the way to do it!
- You've steered clear of Xpaths, attributes, and CSS paths. Excellent choice!
- There's no use of it.only, after, or afterEach hooks. Keep it up!
- You've included multiple assertions. That's thorough testing!
- You're using specific assertion methods like VerifyEvaluatedErrorMessage instead of string comparisons. Very good!
However, there's always room for improvement. Here are a few suggestions to make your test even better:
- Consider using data-* attributes for selectors instead of relying on text content. This will make your tests more robust.
- This test case is quite long. Think about breaking it down into smaller, more focused test cases. Remember, each test should ideally check one specific behavior.
Can you think of ways to refactor this test into smaller, more focused test cases? It would make our test suite even more organized and easier to maintain.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts (1)
Line range hint
232-234: Avoid Using CSS Path Selectors; Use Locator Variables InsteadDear student, in these lines, you're using a CSS path selector
${propPane._segmentedControl("0")}:contains('Large')directly within your test. While it might work initially, relying on CSS paths can make your tests fragile and harder to maintain. It's important to use locator variables with data-* attributes for selectors to enhance the robustness and readability of your tests.Using locator variables not only adheres to best practices but also ensures that minor changes in the UI won't break your tests. Let's refactor this to use a locator variable.
Here's how you can modify your code:
First, define a new locator variable in your locators file:
+ // In locators.ts + export const _largeBoxShadowOption = "[data-testid='box-shadow-large']";Then, update your test to use the locator variable:
- agHelper.GetNClick(`${propPane._segmentedControl("0")}:contains('Large')`); + agHelper.GetNClick(propPane._largeBoxShadowOption);This approach uses a data-* attribute for the selector, aligning with best practices and making your test more resilient to UI changes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect5_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelect5_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.
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_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.
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.
🔇 Additional comments (4)
app/client/cypress/limited-tests.txt (3)
2-2: Excellent addition to our limited test suite, class!I'm pleased to see the inclusion of the Tabs widget test in our limited test suite. This is a smart move, as it ensures we're keeping a close eye on this critical component of our user interface.
Remember, children, thorough testing of our widgets is key to maintaining a robust application. Keep up the good work!
3-3: Another gold star for our test suite, students!I'm thrilled to see the MultiSelect widget test joining our limited test suite. This addition shows great attention to detail in our testing strategy.
Just like in a well-balanced curriculum, having a diverse set of tests helps us ensure that all parts of our application are functioning correctly. Keep this holistic approach in mind as you continue to develop and test!
1-3: A note on our curriculum changes, dear students!I've noticed that we've removed the Fork Template test from our limited test suite. While spring cleaning can be good, we must ensure we're not leaving any gaps in our knowledge!
Could you please explain why this test was removed? Are we covering this functionality in other tests, or has it become less critical? Remember, in the classroom of software development, every test plays a role in our learning journey.
To help us understand the impact of this change, let's do a quick search of our codebase:
This will help us ensure we haven't accidentally dropped an important lesson from our curriculum!
✅ Verification successful
Test Coverage Confirmation for Fork Template Removal
After reviewing the codebase, it appears that the
Fork_Template_spec.jstest has been successfully removed without leaving any gaps in our test coverage. Other related tests, such asFork_Template_Existing_app_spec.jsandFork_Template_To_App_spec.ts, continue to ensure that the Fork Template functionality is thoroughly tested.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to the removed Fork Template test and look for potential replacements echo "Searching for references to the removed Fork Template test:" rg --type-add 'spec:*.{js,ts}' --type spec "Fork_Template_spec" echo "\nChecking for other Template-related tests that might cover similar functionality:" fd -e js -e ts . | rg "Template.*spec"Length of output: 949
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: You might be missing the article “the” here.
Context: ... limited tests - give the spec names in below format: cypress/e2e/Regression/ClientSi...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Tab/Tabs_2_spec.ts (1)
Line range hint
192-241: Test Case Reactivation Enhances Test CoverageExcellent work reactivating the test case for verifying colors, borders, and shadows. This is a vital step in ensuring that the Tabs widget's styling functionalities are thoroughly tested and function as expected. Keep up the good work!
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
Summary by CodeRabbit