test: Add cypress tests for Hubspot Datasource functionalities #39419
Conversation
Signed-off-by: Laveena Enid <[email protected]>
laveena-en
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThis pull request implements a suite of Cypress end-to-end tests for Hubspot datasource functionality. A new test file defines five cases verifying datasource configuration, query execution, widget binding, deletion, and error handling. The update also adds the new test path to the limited test specification file, integrates a Hubspot token into environment configurations within DataManager, and enhances the DataSources class with Hubspot-specific form handling (including a logo reference). Additionally, a new Hubspot tag is added to the tagging module. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Runner
participant DS as DataSources
participant DM as DataManager
participant HS as HubSpot API
participant W as Widget
%% Successful Flow
T->>DS: Call FillHubspotDSForm(valid token)
DS->>DM: Retrieve hubspotBearerToken
DM-->>DS: Return token
DS->>DS: Validate & fill form fields
T->>HS: Execute query with valid config
HS-->>T: Return query result
T->>W: Bind result to widget
W-->>T: Display expected data
%% Error Handling Flow
T->>DS: Call FillHubspotDSForm(invalid token)
DS->>DM: Retrieve hubspotBearerToken
DM-->>DS: Return token (invalid)
T->>HS: Execute query with invalid config
HS-->>T: Return error response
T--)T: Verify error message display
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=1 update_snapshot=false specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.ts |
github-actions
bot
added
skip-changelog
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13497602031. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.ts (3)
93-94: Remove commented code.Remove commented code that's not being used. If it's needed for documentation, add it to the test description.
-// PageLeftPane.switchSegment(PagePaneSegment.UI); // Switching the tab to ensure connection reset from Hubspot platform gets refreshed -// PageLeftPane.switchSegment(PagePaneSegment.Queries);
115-123: Remove commented code block.Large blocks of commented code should be removed if they're not being used.
-// agHelper -// .GetElement(locators._widgetInDeployed(draggableWidgets.TEXT)) -// .then(($elements) => { -// const values = $elements -// .map((_, el) => Cypress.$(el).text().trim()) -// .get(); -// expect(values).to.include("appsmith1"); -// });
20-25: Consider splitting test suite by functionality.The test suite has multiple tags including Git and AccessControl, but the tests only cover Hubspot datasource functionality.
- tags: ["@tag.Datasource", "@tag.Git", "@tag.AccessControl", "@tag.Hubspot"], + tags: ["@tag.Datasource", "@tag.Hubspot"],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.ts(1 hunks)app/client/cypress/limited-tests.txt(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/limited-tests.txtapp/client/cypress/tags.jsapp/client/cypress/support/Objects/DataManager.tsapp/client/cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.tsapp/client/cypress/support/Pages/DataSources.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 (3)
app/client/cypress/tags.js (1)
29-29: LGTM! Tag addition follows conventions.The new Hubspot tag is correctly placed in alphabetical order and follows the established naming pattern.
app/client/cypress/limited-tests.txt (1)
3-3: LGTM! Test spec path follows conventions.The Hubspot test specification path follows the established pattern for regression tests.
app/client/cypress/support/Objects/DataManager.ts (1)
120-120: LGTM! Bearer token configuration follows patterns.The Hubspot bearer token is correctly configured for both environments using environment variables, consistent with other datasource configurations.
Also applies to: 222-222
| @@ -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 with data- attribute selector.*
Using XPath selectors is against the coding guidelines. Use data-* attributes for more reliable and maintainable selectors.
- _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']"; |
| public FillHubspotDSForm( | ||
| environment = this.dataManager.defaultEnviorment, | ||
| password = "", | ||
| ) { | ||
| this.ValidateNSelectDropdown( | ||
| "Authentication type", | ||
| "Please select an option", | ||
| "Bearer token", | ||
| ); | ||
| this.agHelper.TypeText( | ||
| this.locator._inputFieldByName("Bearer token") + | ||
| "//" + | ||
| this.locator._inputField, | ||
| !password | ||
| ? this.dataManager.dsValues[environment].hubspotBearerToken | ||
| : password, | ||
| ); | ||
| this.agHelper.Sleep(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove agHelper.Sleep() and use proper wait conditions.
Using Sleep() is against the coding guidelines. Use proper wait conditions or assertions instead.
public FillHubspotDSForm(
environment = this.dataManager.defaultEnviorment,
password = "",
) {
this.ValidateNSelectDropdown(
"Authentication type",
"Please select an option",
"Bearer token",
);
this.agHelper.TypeText(
this.locator._inputFieldByName("Bearer token") +
"//" +
this.locator._inputField,
!password
? this.dataManager.dsValues[environment].hubspotBearerToken
: password,
);
- this.agHelper.Sleep();
+ this.agHelper.WaitUntilAllToastsDisappear();
}📝 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.
| public FillHubspotDSForm( | |
| environment = this.dataManager.defaultEnviorment, | |
| password = "", | |
| ) { | |
| this.ValidateNSelectDropdown( | |
| "Authentication type", | |
| "Please select an option", | |
| "Bearer token", | |
| ); | |
| this.agHelper.TypeText( | |
| this.locator._inputFieldByName("Bearer token") + | |
| "//" + | |
| this.locator._inputField, | |
| !password | |
| ? this.dataManager.dsValues[environment].hubspotBearerToken | |
| : password, | |
| ); | |
| this.agHelper.Sleep(); | |
| } | |
| public FillHubspotDSForm( | |
| environment = this.dataManager.defaultEnviorment, | |
| password = "", | |
| ) { | |
| this.ValidateNSelectDropdown( | |
| "Authentication type", | |
| "Please select an option", | |
| "Bearer token", | |
| ); | |
| this.agHelper.TypeText( | |
| this.locator._inputFieldByName("Bearer token") + | |
| "//" + | |
| this.locator._inputField, | |
| !password | |
| ? this.dataManager.dsValues[environment].hubspotBearerToken | |
| : password, | |
| ); | |
| this.agHelper.WaitUntilAllToastsDisappear(); | |
| } |
| cy.wait(assertHelper.GetAliasName("@postExecute"), { | ||
| timeout: responseTimeout, | ||
| }) |
There was a problem hiding this comment.
Remove cy.wait and agHelper.Sleep calls.
According to the coding guidelines:
- Avoid using cy.wait in code
- Avoid using agHelper.sleep()
Replace the wait and sleep calls with proper Cypress commands that wait for specific conditions:
-cy.wait(assertHelper.GetAliasName("@postExecute"), {
- timeout: responseTimeout,
-});
+cy.intercept('POST', '**/postExecute').as('postExecute');
+cy.get(assertHelper.GetAliasName("@postExecute")).should('exist');
-assertHelper.Sleep();
+cy.get(assertHelper.GetAliasName("@postExecute")).should('not.exist');Also applies to: 84-84
| it("1. Validate the configuration of Hubspot datasource", () => { | ||
| dataSources.NavigateToDSCreateNew(); | ||
| dataSources.CreatePlugIn("HubSpot"); | ||
| agHelper.AssertElementVisibility(dataSources._imgHubspotLogo, true, 0); // Ensure the Hubspot logo is visible |
There was a problem hiding this comment.
Use data- attributes for selectors.*
According to the guidelines, we should:
- Use data-* attributes for selectors
- Avoid using plain strings in assertions
Replace the current selectors with data-* attributes:
-agHelper.AssertElementVisibility(dataSources._imgHubspotLogo, true, 0);
+agHelper.AssertElementVisibility('[data-testid="hubspot-logo"]', true, 0);
-cy.get(locators._widgetInDeployed(draggableWidgets.TEXT)).should(
- "contain.text",
- "appsmith1",
-);
+cy.get('[data-testid="text-widget"]').should(($el) => {
+ expect($el.text().trim()).to.equal("appsmith1");
+});
-cy.get(locators._widgetInDeployed(draggableWidgets.TEXT)).should(
- "have.text",
- "",
-);
+cy.get('[data-testid="text-widget"]').should(($el) => {
+ expect($el.text().trim()).to.equal("");
+});Also applies to: 111-114, 138-141
| 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.
Remove page refresh call.
The code uses agHelper.RefreshPage() which should be avoided. Instead, handle the connection reset gracefully using proper test setup and teardown.
-agHelper.RefreshPage(); // Refreshing the page due to frequent connection reset from Hubspot
+// Handle connection reset by implementing proper retry mechanism or connection handling📝 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.
| agHelper.RefreshPage(); // Refreshing the page due to frequent connection reset from Hubspot | |
| // Handle connection reset by implementing proper retry mechanism or connection handling |
| const fireApi = (retries = 5, responseTimeout = 100000) => { | ||
| if (retries === 0) { | ||
| throw new Error("Max retries reached, API did not return success."); | ||
| } | ||
|
|
||
| dataSources.RunQuery({ | ||
| toValidateResponse: false, | ||
| }); | ||
| cy.wait(assertHelper.GetAliasName("@postExecute"), { | ||
| timeout: responseTimeout, | ||
| }) | ||
| .then((interceptions) => { | ||
| return cy | ||
| .get(assertHelper.GetAliasName("@postExecute"), { | ||
| timeout: responseTimeout, | ||
| }) | ||
| .its("response"); | ||
| }) | ||
| .then((response) => { | ||
| const { isExecutionSuccess } = response.body.data; | ||
|
|
||
| if (!isExecutionSuccess) { | ||
| cy.log(`Retrying... Attempts left: ${retries - 1}`); | ||
| assertHelper.Sleep(); | ||
| fireApi(retries - 1); | ||
| } else { | ||
| expect(isExecutionSuccess).to.eq(true); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| fireApi(5); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor retry mechanism to use Cypress built-in retries.
The custom retry mechanism can be replaced with Cypress's built-in retry-ability and assertions.
-const fireApi = (retries = 5, responseTimeout = 100000) => {
- if (retries === 0) {
- throw new Error("Max retries reached, API did not return success.");
- }
-
- dataSources.RunQuery({
- toValidateResponse: false,
- });
- cy.wait(assertHelper.GetAliasName("@postExecute"))
- .then((interceptions) => {
- return cy.get(assertHelper.GetAliasName("@postExecute"))
- .its("response");
- })
- .then((response) => {
- const { isExecutionSuccess } = response.body.data;
-
- if (!isExecutionSuccess) {
- cy.log(`Retrying... Attempts left: ${retries - 1}`);
- assertHelper.Sleep();
- fireApi(retries - 1);
- } else {
- expect(isExecutionSuccess).to.eq(true);
- }
- });
-};
-
-fireApi(5);
+dataSources.RunQuery({
+ toValidateResponse: false,
+});
+
+cy.intercept('POST', '**/postExecute').as('postExecute');
+cy.get('@postExecute').should('exist').then((interception) => {
+ expect(interception.response.body.data.isExecutionSuccess).to.eq(true);
+});Committable suggestion skipped: line range outside the PR's diff.
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13497602031. |
|
/ci-test-limit-count run_count=10 update_snapshot=false runId=13497602031 specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13498198564. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13498198564. |
|
/ci-test-limit-count run_count=40 update_snapshot=false runId=13497602031 specs_to_run=cypress/e2e/Regression/ServerSide/Datasources/Hubspot_spec.ts |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13499747208. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13499747208. |
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
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests
Chores