-
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
Do Not merge : Running test #37235
base: release
Are you sure you want to change the base?
Do Not merge : Running test #37235
Conversation
WalkthroughThe pull request updates a configuration file for Cypress tests, specifically changing the test file path from Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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=100 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11683153306. |
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 (1)
app/client/cypress/limited-tests.txt (1)
Line range hint
1-6
: Consider documenting the test selection criteria.Since this file is critical for CI and used for running minimum specs, it would be helpful to document:
- Why this particular test was chosen for limited runs
- What criteria should be used when updating this file
Add a comment explaining the test selection criteria:
# To run only limited tests - give the spec names in below format: cypress/e2e/Regression/ClientSide/EmbedSettings/EmbedSettings_spec.js +# Note: Tests listed here should be: +# - Stable and reliable +# - Representative of critical functionality +# - Quick to execute # For running all specs - uncomment below: #cypress/e2e/**/**/*
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/cypress/limited-tests.txt
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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 (1)
app/client/cypress/limited-tests.txt (1)
2-2
: Verify the existence of the new test file.
Let's ensure the specified test file exists and is the appropriate one for limited test runs.
✅ Verification successful
Test file exists and is correctly referenced
The specified test file EmbedSettings_spec.js
exists at the correct location and follows the expected path format for Cypress tests. The file is only referenced in the limited-tests configuration, which is appropriate for its intended use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of the specified test file
fd "EmbedSettings_spec.js" app/client/cypress/e2e/Regression/ClientSide/EmbedSettings/
# Check if there are any references to this test in other configuration files
rg "EmbedSettings_spec.js" app/client/cypress/
Length of output: 321
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11683153306.
|
/ci-test-limit-count run_count=100 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11792895314. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11792895314.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
37234
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit