-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: Updated file name for remove duplicate #34821
Conversation
Important Review skippedReview was skipped as selected files did not have any reviewable changes. Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThis update involves modifications to Cypress E2E test files, focusing on import path adjustments and the enhancement of test descriptions within an Enterprise context. Additionally, a linter task is outlined for identifying and managing duplicate filenames in spec files. Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_settings_EE_spec.js (1 hunks)
Files skipped from review due to trivial changes (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_settings_EE_spec.js
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.
what sis the naming convention are you suggesting for all devs from going onwards?
app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_settings_EE_spec.js
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (1)
Pattern
app/client/cypress/**/**.*
: Follow best practices for Cypress code and e2e automation.
Avoid using cy.wait in code.
Avoid using cy.pause in code.
Use variables for locators, not strings.
Usedata-*
attributes for selectors; avoid Xpaths and CSS attributes.
Avoid selectors like.btn.submit
orbutton[type=submit]
.
Perform logins via API withLoginFromAPI
.
Only interact with controlled sites/servers.
Ensure tests can run withit.only
and are independent.
Usebefore
,beforeEach
,after
,afterEach
correctly; clean state before tests.
Check new specs for flakiness by running them 10 times on CI.
Use multiple assertions; don't treat Cypress as unit tests.
Use constants for strings.
Include datasource operations inbefore
hooks.
Additional comments not posted (1)
app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (1)
9-9
: LGTM! The test suite description update improves specificity.The updated description "Admin settings page in Enterprise" is more specific and helps in better understanding the context of the tests.
Naming conventions for duplicate file that I can suggest. We can discuss more scenarios as well :
Here is the Google sheet for the duplicate name remove and few examples are given: https://docs.google.com/spreadsheets/d/1UAmtwvKSjFOm8_4uXCSmwbL_YDutlVv1wdXn-xQ18WM/edit?usp=sharing |
@sagar-qa007 let's add this to our best practices doc |
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
app/client/cypress/fixtures/Table/TableFilterImportAppWithVersionTwo.json
Outdated
Show resolved
Hide resolved
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
1 similar comment
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (2 hunks)
Files skipped from review due to trivial changes (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/e2e/Regression/Enterprise/AdminSettings/Admin_Settings_NoAccess_spec.js
Description
Renamed the file name to remove duplicity for FileNames.
Fixes #
34799
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9891607703
Commit: 0cf957b
Cypress dashboard.
Tags:
@tag.All
Spec:
Thu, 11 Jul 2024 13:34:23 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit