-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
chore: unified way of writing messages in cypress #33659
Conversation
WalkthroughThe changes involve a systematic refactoring of message handling in Cypress tests across various files. Instead of directly importing message constants and functions like Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Failed server 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
Outside diff range and nitpick comments (4)
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js (1)
Line range hint
19-39
: Consider refactoring to use arrow functions for consistency and reduced scope complexity.- it("1.Verify omnibar is present across all pages and validate its fields", function () { + it("1.Verify omnibar is present across all pages and validate its fields", () => {This change aligns with modern JavaScript practices and enhances the readability of the code.
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/API_Pane_spec.js (1)
Line range hint
22-75
: Consider refactoring to use arrow functions for consistency and reduced scope complexity.- function () { + () => {This change aligns with modern JavaScript practices and enhances the readability of the code.
app/client/cypress/support/Pages/PartialImportExport.ts (1)
Line range hint
160-165
: Consider refactoringforEach
tofor...of
for better performance and readability.- checkboxesInSection.each((element) => { + for (const element of checkboxesInSection) {This change aligns with modern JavaScript practices and enhances the readability and performance of the code.
Also applies to: 169-171, 183-185
app/client/cypress/e2e/Regression/Apps/ImportExportForkApplication_spec.js (1)
Line range hint
17-65
: Consider refactoring to use arrow functions for consistency and reduced scope complexity.- function () { + () => {This change aligns with modern JavaScript practices and enhances the readability of the code.
Also applies to: 67-81, 83-164
Lgtm |
Description
We are using
createMessage()
andCypress.env("MESSAGES")
in Cypress code to use strings. This can create confusion around the method to use and creates inconsistency in our cypress code base. This PR unifies it by following one way ieCypress.env("MESSAGES")
. MESSAGES is defined in https://github.com/appsmithorg/appsmith/blob/release/app/client/cypress/support/e2e.js#L61Conversation link - https://theappsmith.slack.com/archives/C06FWBV74GH/p1715753205226649
Fixes #
Issue Number
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
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/9252248316
Commit: 5851706
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?