Skip to content
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: tests for wds button widget #34242

Merged
merged 26 commits into from
Jun 20, 2024
Merged

Conversation

jsartisan
Copy link
Contributor

@jsartisan jsartisan commented Jun 14, 2024

/ok-to-test tags="@tag.Anvil"

Summary by CodeRabbit

  • New Features

    • Introduced test cases for Anvil Button Widgets, including Canvas, Preview, and Deploy modes.
  • Bug Fixes

    • Updated CSS and HTML selectors for better element targeting and testing reliability.
  • Style

    • Improved styling logic for buttons in the InlineButtons component.
  • Chores

    • Added data-testid attributes for better test targeting.
    • Refactored code for string concatenations and URL constructions in DeployModeHelper.

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9597157402
Commit: c551705
Cypress dashboard.
Tags: @tag.Anvil

@jsartisan
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jun 14, 2024
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9511361491.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 34242.
recreate: .

Copy link
Contributor

coderabbitai bot commented Jun 14, 2024

Walkthrough

The changes introduce new test cases for various Anvil Button Widgets in Canvas, Preview, and Deploy modes. Enhancements also involve refactoring and updating selectors for better targeting and usability. Noteworthy updates include new data-testid attributes added for more precise DOM element identification and the introduction of the AnvilSnapshot class to streamline widget snapshot testing across different modes and devices.

Changes

File/Directory Change Summary
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts Added tests for Anvil Button Widget in Canvas, Preview, and Deploy modes.
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilIconButtonWidgetSnapshot_spec.ts Added tests for Anvil Icon Button Widget in Canvas, Preview, and Deploy modes.
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilInlineButtonWidgetSnapshot_spec.ts Added tests for Anvil Inline Button Widget in Canvas, Preview, and Deploy modes.
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilToolbarButtonWidgetSnapshot_spec.ts Added tests for Anvil Toolbar Button Widget in Canvas, Preview, and Deploy modes.
app/client/cypress/support/Objects/ObjectsCore.ts Added anvilSnapshot constant to exports.
app/client/cypress/support/Objects/Registry.ts Imported AnvilSnapshot, added anvilSnapshot__ property and AnvilSnapshot getter.
app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts Introduced AnvilSnapshot class with methods for verifying widget modes.
app/client/cypress/locators/commonlocators.json Updated viewerPage selector for better targeting.
app/client/cypress/support/Pages/DeployModeHelper.ts Refactored CSS selectors and string concatenations for improved element targeting and URL logic.
app/client/cypress/support/index.d.ts Added matchImageSnapshot method to the Cypress namespace.
.../design-system/widgets/src/components/InlineButtons/src/styles.module.css Changed styling logic for elements with data-button to flex-shrink: 0;.
app/client/src/pages/AppViewer/AppPage.tsx Replaced className with data-testid in the PageView component.
app/client/src/pages/AppViewer/index.tsx Added data-testid attribute to AppViewerBody component.
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx Added data-testid to SegmentedControl component.

Poem

In fields of code, where buttons gleam,
Anvil tests in modes supreme,
With snapshots crisp, and targets true,
Flex and IDs freshly new.
A rabbit cheers with joyful glee,
For changes made so thoughtfully!


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Outside diff range and nitpick comments (2)
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

Line range hint 144-159: Address accessibility and default behavior concerns by specifying the button type explicitly.

- <button
+ <button type="button"
app/client/cypress/support/Objects/Registry.ts (1)

Line range hint 34-276: The addition of AnvilSnapshot to ObjectsRegistry is correctly implemented. However, consider refactoring to avoid a class with only static members, which can be less maintainable and harder to manage.

Consider refactoring ObjectsRegistry to use a more modular approach, possibly using functions or smaller classes.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dae657e and fa5790cbc71e975bfb76f305489b3a5de7eda576.

Files ignored due to path filters (1)
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetanvilButtonWidget.snap.png is excluded by !**/*.png
Files selected for processing (6)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/fixtures/anvilButtonWidget.json (1 hunks)
  • app/client/cypress/support/Objects/ObjectsCore.ts (1 hunks)
  • app/client/cypress/support/Objects/Registry.ts (2 hunks)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
  • app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1 hunks)
Files not summarized due to errors (1)
  • app/client/cypress/fixtures/anvilButtonWidget.json: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
  • app/client/cypress/support/Objects/ObjectsCore.ts
Additional context used
Biome
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx

[error] 144-159: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

app/client/cypress/support/Objects/Registry.ts

[error] 36-308: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

Additional comments not posted (4)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts (1)

1-2: LGTM! Imports are correctly organized and relevant to the test cases described.

app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (2)

1-3: The class AnvilSnapshot is well-organized with properties and locators clearly defined. Ensure consistency in the use of data-testid attributes across the application.

Also applies to: 8-13


35-41: The loop for device-specific verification is efficient. Ensure that all devices listed are supported and correctly configured in Cypress.

app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

120-120: The addition of the data-testid attribute will assist in more reliable testing. Ensure that the test identifiers are unique and descriptive.

Comment on lines +1 to +2672
"isPermanent": true,
"layout": [
{
"insertChild": true,
"layoutId": "xu64jenr2g",
"layoutType": "ALIGNED_WIDGET_ROW",
"layout": [
{
"alignment": "start",
"widgetId": "ok0k9hn4dg",
"widgetType": "WDS_BUTTON_WIDGET"
}
],
"allowedWidgetTypes": [
"SMALL_WIDGETS"
],
"maxChildLimit": 0
}
],
"layoutId": "4ea1fxmoer",
"layoutStyle": {
"border": "none",
"height": "100%"
},
"layoutType": "ZONE"
}
],
"responsiveBehavior": "fill",
"borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}",
"mobileLeftColumn": 0
}
],
"elevatedBackground": false,
"key": "04mvbegryn",
"isDeprecated": false,
"rightColumn": 64,
"detachFromLayout": false,
"widgetId": "c3qg79mmbs",
"onCanvasUI": {
"selectionBGCSSVar": "--on-canvas-ui-section-selection",
"focusBGCSSVar": "--on-canvas-ui-section-focus",
"selectionColorCSSVar": "--on-canvas-ui-section-focus",
"focusColorCSSVar": "--on-canvas-ui-section-selection",
"disableParentSelection": true
},
"zoneCount": 3,
"isVisible": true,
"version": 1,
"parentId": "0",
"tags": [
"Layout"
],
"renderMode": "CANVAS",
"isLoading": false,
"mobileTopRow": 0,
"layout": [
{
"isContainer": true,
"isDropTarget": true,
"isPermanent": true,
"layout": [
{
"alignment": "start",
"widgetId": "c1zk109ftd",
"widgetType": "ZONE_WIDGET"
},
{
"alignment": "start",
"widgetId": "194q1o3h78",
"widgetType": "ZONE_WIDGET"
},
{
"alignment": "start",
"widgetId": "o7j5kktsuu",
"widgetType": "ZONE_WIDGET"
}
],
"layoutId": "eogsma2p43",
"layoutStyle": {
"border": "none"
},
"layoutType": "SECTION",
"maxChildLimit": 4
}
],
"responsiveBehavior": "fill",
"mobileLeftColumn": 0,
"spaceDistributed": {
"c1zk109ftd": 4,
"194q1o3h78": 4,
"o7j5kktsuu": 4
}
}
]
},
"layoutOnLoadActions": [],
"layoutOnLoadActionErrors": [],
"id": "666a987c0b846c72d3af7fad",
"userPermissions": []
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The key "bn9xz1h2d0" is duplicated across multiple widgets, which can lead to conflicts and errors. Each widget should have a unique key to ensure proper identification and behavior.

  • Duplicate keys found: bn9xz1h2d0

Please ensure that each widget has a unique key.

Analysis chain

Ensure consistency and correctness in widget configurations.

  • It appears that multiple widgets are configured with similar properties, which is expected in a templated layout. Ensure that all properties like widgetType, isVisible, isDeprecated, and renderMode are correctly set according to the widget specifications.
  • The use of "isDeprecated": false across multiple widgets suggests these are current configurations. Confirm that none of these widgets are planned for deprecation unless specified.
  • The "key": "bn9xz1h2d0" is repeated across multiple widgets, which might be an error unless it's intentionally shared. Typically, keys should be unique to ensure that they can be individually addressed.
  • The "buttonVariant": "filled" is consistently used, aligning with a potential style guide. Verify that this aligns with the design specifications for the button widgets in the application.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify unique keys across widgets and ensure no deprecated widgets are included unless specified.

# Test: Search for duplicate keys and deprecated flags.
rg --type json --json $'{"key": "bn9xz1h2d0"}' app/client/cypress/fixtures/anvilButtonWidget.json
rg --type json --json $'{"isDeprecated": true}' app/client/cypress/fixtures/anvilButtonWidget.json

Length of output: 393



Script:

#!/bin/bash
# Description: Verify unique keys across widgets and ensure no deprecated widgets are included unless specified.

# Corrected Test: Search for duplicate keys and deprecated flags.
rg -F '"key": "bn9xz1h2d0"' app/client/cypress/fixtures/anvilButtonWidget.json
rg -F '"isDeprecated": true' app/client/cypress/fixtures/anvilButtonWidget.json

Length of output: 1198

Comment on lines 4 to 26
describe(
`${ANVIL_EDITOR_TEST}: Anvil tests for Button Widget`,
{ tags: ["@tag.Anvil"] },
() => {
const widgetName = "ButtonWidget";

before(() => {
agHelper.AddDsl("anvilButtonWidget");
});

it("1. Canvas Mode", () => {
anvilSnapshot.verifyCanvasMode(widgetName);
});

it("2. Preview Mode", () => {
anvilSnapshot.verifyPreviewMode(widgetName);
});

it("3. Deploy Mode", () => {
anvilSnapshot.verifyDeployMode(widgetName);
});
},
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The search results indicate that the AddDsl method is defined in app/client/cypress/support/Pages/AggregateHelper.ts, but there is no specific implementation for the anvilButtonWidget DSL found in the codebase. This suggests that the DSL definition for anvilButtonWidget might be missing or not correctly implemented.

  • app/client/cypress/support/Pages/AggregateHelper.ts: Definition of AddDsl method.
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts: Usage of AddDsl("anvilButtonWidget").

Please ensure that the DSL for anvilButtonWidget is correctly implemented and defined in the project.

Analysis chain

The test suite is well-structured with clear separation of test cases for different modes. Ensure that the DSL for the widget is correctly implemented as it's critical for the tests.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the implementation of `anvilButtonWidget` DSL in the project.

# Test: Search for the DSL implementation. Expect: At least one occurrence in the codebase.
rg --type typescript $'anvilButtonWidget'

Length of output: 75



Script:

#!/bin/bash
# Description: Verify the implementation of `anvilButtonWidget` DSL in the project.

# Test: Search for the DSL implementation. Expect: At least one occurrence in the codebase.
rg --glob '*.ts' 'anvilButtonWidget'

Length of output: 178



Script:

#!/bin/bash
# Description: Verify the implementation of `AddDsl` method in the project.

# Test: Search for the `AddDsl` method implementation. Expect: At least one occurrence in the codebase.
rg 'AddDsl'

Length of output: 55634

Comment on lines 15 to 21
public verifyCanvasMode = (widgetName: string) => {
cy.get(this.locators.canvas).matchImageSnapshot(`anvil${widgetName}LightMode`);

this.setTheme("dark");

cy.get(this.locators.canvas).matchImageSnapshot(`anvil${widgetName}DarkMode`);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The methods for verifying different modes are properly implemented using Cypress commands. Consider adding error handling for cases where elements might not be found or snapshots fail.

Consider implementing error handling in snapshot verification methods to enhance robustness.

Also applies to: 23-27, 29-33

@jsartisan
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9511954647.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 34242.
recreate: .

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Outside diff range and nitpick comments (2)
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

Line range hint 144-159: Specify the type attribute for button elements to prevent unintended form submissions.

- <button
+ <button type="button"
app/client/cypress/support/Objects/Registry.ts (1)

Line range hint 36-308: Consider refactoring the class to use a more flexible structure instead of static-only members.

This could enhance maintainability and testing. If you need assistance with this refactor, feel free to ask.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fa5790cbc71e975bfb76f305489b3a5de7eda576 and e94a275.

Files selected for processing (6)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/fixtures/anvilButtonWidget.json (1 hunks)
  • app/client/cypress/support/Objects/ObjectsCore.ts (1 hunks)
  • app/client/cypress/support/Objects/Registry.ts (2 hunks)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
  • app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1 hunks)
Files not summarized due to errors (1)
  • app/client/cypress/fixtures/anvilButtonWidget.json: Error: Message exceeds token limit
Files skipped from review as they are similar to previous changes (3)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts
  • app/client/cypress/support/Objects/ObjectsCore.ts
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts
Additional context used
Biome
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx

[error] 144-159: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

app/client/cypress/support/Objects/Registry.ts

[error] 36-308: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

Additional comments not posted (3)
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

120-120: Addition of data-testid attribute improves testability of the component.

app/client/cypress/support/Objects/Registry.ts (2)

34-34: The import of AnvilSnapshot enables the use of this object within the ObjectsRegistry class.


269-275: The new static method AnvilSnapshot follows the existing pattern of lazy initialization for singletons in this class.

@jsartisan jsartisan requested a review from a team June 14, 2024 07:30
@jsartisan
Copy link
Contributor Author

@KelvinOm can you check if i correctly implemented the tests?

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e94a275 and 8fd01eb.

Files ignored due to path filters (6)
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetDarkMode.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetLightMode.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetPreviewMode.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidget_spec.ts/anvilButtonWidgetmacbook-13.snap.png is excluded by !**/*.png
Files selected for processing (4)
  • app/client/cypress/locators/commonlocators.json (2 hunks)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
  • app/client/cypress/support/Pages/DeployModeHelper.ts (4 hunks)
  • app/client/src/pages/AppViewer/AppPage.tsx (1 hunks)
Additional comments not posted (3)
app/client/src/pages/AppViewer/AppPage.tsx (1)

49-49: Change from className to data-testid in PageView component enhances testability. Ensure it doesn't impact any related styling or functionalities.

Verification successful

The search results indicate that the new data-testid attribute for t--app-viewer-page is already being used in various Cypress tests and locators, confirming that the change has been integrated into the testing framework. Additionally, no other instances of PageView using className were found.

  • app/client/src/pages/AppViewer/AppPage.tsx: The new data-testid attribute is present.
  • app/client/cypress/support/Pages/DeployModeHelper.ts: Uses [data-testid=t--app-viewer-page].
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts: Uses [data-testid=t--app-viewer-page].
  • app/client/cypress/locators/commonlocators.json: Uses [data-testid='t--app-viewer-page'].

These results confirm that the change to data-testid is consistent and does not impact any styling or functionality.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that no styles or functionalities are broken by the change from className to data-testid.
# Expect: No issues related to styling or functionality.

# Check for any CSS or JS that might be targeting the old className.
rg --type css --type js ".t--app-viewer-page"

Length of output: 45



Script:

#!/bin/bash
# Description: Verify that no other parts of the codebase are impacted by the change from className to data-testid.

# Search for any mentions of the old className in the entire codebase.
rg "t--app-viewer-page"

# Search for any usages of the PageView component with the className attribute.
rg "<PageView[^>]+className"

Length of output: 508

app/client/cypress/support/Pages/DeployModeHelper.ts (1)

11-11: Refactoring of XPath queries and CSS selectors seems well-targeted. Ensure thorough testing to confirm the functionality of these changes.

Also applies to: 21-21, 33-33, 97-99

app/client/cypress/locators/commonlocators.json (1)

151-151: Update of viewerPage selector to use data-testid aligns with modern testing practices. Verify its usage across the test suite to ensure seamless integration.

Verification successful

Update of viewerPage selector to use data-testid aligns with modern testing practices. Verify its usage across the test suite to ensure seamless integration.

  • The old selector .t--app-viewer-page is not found in the test files.
  • The new selector data-testid='t--app-viewer-page' is used in various locations, including AppPage.tsx, DeployModeHelper.ts, and AnvilSnapshot.ts.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the usage of the updated `viewerPage` selector across the test suite.
# Expect: All tests using this selector should be updated accordingly.

# Search for the old and new selector usage in test files.
rg --type test ".t--app-viewer-page"
rg --type test "data-testid='t--app-viewer-page'"

Length of output: 152



Script:

#!/bin/bash
# Description: Verify the usage of the updated `viewerPage` selector across the test suite.
# Expect: All tests using this selector should be updated accordingly.

# Use `fd` to find common test file extensions and then search for the old and new selector usage in those files.
fd --type f --extension js --extension ts | grep -E '\.(spec|test)\.(js|ts)' | xargs rg ".t--app-viewer-page"
fd --type f --extension js --extension ts | grep -E '\.(spec|test)\.(js|ts)' | xargs rg "data-testid='t--app-viewer-page'"

Length of output: 240



Script:

#!/bin/bash
# Description: Verify the usage of the updated `viewerPage` selector across the test suite.
# Expect: All tests using this selector should be updated accordingly.

# Use `fd` to find common test file extensions and directories, then search for the old and new selector usage in those files.
fd --type f --extension js --extension ts --extension jsx --extension tsx --glob "**/test/**" | xargs rg ".t--app-viewer-page"
fd --type f --extension js --extension ts --extension jsx --extension tsx --glob "**/test/**" | xargs rg "data-testid='t--app-viewer-page'"

Length of output: 830

Comment on lines 16 to 24
public verifyCanvasMode = (widgetName: string) => {
cy.get(this.locators.canvas).matchImageSnapshot(`anvil${widgetName}LightMode`);

this.setTheme("dark");

cy.get(this.locators.canvas).matchImageSnapshot(`anvil${widgetName}DarkMode`);

this.setTheme("light");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of snapshot verification methods using Cypress commands is well done. Consider adding error handling for cases where elements might not be found or snapshots fail to enhance robustness.

Also applies to: 26-32, 34-38, 40-46, 48-50, 52-54, 56-61

@jsartisan jsartisan marked this pull request as draft June 14, 2024 08:00
@jsartisan jsartisan added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jun 17, 2024
@jsartisan jsartisan requested a review from KelvinOm June 18, 2024 11:37
@jsartisan jsartisan marked this pull request as ready for review June 18, 2024 11:40
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8fd01eb and dd16197022abe53ef34cbb1059d9a3af8fba926e.

Files ignored due to path filters (24)
  • app/client/cypress/snapshots/anvilButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilIconButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilInlineButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/anvilToolbarButtonWidgetPreview.snap.png is excluded by !**/*.png
Files selected for processing (10)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/IconButtonWidget/AnvilIconButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/InlineButtonWidget/AnvilInlineButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ToolbarButtonWidget/AnvilToolbarButtonWidget_spec.ts (1 hunks)
  • app/client/cypress/fixtures/anvilIconButtonWidget.json (1 hunks)
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json (1 hunks)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
  • app/client/cypress/support/Pages/DeployModeHelper.ts (1 hunks)
  • app/client/cypress/support/index.d.ts (1 hunks)
  • app/client/src/pages/AppViewer/index.tsx (1 hunks)
Files not summarized due to errors (2)
  • app/client/cypress/fixtures/anvilIconButtonWidget.json: Error: Message exceeds token limit
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json: Error: Message exceeds token limit
Files skipped from review due to trivial changes (3)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/InlineButtonWidget/AnvilInlineButtonWidget_spec.ts
  • app/client/cypress/support/Pages/DeployModeHelper.ts
  • app/client/src/pages/AppViewer/index.tsx
Files skipped from review as they are similar to previous changes (1)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts
Additional comments not posted (5)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ButtonWidget/AnvilButtonWidget_spec.ts (1)

7-29: The test structure looks comprehensive and well-organized, covering the different modes for the widget. Ensure that the DSL anvilButtonWidget is correctly implemented as it is critical for these tests to function correctly.

#!/bin/bash
# Description: Verify the implementation of `anvilButtonWidget` DSL in the project.

# Test: Search for the DSL implementation. Expect: At least one occurrence in the codebase.
rg --type typescript $'anvilButtonWidget'
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/IconButtonWidget/AnvilIconButtonWidget_spec.ts (1)

7-29: The test suite is well-structured and seems to cover different operational modes for the Icon Button Widget. Consistency in test structure across different widgets is good for maintainability.

app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/ToolbarButtonWidget/AnvilToolbarButtonWidget_spec.ts (1)

7-29: Excellent consistency in test structure across different widget tests. This should aid in both readability and ease of maintenance. Ensure that all necessary widgets are correctly implemented in the DSL.

app/client/cypress/fixtures/anvilInlineButtonWidget.json (1)

1-2242: The JSON structure appears well-organized and consistent, providing detailed configuration for UI components. Ensure all widget IDs and keys are unique and correctly referenced in the application to prevent runtime errors.

app/client/cypress/fixtures/anvilIconButtonWidget.json (1)

1-2367: The JSON structure is valid and well-formed, ensuring proper loading and parsing during testing.

@jsartisan jsartisan added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jun 19, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between dd16197022abe53ef34cbb1059d9a3af8fba926e and e81063d952a5b8b1148a495c3170462154867d8a.

Files ignored due to path filters (24)
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetPreview.snap.png is excluded by !**/*.png
Files selected for processing (4)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilIconButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilInlineButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilToolbarButtonWidgetSnapshot_spec.ts (1 hunks)
Files skipped from review due to trivial changes (2)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilInlineButtonWidgetSnapshot_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilToolbarButtonWidgetSnapshot_spec.ts
Additional comments not posted (6)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts (3)

1-5: Imports are correctly structured and appropriate for the required functionality in this test suite.


7-29: The structure and setup of the test suite are well-organized, with clear separation of setup, test cases, and teardown. The use of descriptive tags and titles enhances readability and maintainability.


17-27: Each test case is clearly defined and appropriately uses the helper functions to verify the widget's behavior in different modes. This ensures thorough testing of the widget across various scenarios.

app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilIconButtonWidgetSnapshot_spec.ts (3)

1-5: Imports are correctly structured and appropriate for the required functionality in this test suite, similar to the previous file.


7-29: The structure and setup of the test suite are well-organized, with clear separation of setup, test cases, and teardown. The use of descriptive tags and titles enhances readability and maintainability, similar to the previous file.


17-27: Each test case is clearly defined and appropriately uses the helper functions to verify the widget's behavior in different modes. This ensures thorough testing of the widget across various scenarios, similar to the previous file.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between e81063d952a5b8b1148a495c3170462154867d8a and e98dcd026bee9c62b66b1c1247fea7bbf7aa3212.

Files ignored due to path filters (6)
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetPreview.snap.png is excluded by !**/*.png
Files selected for processing (1)
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json (1 hunks)
Files not summarized due to errors (1)
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json: Error: Message exceeds token limit
Additional comments not posted (1)
app/client/cypress/fixtures/anvilInlineButtonWidget.json (1)

1-2523: The JSON structure appears correctly formatted and follows a consistent structure for defining the widget layout. Ensure that all widget IDs and properties are consistent with the application's requirements and that no deprecated or unused properties are included.

@jsartisan
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9595949257.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 34242.
recreate: .

Copy link

Deploy-Preview-URL: https://ce-34242.dp.appsmith.com

@jsartisan jsartisan requested a review from KelvinOm June 20, 2024 11:27
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

Line range hint 144-159: Ensure all buttons specify an explicit type attribute to prevent unexpected form submissions.

- <button
+ <button type="button"
app/client/cypress/support/Objects/Registry.ts (1)

269-275: The addition of AnvilSnapshot follows existing patterns for singleton access, which is consistent but consider alternatives to static-only classes for better scalability and testability.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d9e953e229d1c7b4d1328f718512db5d597a6687 and c551705.

Files ignored due to path filters (20)
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilButtonWidgetSnapshot_spec.ts/anvilButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilIconButtonWidgetSnapshot_spec.ts/anvilIconButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilInlineButtonWidgetSnapshot_spec.ts/anvilInlineButtonWidgetPreview.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetCanvas.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetCanvasDark.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeployipad-2.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeployiphone-6.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetDeploymacbook-13.snap.png is excluded by !**/*.png
  • app/client/cypress/snapshots/AnvilToolbarButtonWidgetSnapshot_spec.ts/anvilToolbarButtonWidgetPreview.snap.png is excluded by !**/*.png
Files selected for processing (17)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilIconButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilInlineButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilToolbarButtonWidgetSnapshot_spec.ts (1 hunks)
  • app/client/cypress/fixtures/anvilButtonWidget.json (1 hunks)
  • app/client/cypress/fixtures/anvilIconButtonWidget.json (1 hunks)
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json (1 hunks)
  • app/client/cypress/locators/commonlocators.json (2 hunks)
  • app/client/cypress/support/Objects/ObjectsCore.ts (1 hunks)
  • app/client/cypress/support/Objects/Registry.ts (2 hunks)
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts (1 hunks)
  • app/client/cypress/support/Pages/DeployModeHelper.ts (1 hunks)
  • app/client/cypress/support/index.d.ts (1 hunks)
  • app/client/packages/design-system/widgets/src/components/InlineButtons/src/styles.module.css (1 hunks)
  • app/client/src/pages/AppViewer/AppPage.tsx (1 hunks)
  • app/client/src/pages/AppViewer/index.tsx (1 hunks)
  • app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1 hunks)
Files not summarized due to errors (3)
  • app/client/cypress/fixtures/anvilButtonWidget.json: Error: Message exceeds token limit
  • app/client/cypress/fixtures/anvilIconButtonWidget.json: Error: Message exceeds token limit
  • app/client/cypress/fixtures/anvilInlineButtonWidget.json: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
  • app/client/cypress/support/Objects/ObjectsCore.ts
  • app/client/packages/design-system/widgets/src/components/InlineButtons/src/styles.module.css
Files skipped from review as they are similar to previous changes (9)
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilButtonWidgetSnapshot_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilIconButtonWidgetSnapshot_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilInlineButtonWidgetSnapshot_spec.ts
  • app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilToolbarButtonWidgetSnapshot_spec.ts
  • app/client/cypress/locators/commonlocators.json
  • app/client/cypress/support/Pages/Anvil/AnvilSnapshot.ts
  • app/client/cypress/support/Pages/DeployModeHelper.ts
  • app/client/src/pages/AppViewer/AppPage.tsx
  • app/client/src/pages/AppViewer/index.tsx
Additional context used
Biome
app/client/cypress/support/index.d.ts

[error] 66-66: Duplicate parameter name. (lint/suspicious/noDuplicateParameters)

The parameter overrides a preceding parameter by using the same name.


[error] 70-70: Duplicate parameter name. (lint/suspicious/noDuplicateParameters)

The parameter overrides a preceding parameter by using the same name.


[error] 75-75: Duplicate parameter name. (lint/suspicious/noDuplicateParameters)

The parameter overrides a preceding parameter by using the same name.


[error] 130-130: This optional parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)

The last required parameter is here:

A optional parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.


[error] 66-66: Shouldn't redeclare 'number'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)

'number' is defined here:


[error] 70-70: Shouldn't redeclare 'number'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)

'number' is defined here:


[error] 75-75: Shouldn't redeclare 'number'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)

'number' is defined here:

app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx

[error] 144-159: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

app/client/cypress/support/Objects/Registry.ts

[error] 36-308: Avoid classes that contain only static members. (lint/complexity/noStaticOnlyClass)

Prefer using simple functions instead of classes with only static members.

Additional comments not posted (3)
app/client/src/pages/Editor/WDSThemePropertyPane/index.tsx (1)

120-120: Adding data-testid to SegmentedControl enhances testability and is a good practice.

app/client/cypress/fixtures/anvilIconButtonWidget.json (1)

1-2367: The JSON structure appears to be well-formed and adheres to the expected schema for widget configurations. Ensure all dynamic bindings and properties are correctly set up and match the expected values for your tests.

app/client/cypress/fixtures/anvilInlineButtonWidget.json (1)

1-2625: The JSON structure is well-formed and correctly defines a complex layout for widget testing. Ensure all widgetId and related properties are correctly matched with actual widget definitions in the application to prevent reference errors.

"dynamicBindingPathList": [],
"text": "Do something",
"isDisabled": false,
"key": "bn9xz1h2d0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate key issue detected.

The key "bn9xz1h2d0" is used multiple times across different widget configurations. Each widget should have a unique key to prevent conflicts and ensure correct behavior.

Please ensure that each widget has a unique key or adjust the logic that handles these keys if they are intentionally shared.

Also applies to: 237-237, 289-289, 341-341, 393-393, 445-445, 497-497, 549-549, 601-601, 653-653, 705-705, 757-757, 809-809, 861-861, 913-913, 965-965, 1018-1018, 1177-1177, 1543-1543, 1601-1601, 1653-1653, 1876-1876, 2236-2236, 2518-2518

Comment on lines +180 to +183
matchImageSnapshot(
name: string,
options?: Partial<Cypress.ScreenshotOptions>,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure matchImageSnapshot is well-documented and handles edge cases effectively.

Would you like assistance with documenting the options for matchImageSnapshot or developing edge case tests?

@jsartisan jsartisan added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jun 20, 2024
@jsartisan
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9597486700.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 34242.
recreate: .

Copy link

Deploy-Preview-URL: https://ce-34242.dp.appsmith.com

@jsartisan jsartisan merged commit e01b34e into release Jun 20, 2024
51 checks passed
@jsartisan jsartisan deleted the chore/button-widget-tests branch June 20, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants