test: replace 3rd party API1 CE #36518
Conversation
NandanAnantharamu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThis pull request introduces changes to two files related to Cypress testing. The URL in the 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
|
github-actions
bot
added
skip-changelog
|
/ci-test-limit-count run_count=25 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11017196827. |
![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 (3)
app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js (3)
Line range hint
99-103: Good job updating the API endpoint, class! Let's make it even better!I see you've successfully replaced the Facebook API with a local Docker host. That's a gold star for following the PR objectives! However, let's take this opportunity to improve our code a bit more.
Consider extracting the base URL into a constant or environment variable. This will make our tests more flexible and easier to maintain. Here's how you could do it:
const API_BASE_URL = Cypress.env('API_BASE_URL') || 'http://host.docker.internal:5001'; // Then use it in your test `${API_BASE_URL}/{{Button2.text}}?key=test&val={{Button2.text}}`This way, we can easily change the base URL for different environments without modifying the test code. Remember, good developers always think about maintainability!
109-111: Excellent work on updating the validation, students! Let's make it even more robust!You've done a splendid job ensuring that the validation matches the new API endpoint. That's very important for maintaining the integrity of our tests.
To make our test even more robust, let's consider using dynamic values for validation instead of hardcoded ones. Here's an example of how we could improve this:
const expectedText = 'Cancel'; const expectedUrl = `http://host.docker.internal:5001/${expectedText}?key=test&val=${expectedText}`; cy.validateEvaluatedValue(expectedUrl);By using variables, we make our test more flexible and easier to update if the expected values change. It's also a good practice to avoid repeating literal values in our code. Remember, DRY (Don't Repeat Yourself) is not just for staying hydrated during long coding sessions!
Line range hint
1-114: Class, let's review our overall test structure and practices!While we've made good progress with our recent changes, there are a few areas where we can improve our test suite to align better with Cypress best practices:
Avoid using
cy.wait(): I noticed we're usingcy.wait(3000)on line 102. Instead of hard-coded waits, let's use Cypress's built-in retry-ability andcy.intercept()for network requests.Use data attributes for selectors: Our test is using class selectors like
.CodeMirror textarea. It's better to use data attributes likedata-cy="api-textarea"for more stable selectors.Avoid using
force: true: We're using this in several places. It's generally a sign that our app's state management could be improved. Let's review why these are necessary and see if we can make our app more testable.Group related tests: Consider using
describeblocks to group related tests together. This improves readability and organization.Use
beforeorbeforeEachfor setup: Move common setup steps (like creating an API) into abeforeorbeforeEachblock to reduce repetition.Remember, writing good tests is just as important as writing good application code. It's an art that we're all continually perfecting!
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js (2 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
Additional context used
Path-based instructions (2)
app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Edit_spec.js (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 not posted (1)
app/client/cypress/limited-tests.txt (1)
2-2: Well done, class! This change looks good.Now, let's take a moment to appreciate the importance of this modification. By switching from a client-side template test to a server-side API test, we're focusing our limited tests on the area that's most relevant to our current lesson plan - replacing the 3rd party API.
Remember, children, in the world of software development, it's crucial to adapt our tests to match the changes we're making. Just like how we update our lesson plans when we introduce new topics!
However, I'd like to assign a bit of homework. While we're focusing on API tests now, don't forget the value of comprehensive testing. When you have time, consider running the full test suite to ensure all areas of our application are still functioning correctly. It's like checking all the chapters in your textbook, not just the one we're currently studying!
Verification successful
Action Required: The
cypress/e2edirectory was not found. Let's locate the correct Cypress test files and verify API references.
Verification Successful: No Additional Test Updates Required
Excellent work! We've confirmed that there are no other tests mentioning either the old API (
API1 CE) or the new API (API_Edit). Your changes are appropriately isolated, ensuring that our test suite remains accurate and maintainable.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are other tests that might need updating echo "Let's see if there are any other tests mentioning our old API:" rg --type js "API1 CE" cypress/e2e echo "And now, let's check if our new API is mentioned in other tests:" rg --type js "API_Edit" cypress/e2eLength of output: 456
Script:
#!/bin/bash # Description: Locate Cypress test files and search for API mentions echo "Locating all Cypress test directories and files:" fd --type d cypress || echo "No Cypress directories found." echo "Searching for 'API1 CE' in Cypress test files:" rg --type js "API1 CE" || echo "No occurrences of 'API1 CE' found." echo "Searching for 'API_Edit' in Cypress test files:" rg --type js "API_Edit" || echo "No occurrences of 'API_Edit' found."Length of output: 589
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11017196827. |
NandanAnantharamu
added
ok-to-test
Replacing the 3rd party API with TED API
/ok-to-test tags="@tag.Datasource"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11027894410
Commit: 0446747
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Wed, 25 Sep 2024 07:37:49 UTC