-
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
chore: add snapshots for text widgets #34546
Conversation
WalkthroughThe recent updates introduce Cypress end-to-end test cases for Anvil's Heading and Paragraph Widgets. These tests verify the functionalities of the widgets in Canvas, Preview, and Deploy modes, ensuring consistency across different environments. Changes
Possibly related issues
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 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 ignored due to path filters (12)
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetCanvas.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetCanvasDark.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetDeployipad-2.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetDeployiphone-6.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetDeploymacbook-13.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilHeadingWidgetSnapshot_spec.ts/anvilHeadingWidgetPreview.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetCanvas.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetCanvasDark.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetDeployipad-2.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetDeployiphone-6.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetDeploymacbook-13.snap.png
is excluded by!**/*.png
app/client/cypress/snapshots/AnvilParagraphWidgetSnapshot_spec.ts/anvilParagraphWidgetPreview.snap.png
is excluded by!**/*.png
Files selected for processing (4)
- app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilHeadingWidgetSnapshot_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilParagraphWidgetSnapshot_spec.ts (1 hunks)
- app/client/cypress/fixtures/anvilHeadingWidget.json (1 hunks)
- app/client/cypress/fixtures/anvilParagraphWidget.json (1 hunks)
Files not summarized due to errors (2)
- app/client/cypress/fixtures/anvilHeadingWidget.json: Error: Message exceeds token limit
- app/client/cypress/fixtures/anvilParagraphWidget.json: Error: Message exceeds token limit
Additional comments not posted (6)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilHeadingWidgetSnapshot_spec.ts (2)
1-5
: Ensure correct and optimized imports.The imports are correctly structured, but ensure that
agHelper
andanvilSnapshot
are used in the file, which they are. Also, check if the path to these imports is correct and optimized.
7-27
: Comprehensive testing of widget modes.The test suite covers Canvas, Preview, and Deploy modes for the Heading Widget, which is excellent for ensuring widget functionality across different states.
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilParagraphWidgetSnapshot_spec.ts (2)
1-5
: Ensure correct and optimized imports.The imports are correctly structured, but ensure that
agHelper
andanvilSnapshot
are used in the file, which they are. Also, check if the path to these imports is correct and optimized.
7-27
: Comprehensive testing of widget modes.The test suite covers Canvas, Preview, and Deploy modes for the Paragraph Widget, which is excellent for ensuring widget functionality across different states.
app/client/cypress/fixtures/anvilParagraphWidget.json (1)
1-2525
: Comprehensive Review of JSON Structure for UI TestingThe JSON structure is well-formed and appears consistent across different sections. Each widget and section is defined with appropriate properties that are crucial for rendering and interaction within the UI. Here are some specific observations and suggestions:
Consistency and Clarity: The property names like
widgetId
,widgetType
,isVisible
, andtextColor
are consistently used, which is good for maintainability. However, it might be beneficial to add comments or documentation within the JSON or accompanying files to explain the purpose of less intuitive properties likedynamicTriggerPathList
anddynamicBindingPathList
.Testing Readiness: The structure includes
isVisible
,textColor
, andfontSize
properties which are essential for visual testing. Ensure these properties are leveraged in snapshot tests to validate visual consistency across different states and environments.Optimization Suggestion: There are repeated sections with similar properties (
widgetId
,widgetType
, etc.). If these sections are meant to represent similar UI elements under different conditions or states, consider abstracting common properties into reusable components or templates to reduce redundancy and improve test maintenance.Error Handling and Validation: The properties like
needsErrorInfo
andisDeprecated
are crucial for understanding component states during testing. Ensure that tests explicitly check these states to prevent deprecated or error-prone components from affecting user experience.Layout and Responsiveness: The properties related to layout and responsiveness (
mobileTopRow
,mobileLeftColumn
,responsiveBehavior
) are crucial for ensuring the UI adapts correctly to different screen sizes. It's recommended to include specific tests that simulate various device sizes to verify responsiveness.Overall, the JSON file is well-prepared for integration with snapshot tests for the text widgets. Make sure to keep the structure updated in line with any UI changes to maintain the accuracy of tests.
app/client/cypress/fixtures/anvilHeadingWidget.json (1)
1-2591
: JSON structure and content validation:
- The JSON file structure appears to be correct and well-formed.
- The
dsl
key contains a complex nested structure representing the layout, which is expected in the context of this file.- The
id
is correctly formatted as a string.- The
userPermissions
array is empty, which might be intentional depending on the test setup.- No syntax errors or misplaced keys/values were detected.
Overall, the JSON file meets the expected standards for a configuration file used in Cypress testing for UI layouts.
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. #34528 #34546 #34676 #34729 #34638 #34639 #34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local> Co-authored-by: unknown <vadim@appsmith.com>
Merged here. |
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. appsmithorg#34528 appsmithorg#34546 appsmithorg#34676 appsmithorg#34729 appsmithorg#34638 appsmithorg#34639 appsmithorg#34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local> Co-authored-by: unknown <vadim@appsmith.com>
## Description By re organising the panels in the IDE, we are creating a new Code Editor only panel which is separate from the main / canvas and the explorer panel. This ensures that the Code Editor is not remounted when switching between Split screen and Full screen. This PR is also improving the Preview mode by not changing urls when entering and exiting preview modes. EE PR: appsmithorg/appsmith-ee#4760 Fixes #31707 Fixes #31387 Fixes #34545 Fixes #34546 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10177085462> > Commit: 26688b0 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10177085462&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Wed, 31 Jul 2024 09:42:50 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced `EditorPaneExplorer` and `QueryExplorer` components for enhanced navigation and query management within the IDE. - Added visual enhancements to `ListJSObjects` and `AddWidgets` components with new style properties. - **Improvements** - Implemented clamping in the `useDynamicAppLayout` hook to prevent negative width values, enhancing layout stability. - Updated layout management to ensure a responsive user experience based on IDE view modes. - **Style** - Enhanced visual clarity by adding borders to various components, improving UI separation and organization. - **Bug Fixes** - Streamlined test cases in the `Postgres2_Spec.ts` to maintain coherent sequencing after removing unnecessary tests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
/ok-to-test tags="@tag.Anvil"
Summary by CodeRabbit
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9696369218
Commit: 5201281
Cypress dashboard.
Tags:
@tag.Anvil
The following are new failures, please fix them before merging the PR: