test: Add cypress tests for Hubspot Datasource functionalities #39383
Conversation
Signed-off-by: Laveena Enid <[email protected]>
Signed-off-by: Laveena Enid <[email protected]>
Signed-off-by: Laveena Enid <[email protected]>
laveena-en
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThis pull request introduces a new Cypress test suite for validating Hubspot datasource functionality. The tests cover datasource configuration, query creation and execution, widget binding and deployment, datasource deletion, and error handling. Additionally, the changes add a new Hubspot API token property in the DataManager, extend the DataSources class with a new method for filling out Hubspot forms, and include a new tag for Hubspot tests. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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
|
github-actions
bot
added
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts(1 hunks)app/client/cypress/support/Objects/DataManager.ts(2 hunks)app/client/cypress/support/Pages/DataSources.ts(2 hunks)app/client/cypress/tags.js(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/DataManager.tsapp/client/cypress/tags.jsapp/client/cypress/support/Pages/DataSources.tsapp/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
🔇 Additional comments (4)
app/client/cypress/tags.js (1)
29-29: LGTM! Tag added in correct alphabetical order.app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts (1)
18-129: Well-structured test suite with comprehensive coverage!The test suite effectively covers:
- Datasource configuration validation
- Query creation and execution
- Widget binding and deployment
- Datasource deletion
- Error handling
app/client/cypress/support/Objects/DataManager.ts (1)
120-120: LGTM! Secure token handling using environment variables.The Hubspot bearer token is properly managed using Cypress environment variables for both Production and Staging environments.
Also applies to: 222-222
app/client/cypress/support/Pages/DataSources.ts (1)
612-628: LGTM! Well-structured form fill method.The FillHubspotDSForm method follows the established pattern and properly handles authentication type and bearer token.
| dataSources.FillHubspotDSForm(undefined, "wrongpassword"); | ||
| dataSources.SaveDatasource(false); | ||
| dataSources.CreateQueryForDS("Untitled datasource 1"); | ||
| agHelper.RefreshPage(); // Refreshing the page due to frequent connection reset from Hubspot |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove agHelper.RefreshPage() call.
Consider using proper wait conditions or retry mechanisms instead of refreshing the page to handle connection reset from Hubspot.
| PageLeftPane.switchSegment(PagePaneSegment.UI); // Switching the tab to ensure connection reset from Hubspot platform gets refreshed | ||
| PageLeftPane.switchSegment(PagePaneSegment.Queries); | ||
| cy.get("@postExecute").then((resObj: any) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace tab switching with proper wait conditions.
Instead of switching tabs to refresh connection, implement proper wait conditions or retry mechanisms.
-PageLeftPane.switchSegment(PagePaneSegment.UI); // Switching the tab to ensure connection reset from Hubspot platform gets refreshed
-PageLeftPane.switchSegment(PagePaneSegment.Queries);
+// Add proper wait condition for connection reset
+cy.wait("@hubspotConnection").its("response.statusCode").should("eq", 200);Committable suggestion skipped: line range outside the PR's diff.
| @@ -317,6 +317,7 @@ export class DataSources { | |||
| private _entityTriggerElement = ".t--template-menu-trigger"; | |||
| _dsSchemaTableResponse = ".t--table-response"; | |||
| _imgSnowflakeLogo = "//img[contains(@src, 'snowflake.svg')]"; | |||
| _imgHubspotLogo = "//img[contains(@src, 'hubspot.png')]"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace XPath selector with data- attribute.*
Using XPath selectors is not recommended. Use data-* attributes instead.
-_imgHubspotLogo = "//img[contains(@src, 'hubspot.png')]";
+_imgHubspotLogo = "[data-testid='hubspot-logo-img']";📝 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.
| _imgHubspotLogo = "//img[contains(@src, 'hubspot.png')]"; | |
| _imgHubspotLogo = "[data-testid='hubspot-logo-img']"; |
Signed-off-by: Laveena Enid <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/limited-tests.txt (1)
8-8: Comment Formatting Suggestion
The comment on this line could be slightly improved for clarity. For example, consider changing it to:
#ci-test-limit uses this file to run a minimal set of specs. Do not run the entire suite with this command.
This minor tweak improves readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Possible missing article found.
Context: ...ile to run minimum of specs. Do not run entire suite with this command.(AI_HYDRA_LEO_MISSING_THE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/limited-tests.txt(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/limited-tests.txt
🪛 LanguageTool
app/client/cypress/limited-tests.txt
[uncategorized] ~8-~8: Possible missing article found.
Context: ...ile to run minimum of specs. Do not run entire suite with this command.
(AI_HYDRA_LEO_MISSING_THE)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/cypress/limited-tests.txt (1)
3-3: Hubspot Test Spec Inclusion
The new test specification for Hubspot is correctly added. Please ensure that the corresponding spec file (Hubspot_Spec.ts) adheres to Cypress best practices (e.g., using data-* attributes for selectors, avoidingcy.wait, etc.) in its implementation.
|
/ci-test-limit-count run_count=1 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13450903853. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13450903853. |
Signed-off-by: Laveena Enid <[email protected]>
|
/ci-test-limit-count run_count=1 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13452096290. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13452096290. |
Signed-off-by: Laveena Enid <[email protected]>
|
/ci-test-limit-count run_count=1 update_snapshot=false specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13453258787. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts (4)
22-22: Remove unnecessary tags.The tags
@tag.Gitand@tag.AccessControlseem unrelated to Hubspot datasource testing. Consider removing them to maintain accurate test categorization.- tags: ["@tag.Datasource", "@tag.Git", "@tag.AccessControl", "@tag.Hubspot"], + tags: ["@tag.Datasource", "@tag.Hubspot"],
84-91: Remove commented code block.Remove the commented code block as it's no longer needed and can cause confusion.
- // agHelper - // .GetElement(locators._widgetInDeployed(draggableWidgets.TEXT)) - // .then(($elements) => { - // const values = $elements - // .map((_, el) => Cypress.$(el).text().trim()) - // .get(); - // expect(values).to.include("appsmith1"); - // });
64-69: Improve response validation.Combine multiple assertions for more comprehensive validation of the response.
cy.get("@postExecute").then((resObj: any) => { const json = resObj.response.body.data.body; const name = json.name; - cy.log("Name is :" + name); - expect(name).to.equal("appsmith1"); //Verify if record contains the table + expect(json).to.deep.include({ + name: "appsmith1", + // Add more expected properties + }); });
80-83: Use agHelper methods for widget assertions.Replace direct cy.get calls with agHelper methods for consistent selector usage across tests.
- cy.get(locators._widgetInDeployed(draggableWidgets.TEXT)).should( - "contain.text", - "appsmith1", - ); + agHelper.AssertElementText( + locators._widgetInDeployed(draggableWidgets.TEXT), + "appsmith1" + );Also applies to: 107-110
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.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/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts (2)
125-125: Remove agHelper.RefreshPage() call.Consider using proper wait conditions or retry mechanisms instead of refreshing the page to handle connection reset from Hubspot.
62-63: Replace tab switching with proper wait conditions.Instead of switching tabs to refresh connection, implement proper wait conditions or retry mechanisms.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13453258787. |
|
/ci-test-limit-count run_count=20 update_snapshot=false runId=13453258787 specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13455468849. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13455468849. |
|
/ci-test-limit-count run_count=5 update_snapshot=false runId=13453258787 specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13456398195. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13456398195. |
Signed-off-by: Laveena Enid <[email protected]>
|
/ci-test-limit-count run_count=1 update_snapshot=false specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_Spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13461115389. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13461115389. |
Signed-off-by: Laveena Enid [email protected]## Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD e96cdde yet
Fri, 21 Feb 2025 16:34:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Signed-off-by: Laveena Enid [email protected]Signed-off-by: Laveena Enid [email protected]
Summary by CodeRabbit
New Features
Tests