test: replaced 3rd apty api in API_On_Load spec #36529
Conversation
NandanAnantharamu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
|
/ci-test-limit-count run_count=25 |
WalkthroughThis pull request introduces changes primarily focused on updating API endpoint references and test specifications. The API test case in 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
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11028380622. |
![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/limited-tests.txt (1)
2-2: Well done, class! This change looks splendid!Now, let's take a moment to appreciate the thoughtful modification you've made. You've successfully replaced the client-side template test with a server-side API load test. This aligns perfectly with our lesson plan of replacing the third-party API with the TED API.
Remember, children, in the world of testing, it's important to keep our focus on what matters most. By updating this file, you're ensuring that our limited tests are targeting the right areas.
As your teacher, I'd like to give you a little homework assignment. Consider if there are any additional tests we should include in our limited test suite. It's always good to be thorough!
If you think of any other crucial tests, feel free to add them to this file. For example:
cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts + cypress/e2e/Regression/ServerSide/AnotherImportantTest_Spec.tsDon't forget to raise your hand if you need any help with this assignment!
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts (1)
57-57: Let's discuss best practices for our test code, class!While our code is looking good overall, I'd like to bring your attention to a small improvement we can make. In line 57, we're using a string selector for "Table data". Remember, it's a best practice to use locator variables for our selectors. This makes our tests more maintainable and easier to update if our UI changes.
Here's a little homework assignment:
- Create a locator variable for the "Table data" selector.
- Use this variable in our propPane.UpdatePropertyFieldValue() call.
This small change will make our code more robust and easier to maintain in the long run. Who wants to give it a try?
Consider creating a locator variable for the "Table data" selector and using it in the propPane.UpdatePropertyFieldValue() call. This will improve the maintainability of our test code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_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 not posted (2)
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts (2)
49-49: Class, let's examine the API endpoint change.I see you've updated the API endpoint to use a local Docker address. While this might be helpful for local testing, we need to ensure it doesn't cause issues in other environments. Let's consider the following:
- Is this change intentional for all environments or just for local testing?
- Should we use an environment variable instead of hardcoding the URL?
- How will this affect our CI/CD pipeline?
Remember, in the real world, we often need to handle multiple environments. Let's discuss how we can make this more flexible!
Can you please clarify the intention behind this change and how it will be managed across different environments?
57-57: Now, let's turn our attention to the Table data property update.I notice you've simplified the data access by removing one level of nesting. This is an interesting change! Let's consider a few points:
- Has the structure of
PageLoadApi2.datachanged to warrant this update?- Will this affect how the data is displayed in the table?
- Have we tested this change to ensure the table still displays correctly?
Remember, when we make changes to data structures, it's crucial to verify that all dependent components still function as expected. Let's make sure our table is still happy with this new data format!
Could you please confirm that you've tested the table display with this new data structure? It would be great to see some test results or screenshots to ensure everything is working as intended.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11028380622. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
closing this as its redundant |
replacing 3rd party API with TED api
/ok-to-test tags="@tag.JS"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11030794737
Commit: ee0ac5c
Cypress dashboard.
Tags:
@tag.JSSpec:
Wed, 25 Sep 2024 10:41:35 UTC