-
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
test: PartialExport_Widgets_spec - updated exported json file to compare and added confirmation of successful import #33090
Conversation
/ci-test-limit |
WalkthroughWalkthroughThe changes across several files in the Cypress test suite involve importing and utilizing message constants for error and success notifications. This standardizes the messages used in assertions and error handling, enhancing maintainability and readability. The modifications also include updates to test configurations and minor adjustments in test behavior. Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 (
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/8906254185. |
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: 1
Out of diff range and nitpick comments (1)
app/client/cypress/limited-tests.txt (1)
Line range hint
7-7
: Clarify the instruction for running a minimum number of specs.- #ci-test-limit uses this file to run minimum of specs. Do not run entire suite with this command. + #ci-test-limit uses this file to run a minimum number of specs. Do not run the entire suite with this command.This change improves the clarity of the instructions, ensuring that users understand the purpose and limitations of using the
ci-test-limit
command.
app/client/cypress/limited-tests.txt
Outdated
@@ -1,5 +1,5 @@ | |||
# To run only limited tests - give the spec names in below format: |
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.
Consider adding "the" for grammatical correctness.
- # To run only limited tests - give spec names in below format:
+ # To run only limited tests - give the spec names in below format:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
# To run only limited tests - give the spec names in below format: | |
# To run only limited tests - give the spec names in below format: |
/ci-test-limit runId=8906254185 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/8907505353. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/8907505353. |
/ci-test-limit runId=8906254185 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/8908043563. |
/ci-test-limit runId=8906254185 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/8908249893. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/8908249893. |
/ci-test-limit runId=8906254185 |
9 similar comments
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
/ci-test-limit runId=8906254185 |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
/ok-to-test tags="@tag.Workspace @tag.ImportExport" |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
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: 1
"Unable to import application in workspace", | ||
"not.exist", | ||
); | ||
assertHelper.AssertContains(createMessage(UNABLE_TO_IMPORT_APP), "not.exist"); |
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.
Tip
Codebase Verification
The usage of createMessage(UNABLE_TO_IMPORT_APP)
is found only at one location in the codebase (./app/client/cypress/support/gitSync.js:237
). This might indicate that the message is either very specific to a particular scenario or underutilized. It would be beneficial to review whether this limited usage aligns with the intended enhancements of messaging consistency.
./app/client/cypress/support/gitSync.js:237
Analysis chain
Usage of createMessage(UNABLE_TO_IMPORT_APP)
aligns with PR objectives to enhance messaging consistency.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new message is integrated correctly across the test suite.
# Test: Search for the usage of `createMessage(UNABLE_TO_IMPORT_APP)` to ensure it's used consistently. Expect: Matches.
ast-grep --lang javascript --pattern $'createMessage(UNABLE_TO_IMPORT_APP)'
Length of output: 201
/ok-to-test tags="@tag.Workspace @tag.ImportExport" |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
/ok-to-test tags="@tag.Workspace,@tag.ImportExport" |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
/ok-to-test tags="@tag.Workspace,@tag.ImportExport" |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
/ok-to-test tags="@tag.Workspace,@tag.ImportExport" |
Whoops! Looks like you're using an outdated method of running the Cypress suite. |
RCA:
Test was failing at comparison step where we export and compare
Solution:
Updated the WidgetsExportedOnly.json, now the comparison step is working as expected
Improved the assertions within importApp() reduced wait time and also removed the Toast message verification
/ok-to-test tags="@tag.Workspace,@tag.ImportExport"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/8941790028
Commit: 23535e8
Cypress dashboard url: Click here!