-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: custom widget regression tests #39101
base: release
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a new end-to-end test file for validating the property pane of a custom widget using Cypress. It adds a JSON fixture that defines a custom widget with two child widgets, updates a limited-tests file to reference the new test, and adds new locator properties in support files for custom widget interactions and default model references. Changes
Sequence Diagram(s)sequenceDiagram
participant Cypress as "Cypress Test Runner"
participant DSL as "DSL Loader"
participant IFrame as "IFrame Body"
participant Pane as "Property Pane"
participant Widget as "Custom Widget"
Cypress->>DSL: Load custom widget DSL
DSL-->>Widget: Initialize widget
Cypress->>IFrame: Retrieve iframe body via getIframeBody()
IFrame-->>Cypress: Return document body for assertions
Cypress->>Pane: Verify tips and default model details
Pane-->>Cypress: Display property pane data
Cypress->>Widget: Trigger UI actions (toggle visibility, update properties)
Widget-->>Cypress: Reflect updated model and CSS changes
Cypress->>Widget: Execute alert triggering and observe JS property responses
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
/ci-test-limit-count run_count= 2 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13192608479. |
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: 4
🔭 Outside diff range comments (1)
app/client/cypress/fixtures/customWidgetWithSrc.json (1)
149-150
: Remove Extra Trailing Content:
A stray line containing "150" appears after the closing bracket, which would render the JSON invalid. Please remove this extra line.Suggested diff:
-150
🧹 Nitpick comments (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidget_PropertyPane_Validation.ts (2)
74-75
: Replace agHelper.ValidateToastMessage with cy.contains assertion.Using
agHelper.ValidateToastMessage
adds unnecessary complexity. Use Cypress's built-in assertions.- agHelper.ValidateToastMessage("Reset Clicked"); + cy.contains(".t--toast-action", "Reset Clicked").should("exist");
84-86
: Use RGB values consistently.The test asserts "rgb(239, 117, 65)" but sets "#eab308". Use consistent color formats.
- propPane.UpdatePropertyFieldValue("Background color", "#eab308"); + propPane.UpdatePropertyFieldValue("Background color", "rgb(239, 117, 65)");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidget_PropertyPane_Validation.ts
(1 hunks)app/client/cypress/fixtures/customWidgetWithSrc.json
(1 hunks)app/client/cypress/limited-tests.txt
(1 hunks)app/client/cypress/support/Objects/CommonLocators.ts
(1 hunks)app/client/cypress/support/Pages/PropertyPane.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test ...
app/client/cypress/**/**.*
: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/support/Objects/CommonLocators.ts
app/client/cypress/support/Pages/PropertyPane.ts
app/client/cypress/fixtures/customWidgetWithSrc.json
app/client/cypress/limited-tests.txt
app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidget_PropertyPane_Validation.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (4)
app/client/cypress/support/Pages/PropertyPane.ts (1)
184-184
: LGTM!The selector follows the established pattern and uses the recommended data-testid attribute.
app/client/cypress/limited-tests.txt (1)
2-2
: LGTM!The test file path is correctly formatted and follows the established pattern.
app/client/cypress/fixtures/customWidgetWithSrc.json (2)
1-22
: Main Container DSL Configuration:
The DSL container structure is clearly defined and well-organized. All layout properties (e.g., snapColumns, snapRows, parentRowSpace) and mustache bindings (e.g., for theme) appear correctly specified.
23-64
: Custom Widget "Custom1" Configuration:
The configuration for the "Custom1" widget is well-constructed. Its "srcDoc" section cleanly separates HTML, CSS, and JS. The JS code correctly uses React (with proper useEffect for model changes) and the event "onResetClick" is set up using a mustache binding. No issues detected in this section.
] | ||
}`; | ||
const custom2WidgetBoundary = | ||
"//div[@data-widgetname-cy='Custom2']//div[contains(@class,'widget-component-boundary')]//div//div"; |
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.
🛠️ Refactor suggestion
Replace XPath with data-testid attribute.
Using XPath selectors is discouraged in Cypress tests. Use data-testid attributes instead.
- "//div[@data-widgetname-cy='Custom2']//div[contains(@class,'widget-component-boundary')]//div//div";
+ "[data-testid='custom2-widget-boundary']";
return cy | ||
.get(".t--widget-customwidget iframe") | ||
.last() | ||
.its("0.contentDocument.body") | ||
.should("not.be.empty") | ||
.then(cy.wrap); |
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.
🛠️ Refactor suggestion
Avoid using cy.get().last() for iframe selection.
Using .last()
to select an iframe is fragile and may break if more iframes are added. Consider using a more specific selector.
- .get(".t--widget-customwidget iframe")
- .last()
+ .get(".t--widget-customwidget iframe[data-testid='custom-widget-iframe']")
_draggableCustomWidget = | ||
"//div[contains(@class,'t--draggable-customwidget')]"; |
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.
🛠️ Refactor suggestion
Replace XPath with data-testid attribute for custom widget selector.
Using XPath selectors is discouraged. Use data-testid attributes for better maintainability.
- _draggableCustomWidget =
- "//div[contains(@class,'t--draggable-customwidget')]";
+ _draggableCustomWidget = "[data-testid='draggable-custom-widget']";
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
_draggableCustomWidget = | |
"//div[contains(@class,'t--draggable-customwidget')]"; | |
_draggableCustomWidget = "[data-testid='draggable-custom-widget']"; |
{ | ||
"needsErrorInfo": false, | ||
"boxShadow": "{{appsmith.theme.boxShadow.appBoxShadow}}", | ||
"mobileBottomRow": 75, | ||
"widgetName": "Custom2", | ||
"borderColor": "#E0DEDE", | ||
"srcDoc": { | ||
"html": "<!-- no need to write html, head, body tags, it is handled by the widget -->\n<div id=\"root\"></div>\n", | ||
"css": ".app {\n\twidth: calc(var(--appsmith-ui-width) * 1px);\n\tjustify-content: center;\n\tborder-radius: 0px;\n\tborder: none;\n}\n\n.tip-container {\n margin-bottom: 20px;\n}\n\n.tip-container h2 {\n margin-bottom: 20px;\n\tfont-size: 16px;\n\tfont-weight: 700;\n}\n\n.tip-header {\n\tdisplay: flex;\n\tjustify-content: space-between;\n\talign-items: baseline;\n}\n\n.tip-header div {\n\tcolor: #999;\n}\n\n.button-container {\n\ttext-align: right;\t\n}\n\n.button-container button {\n margin: 0 10px;\n\tborder-radius: var(--appsmith-theme-borderRadius) !important;\n}\n\n.button-container button.primary {\n\tbackground: var(--appsmith-theme-primaryColor) !important;\n}\n\n.button-container button.reset:not([disabled]) {\n\tcolor: var(--appsmith-theme-primaryColor) !important;\n\tborder-color: var(--appsmith-theme-primaryColor) !important;\n}", | ||
"js": "import React from 'https://cdn.jsdelivr.net/npm/react@18.2.0/+esm';\nimport reactDom from 'https://cdn.jsdelivr.net/npm/react-dom@18.2.0/+esm';\nimport { Button, Card } from 'https://cdn.jsdelivr.net/npm/antd@5.11.1/+esm';\nimport Markdown from 'https://cdn.jsdelivr.net/npm/react-markdown@9.0.1/+esm';\n\nfunction App() {\n const [currentIndex, setCurrentIndex] = React.useState(0);\n const handleNext = () => {\n setCurrentIndex(prevIndex => (prevIndex + 1) % appsmith.model.tips.length);\n };\n const handleReset = () => {\n setCurrentIndex(0);\n appsmith.triggerEvent(\"onResetClick\");\n };\n return /*#__PURE__*/React.createElement(Card, {\n className: \"app\",\n }, /*#__PURE__*/React.createElement(\"div\", {\n className: \"tip-container\"\n }, /*#__PURE__*/React.createElement(\"div\", {\n className: \"tip-header\"\n }, /*#__PURE__*/React.createElement(\"h2\", null, \"Custom Widget\"), /*#__PURE__*/React.createElement(\"div\", null, currentIndex + 1, \" / \", appsmith.model.tips.length, \" \")), /*#__PURE__*/React.createElement(Markdown, null, appsmith.model.tips[currentIndex])), /*#__PURE__*/React.createElement(\"div\", {\n className: \"button-container\"\n }, /*#__PURE__*/React.createElement(Button, {\n className: \"primary\",\n onClick: handleNext,\n type: \"primary\"\n }, \"Next Tip\"), /*#__PURE__*/React.createElement(Button, {\n\tclassName: \"reset\",\n\tdisabled: currentIndex === 0,\n onClick: handleReset\n }, \"Reset\")));\n}\nappsmith.onReady(() => {\n reactDom.render( /*#__PURE__*/React.createElement(App, null), document.getElementById(\"root\"));\n});" | ||
}, | ||
"isCanvas": false, | ||
"topRow": 43, | ||
"bottomRow": 73, | ||
"parentRowSpace": 10, | ||
"type": "CUSTOM_WIDGET", | ||
"mobileRightColumn": 41, | ||
"dynamicTriggerPathList": [ | ||
{ | ||
"key": "onResetClick" | ||
} | ||
], | ||
"parentColumnSpace": 14.21875, | ||
"dynamicBindingPathList": [ | ||
{ | ||
"key": "theme" | ||
}, | ||
{ | ||
"key": "borderRadius" | ||
}, | ||
{ | ||
"key": "boxShadow" | ||
} | ||
], | ||
"leftColumn": 19, | ||
"defaultModel": "{\n \"tips\": [\n \"Pass data to this widget in the default model field\",\n \"Access data in the javascript file using the appsmith.model variable\",\n \"Create events in the widget and trigger them in the javascript file using appsmith.triggerEvent('eventName')\",\n \"Access data in CSS as var(--appsmith-model-{property-name})\"\n ]\n}", | ||
"borderWidth": "1", | ||
"theme": "{{appsmith.theme}}", | ||
"onResetClick": "{{showAlert('Successfully reset!!', '');}}", | ||
"events": ["onResetClick"], | ||
"key": "podxgd85w9", | ||
"backgroundColor": "#FFFFFF", | ||
"rightColumn": 42, | ||
"dynamicHeight": "FIXED", | ||
"isSearchWildcard": true, | ||
"widgetId": "9iep04z42x", | ||
"isVisible": true, | ||
"version": 1, | ||
"uncompiledSrcDoc": { | ||
"html": "<!-- no need to write html, head, body tags, it is handled by the widget -->\n<div id=\"root\"></div>\n", | ||
"css": ".app {\n width: calc(1px * var(--appsmith-ui-width));\n justify-content: center;\n border-radius: 0px;\n border: none;\n \n .tip-container {\n margin-bottom: 20px;\n\n h2 {\n margin-bottom: 20px;\n font-size: 16px;\n font-weight: 700;\n }\n\n .tip-header {\n display: flex;\n justify-content: space-between;\n align-items: baseline;\n\n div {\n color: #999;\n }\n }\n }\n\t\n\t.button-container {\n text-align: right;\n\n button {\n margin: 0 10px;\n border-radius: var(--appsmith-theme-borderRadius) !important;\n\n &.primary {\n background: var(--appsmith-theme-primaryColor) !important;\n }\n\n &.reset:not([disabled]) {\n color: var(--appsmith-theme-primaryColor) !important;\n border-color: var(--appsmith-theme-primaryColor) !important;\n }\n }\n }\n}\n", | ||
"js": "import React from 'https://cdn.jsdelivr.net/npm/react@18.2.0/+esm'\nimport reactDom from 'https://cdn.jsdelivr.net/npm/react-dom@18.2.0/+esm'\nimport { Button, Card } from 'https://cdn.jsdelivr.net/npm/antd@5.11.1/+esm'\nimport Markdown from 'https://cdn.jsdelivr.net/npm/react-markdown@9.0.1/+esm';\n\nfunction App() {\n\tconst [currentIndex, setCurrentIndex] = React.useState(0);\n\n\tconst handleNext = () => {\n\t\tsetCurrentIndex((prevIndex) => (prevIndex + 1) % appsmith.model.tips.length);\n\t};\n\n\tconst handleReset = () => {\n\t\tsetCurrentIndex(0);\n\t\tappsmith.triggerEvent(\"onResetClick\");\n\t};\n\n\treturn (\n\t\t<Card className=\"app\">\n\t\t\t<div className=\"tip-container\">\n\t\t\t\t<div className=\"tip-header\">\n\t\t\t\t\t<h2>Custom Widget</h2>\n\t\t\t\t\t<div>{currentIndex + 1} / {appsmith.model.tips.length}\t\t</div>\n\t\t\t\t</div>\n\t\t\t\t<Markdown>{appsmith.model.tips[currentIndex]}</Markdown>\n\t\t\t</div>\n\t\t\t<div className=\"button-container\">\n\t\t\t\t<Button className=\"primary\" onClick={handleNext} type=\"primary\">Next Tip</Button>\n\t\t\t\t<Button className=\"reset\" disabled={currentIndex === 0} onClick={handleReset}>Reset</Button>\n\t\t\t</div>\n\t</Card>\n);\n}\n\nappsmith.onReady(() => {\n\t/*\n\t * This handler function will get called when parent application is ready.\n\t * Initialize your component here\n\t * more info - https://docs.appsmith.com/reference/widgets/custom#onready\n\t */\n\treactDom.render(<App />, document.getElementById(\"root\"));\n});" | ||
}, | ||
"parentId": "0", | ||
"renderMode": "CANVAS", | ||
"isLoading": false, | ||
"mobileTopRow": 45, | ||
"borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", | ||
"mobileLeftColumn": 18, | ||
"maxDynamicHeight": 9000, |
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.
Custom Widget "Custom2" Configuration:
While the overall structure for "Custom2" is consistent with "Custom1," a few points need attention:
- The "key" property on line 125 has the same value ("podxgd85w9") as used in "Custom1" (line 63). Unique keys are typically expected to avoid identification or rendering issues.
- In the "uncompiledSrcDoc" (lines 133-137), the CSS block contains nested selectors. Plain CSS does not allow nesting unless a preprocessor (e.g., Sass) is in use; please verify if this is intentional.
- Additionally, the import of react-dom is done as "reactDom" (in contrast to "ReactDOM" in "Custom1"); standardizing the naming might help maintain consistency.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13192608479.
|
/ci-test-limit-count run_count=20 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13221841690. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13221841690.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
/ok-to-test tags="@tag.Sanity"
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13419392102
Commit: 2b1b464
Cypress dashboard.
Tags:
@tag.Sanity
Spec:
Wed, 19 Feb 2025 18:53:49 UTC