-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
test: udpated 3rd party api for Commandclick nav spec #36545
Conversation
WalkthroughThe pull request introduces changes to test specifications within the Cypress framework, specifically updating the URL for a command click navigation test from a public domain to a local development address. Additionally, it modifies the list of tests executed during limited runs, replacing a template test with the new command click navigation test, thereby refocusing the testing efforts. 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
|
/ci-test-limit-count run_count=25 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11037072343. |
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: 0
🧹 Outside diff range and nitpick comments (2)
app/client/cypress/e2e/Regression/ClientSide/IDE/Command_Click_Navigation_spec.js (2)
82-82
: Class, let's examine this URL change, shall we?I see you've updated the URL to use a local development address. Very good! This change allows for testing against a local environment, which can be quite useful. However, let's consider a few things:
Remember that using "host.docker.internal" might not work in all environments. It's specific to Docker for Mac and Windows.
We should think about how this will affect our CI/CD pipeline. Will this URL work in all testing environments?
For extra credit, consider using environment variables for the URL. This way, we can easily switch between different environments without changing the code. For example:
Cypress.env('API_URL') + "/{{ SQL_Query.data }}"Then you can set the API_URL in your Cypress configuration or when running the tests.
Any questions, class?
Line range hint
1-238
: Attention, class! Let's review our test file hygiene.While you've done a commendable job on many aspects, there are a few areas where we need to improve to meet our coding guidelines:
I've noticed several instances of
cy.wait()
andagHelper.Sleep()
. Remember, class, we want to avoid these as they can make our tests flaky and slow. Instead, let's use Cypress's built-in retry-ability and wait for specific elements or network requests.Some of our selectors could use a bit of polishing. While we're using data-* attributes in some places (good job!), we're still relying on class names or complex selectors in others. Let's aim to use data-* attributes consistently throughout our tests.
Well done on avoiding
cy.pause()
,it.only
, and duplicate filenames! You're showing great progress in these areas.For homework, I'd like you to:
- Replace
cy.wait()
andagHelper.Sleep()
with appropriate Cypress commands that wait for specific conditions.- Update all selectors to use data-* attributes.
Here's an example of how you might refactor a wait:
// Instead of: cy.wait(2000); cy.get('.some-class').click(); // Do this: cy.get('[data-cy=some-element]').should('be.visible').click();Any questions before you start your revisions?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/IDE/Command_Click_Navigation_spec.js (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ClientSide/IDE/Command_Click_Navigation_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
: Class, let's review this change together.Now, children, pay attention to line 2. We've updated our test file selection, haven't we? This is an excellent example of focusing our efforts on a specific area of study. We've moved from studying templates to examining the IDE's command click navigation. This aligns perfectly with our lesson plan, doesn't it?
Remember, when we make changes like this, we're telling our testing system which areas to concentrate on. It's just like when I ask you to open your textbooks to a specific page - we're directing our attention to what's most important right now.
Any questions before we move on?
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11037072343.
|
) replace 3rd API with TED /ok-to-test tags="@tag.IDE" <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated the test suite to focus on command click navigation within the IDE. - **Bug Fixes** - Adjusted the base URL for the data source in the navigation test to reflect local development settings. - **Chores** - Modified the limited tests execution list to prioritize IDE command click navigation tests over template-related tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11046090068> > Commit: fd95c8b > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11046090068&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.IDE` > Spec: > <hr>Thu, 26 Sep 2024 05:55:50 UTC <!-- end of auto-generated comment: Cypress test results --> --------- Co-authored-by: “NandanAnantharamu” <“nandan@thinkify.io”>
replace 3rd API with TED
/ok-to-test tags="@tag.IDE"
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11046090068
Commit: fd95c8b
Cypress dashboard.
Tags:
@tag.IDE
Spec:
Thu, 26 Sep 2024 05:55:50 UTC