chore: Abstraction of Bottom View in Cypress#37410
Conversation
hetunandu
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestCase
participant BottomPane
participant Response
User->>TestCase: Trigger test case
TestCase->>BottomPane: switchResponseType("JSON")
BottomPane->>Response: switchToResponseTab()
Response->>Response: Click response tab
BottomPane->>Response: getResponseTypeSelector("JSON")
Response-->>TestCase: Return response type selector
TestCase->>TestCase: Assert response visibility
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
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: 6
🧹 Outside diff range and nitpick comments (13)
app/client/cypress/support/Pages/IDE/BottomPane/index.ts (1)
3-9: Add JSDoc documentation for the BottomPane classConsider adding documentation to describe the class's purpose and usage in Cypress tests.
+/** + * BottomPane represents the bottom panel in the IDE + * Used for managing response-related interactions in Cypress tests + */ class BottomPane { public readonly response: Response; constructor() { this.response = new Response(); } }app/client/cypress/support/Pages/IDE/BottomPane/Response.ts (1)
17-19: Implement record count validation methodThe TODO comment indicates pending implementation. This could block test coverage for response validation.
Would you like me to help create a GitHub issue to track this implementation task or provide a suggested implementation?
app/client/cypress/e2e/Regression/ClientSide/Binding/JSObjectToListWidget_Spec.ts (2)
Line range hint
45-47: Critical: Update test patterns to follow Cypress guidelinesSeveral patterns in the test need to be updated:
- Replace
cy.wait("@postExecute")with proper route aliases and intercept patterns- Use data-* attributes instead of direct selectors
- Group related assertions using
expect().to.satisfy()for better error reportingExample refactor for the assertions:
- cy.wait("@postExecute").then((interception: any) => { - valueToTest = JSON.stringify(interception.response.body.data.body[0].name).replace(/['"]+/g, ""); - cy.get(_.locators._textWidgetInDeployed) - .first() - .invoke("text") - .then((text) => { - expect(text).to.equal((valueToTest as string).trimEnd()); - }); - }); + cy.intercept('POST', '**/postExecute').as('apiResponse'); + cy.wait('@apiResponse').then((interception: any) => { + const expectedValue = JSON.stringify(interception.response.body.data.body[0].name).replace(/['"]+/g, ""); + cy.get('[data-cy=deployed-text-widget]') + .first() + .should('satisfy', ($el) => { + const text = $el.text().trim(); + expect(text).to.equal(expectedValue); + return true; + }); + });Also applies to: 84-86
Line range hint
71-97: Improve test maintainability and readabilityThe test contains repetitive navigation checks and magic numbers. Consider:
- Extracting page navigation assertions into a helper function
- Defining constants for expected widget counts
- Breaking down the long test case into smaller, focused ones
Example refactor:
// Define constants const ITEMS_PER_PAGE = 6; const TOTAL_ITEMS = 20; // Extract helper function const assertPageNavigation = (pageNumber: number, expectedItems: number) => { _.table.AssertPageNumber_List(pageNumber); _.agHelper.AssertElementLength(_.locators._textWidgetInDeployed, expectedItems); }; // Use in test it("3. Validate pagination with updated spacing", () => { // Setup EditorNavigation.SelectEntityByName("List1", EntityType.Widget); _.propPane.MoveToTab("Style"); _.propPane.UpdatePropertyFieldValue("Item Spacing (px)", "50"); // Deploy and verify initial state _.deployMode.DeployApp(_.locators._textWidgetInDeployed); assertPageNavigation(1, ITEMS_PER_PAGE); // Navigate forward _.table.NavigateToNextPage_List(); assertPageNavigation(2, ITEMS_PER_PAGE); // ... continue with other navigation checks });app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/OnLoadActions_Spec.ts (3)
Line range hint
113-113: Remove explicit sleep and use proper wait conditionsReplace
agHelper.Sleep(5000)with proper wait conditions. Explicit sleeps can make tests flaky and are against our coding guidelines.Consider using:
-agHelper.Sleep(5000); //for all api's to ccomplete call! +cy.wait("@getConsolidatedData").should("have.property", "response");
Line range hint
62-64: Consider extracting API URLs to configurationThe hardcoded URLs with port 5001 should be moved to a configuration file for better maintainability.
Consider creating a config file:
// config/apiEndpoints.ts export const API_ENDPOINTS = { QUOTES: "http://host.docker.internal:5001/v1/favqs/qotd", ACTIVITY: "http://host.docker.internal:5001/v1/boredapi/activity", GENDERIZE: "http://host.docker.internal:5001/v1/genderize/sampledata" };Also applies to: 71-74, 80-83
Line range hint
121-157: Extract response validation logicThe duplicate response validation logic should be extracted into a helper function to improve maintainability and reduce code duplication.
Consider creating a helper:
const validateLayoutActions = (layoutActions: any[]) => { const [randomFlora, randomUser, inspiringQuotes, genderize, suggestions] = layoutActions.map(action => action[0]); expect(randomFlora.name).to.eq("RandomFlora"); expect(randomUser.name).to.eq("RandomUser"); expect(inspiringQuotes.name).to.eq("InspiringQuotes"); expect(genderize.name).to.eq("Genderize"); expect(suggestions.name).to.eq("Suggestions"); };Also applies to: 175-204
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js (4)
Line range hint
33-38: Remove commented afterEach blocksThese commented-out afterEach blocks should be removed as they contain cy.pause() which is against our coding guidelines.
- // afterEach(function() { - // if (this.currentTest.state === "failed") { - // Cypress.runner.stop(); - // } - // }); - - // afterEach(() => { - // if (queryName) - // cy.actionContextMenuByEntityName(queryName); - // });
Line range hint
264-275: Remove commented selector verificationThe commented-out selector verification code should be removed.
- // cy.get(generatePage.updateBtn) - // .closest("button") - // .then((selector) => { - // cy.get(selector) - // .invoke("attr", "class") - // .then((classes) => { - // cy.log("classes are:" + classes); - // expect(classes).not.contain("bp3-disabled"); - // }); - // });
Line range hint
492-509: Remove commented update document test casesLarge blocks of commented-out test cases for update document operations should be removed.
- // //Update document - All matching documents - // cy.validateNSelectDropdown("Command", "Find document(s)", "Update document(s)"); - // cy.typeValueNValidate("{_id: {$gte:2}}", "Query"); - // cy.typeValueNValidate("{$set:{ 'Frõ': 'InActive'}}", "Update"); - // cy.validateNSelectDropdown("Limit", "Single document", "All matching documents"); - // cy.runQuery() - // cy.wait("@postExecute").then(({ response }) => { - // expect(response.body.data.body.nModified).to.eq(2); - // }); - - // //Verify Updation Successful: - // cy.validateNSelectDropdown("Command", "Update document(s)", "Find document(s)"); - // cy.runQuery() - // cy.wait("@postExecute").then(({ response }) => { - // expect(response.body.data.body[0].Frõ).to.eq('InActive'); - // });
Line range hint
551-554: Remove commented query validation codeRemove the commented-out code for query validation.
- // cy.typeValueNValidate( - // "{_id : {$lte: 3 }}", - // formControls.mongoDeleteDocumentsQuery, - // );app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (1)
Replace Sleep() calls with proper Cypress wait mechanisms
The test has unnecessary Sleep() calls that should be replaced with Cypress's built-in wait mechanisms. Specifically:
- Replace
agHelper.Sleep(2000)beforeRunQuery()with proper form value stabilization check- Replace
agHelper.Sleep(4000)afterRunQuery()withcy.wait('@postExecute')since the test already uses@postExecutealiasExample locations:
- Line ~430:
agHelper.Sleep(2000); //for documents value to settle!- Line ~431:
agHelper.Sleep(4000); //for capturing right response!🔗 Analysis chain
Line range hint
1-1000: Verify sleep duration necessityThe test contains multiple instances of
agHelper.Sleep()with hardcoded durations. While sometimes necessary, sleep commands can make tests flaky and slow.Let's analyze the sleep usage:
Consider replacing
Sleep()calls with Cypress's built-in retry-ability and waiting mechanisms:
- Use
cy.wait('@aliasName')for network requests- Use
cy.should('condition')for UI elements- Use
cy.waitUntil()for complex conditions
Let me analyze the sleep usage further to provide a more complete verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all Sleep() calls and their durations echo "Analyzing Sleep() calls in the test file:" rg -A 1 "Sleep\(" app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.tsLength of output: 593
Script:
#!/bin/bash # Check for existing Cypress wait/intercept patterns rg -B 2 -A 2 "@postExecute|@trigger|@getDatasourceStructure|@replaceLayoutWithCRUDPage" app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.tsLength of output: 3497
app/client/cypress/support/Pages/DataSources.ts (1)
1141-1152: LGTM: Good refactoring of response validationThe changes improve code organization by:
- Using the new BottomPane.response methods for response type validation
- Centralizing record count validation logic
Consider extracting the response type constants ("TABLE", "JSON", "RAW") to an enum or constants object to avoid magic strings:
enum ResponseType { TABLE = "TABLE", JSON = "JSON", RAW = "RAW" }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
app/client/cypress/e2e/Regression/ClientSide/Binding/JSObjectToListWidget_Spec.ts(2 hunks)app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts(2 hunks)app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/OnLoadActions_Spec.ts(6 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts(10 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js(6 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js(2 hunks)app/client/cypress/support/Pages/DataSources.ts(2 hunks)app/client/cypress/support/Pages/IDE/BottomPane/Response.ts(1 hunks)app/client/cypress/support/Pages/IDE/BottomPane/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
app/client/cypress/e2e/Regression/ClientSide/Binding/JSObjectToListWidget_Spec.ts (1)
Pattern 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/ClientSide/BugTests/Binding_Bug28287_Spec.ts (1)
Pattern 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/OnLoadTests/OnLoadActions_Spec.ts (1)
Pattern 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/QueryPane/Mongo1_spec.ts (1)
Pattern 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/QueryPane/Querypane_Mongo_Spec.js (1)
Pattern 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/QueryPane/S3_1_spec.js (1)
Pattern 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/Pages/DataSources.ts (1)
Pattern 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/Pages/IDE/BottomPane/Response.ts (1)
Pattern 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/Pages/IDE/BottomPane/index.ts (1)
Pattern 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.
🔇 Additional comments (14)
app/client/cypress/support/Pages/IDE/BottomPane/index.ts (1)
11-11: Verify singleton usage across test files
The singleton pattern implementation looks correct. Let's verify its usage to ensure there are no duplicate instantiations.
✅ Verification successful
Singleton pattern implementation verified - no duplicate instantiations found
The verification confirms proper singleton usage:
- Single instantiation at
app/client/cypress/support/Pages/IDE/BottomPane/index.ts - All other files correctly import the singleton instance via default import
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any direct instantiations of BottomPane class
# Expect: Only the default export should create an instance
# Search for new BottomPane() instances
rg "new BottomPane\(\)" --type ts
# Search for class imports to ensure they're using the default export
rg "import.*BottomPane.*from.*BottomPane" --type ts
Length of output: 848
app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts (1)
47-51: LGTM! Good use of the BottomPane component
The refactoring to use BottomPane improves code maintainability and follows best practices by:
- Using proper component abstraction
- Following data-* selector patterns
- Making assertions more readable
app/client/cypress/e2e/Regression/ClientSide/Binding/JSObjectToListWidget_Spec.ts (1)
5-6: LGTM: BottomPane abstraction improves maintainability
The introduction of BottomPane and its usage for response type switching aligns well with the PR's objective of abstracting the Bottom View.
Also applies to: 22-22
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/OnLoadActions_Spec.ts (2)
16-16: LGTM: Clean import addition
The BottomPane import is properly placed with other IDE-related imports.
59-59: LGTM: Consistent usage of BottomPane abstraction
The replacement of direct response type switching with the BottomPane abstraction is consistent across all instances, improving maintainability.
Also applies to: 68-68, 77-77, 86-86, 166-166
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js (4)
20-20: LGTM: Clean import of BottomPane component
The import statement follows the project's import organization pattern.
74-74: LGTM: Consistent use of BottomPane for record count validation
The changes standardize the way record counts are validated across different test cases using the new BottomPane component.
Also applies to: 96-96, 104-104, 112-112, 134-134, 148-148
436-436: LGTM: Record count validation in non-ASCII test case
The validation is correctly placed after the find operation in the non-ASCII test case.
71-73: 🛠️ Refactor suggestion
Remove commented code blocks
Several instances of commented-out code should be removed to improve maintainability.
- // cy.get(".CodeMirror textarea")
- // .first()
- // .focus()
- // .type(`{"find": "listingAndReviews","limit": 10}`, {
- // parseSpecialCharSequences: false,
- // });
- // cy.EvaluateCurrentValue(`{"find": "listingAndReviews","limit": 10}`);Likely invalid or redundant comment.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js (3)
3-3: LGTM!
The import statement is correctly placed and follows the project's import conventions.
194-195: LGTM!
The response type switching using the new BottomPane component is implemented correctly.
Line range hint 1-650: Remove explicit wait times and use Cypress best practices
The test file contains multiple instances of explicit waits (cy.wait) with hardcoded timeouts. This is against Cypress best practices and can make tests flaky.
Consider these improvements:
- Replace
cy.wait(3000)with built-in Cypress assertions - Instead of
cy.wait('@postExecute'), usecy.intercept()with proper routing and assertions - Remove
cy.setQueryTimeout(30000)and use Cypress's retry-ability
Example refactor:
- cy.wait(3000);
- cy.wait('@postExecute')
+ cy.intercept('POST', '/api/v1/actions/execute').as('postExecute');
+ cy.get('@postExecute').should('have.nested.property', 'response.body.data.isExecutionSuccess', true);Let's scan for all wait instances in the codebase:
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (1)
19-19: LGTM: Import statement is correctly placed
The import of BottomPane from the IDE directory is properly structured.
app/client/cypress/support/Pages/DataSources.ts (1)
14-14: LGTM: Clean import of BottomPane component
The addition of the BottomPane import aligns with the abstraction of bottom view functionality.
| class Response { | ||
| private ResponseTab = "//button[@data-testid='t--tab-RESPONSE_TAB']"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace XPath selector with data- attribute selector*
The XPath selector usage violates our coding guidelines. Use data-* attributes for more maintainable selectors.
class Response {
- private ResponseTab = "//button[@data-testid='t--tab-RESPONSE_TAB']";
+ private ResponseTab = "[data-testid='t--tab-RESPONSE_TAB']";📝 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.
| class Response { | |
| private ResponseTab = "//button[@data-testid='t--tab-RESPONSE_TAB']"; | |
| class Response { | |
| private ResponseTab = "[data-testid='t--tab-RESPONSE_TAB']"; |
| public switchToResponseTab(): void { | ||
| cy.xpath(this.ResponseTab).click({ force: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid force clicks and XPath in Cypress commands
Force clicks can make tests brittle and mask underlying issues. Consider adding proper wait conditions or element visibility checks.
public switchToResponseTab(): void {
- cy.xpath(this.ResponseTab).click({ force: true });
+ cy.get(this.ResponseTab)
+ .should('be.visible')
+ .click();
}Committable suggestion skipped: line range outside the PR's diff.
| public switchResponseType(type: string): void { | ||
| this.switchToResponseTab(); | ||
| cy.xpath(this.getResponseTypeSelector(type)).click({ force: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve response type switching reliability
Similar to the tab switching, avoid force clicks and use proper wait conditions.
public switchResponseType(type: string): void {
this.switchToResponseTab();
- cy.xpath(this.getResponseTypeSelector(type)).click({ force: true });
+ cy.get(this.getResponseTypeSelector(type))
+ .should('be.visible')
+ .click();
}Committable suggestion skipped: line range outside the PR's diff.
| public getResponseTypeSelector(type: string): string { | ||
| return `//div[@data-testid='t--response-tab-segmented-control']//span[text()='${type}']`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use data attributes for response type selection
String interpolation in selectors is fragile. Consider using data attributes with specific values for each type.
public getResponseTypeSelector(type: string): string {
- return `//div[@data-testid='t--response-tab-segmented-control']//span[text()='${type}']`;
+ return `[data-testid='t--response-type-${type.toLowerCase()}']`;
}Committable suggestion skipped: line range outside the PR's diff.
| @@ -43,7 +44,11 @@ describe( | |||
|
|
|||
| EditorNavigation.SelectEntityByName(queryName, EntityType.Query); | |||
|
|
|||
There was a problem hiding this comment.
Remove agHelper.Sleep(1000)
Replace the sleep with proper Cypress wait commands or assertions to ensure the page has loaded.
Apply this change:
- agHelper.Sleep(1000);
+ cy.waitUntil(() => EditorNavigation.isEntityPresent(queryName, EntityType.Query));Committable suggestion skipped: line range outside the PR's diff.
| agHelper.AssertElementVisibility( | ||
| BottomPane.response.getResponseTypeSelector("JSON"), | ||
| ); | ||
| agHelper.AssertElementVisibility( | ||
| BottomPane.response.getResponseTypeSelector("RAW"), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting repeated assertions into a helper function
The same response type selector assertions are repeated across multiple test cases. This violates the DRY principle and makes maintenance harder.
Consider creating a helper function in the test utilities:
// In support/Objects/ObjectsCore.ts
export const assertResponseTypeSelectors = () => {
agHelper.AssertElementVisibility(
BottomPane.response.getResponseTypeSelector("JSON")
);
agHelper.AssertElementVisibility(
BottomPane.response.getResponseTypeSelector("RAW")
);
};
// In the test file
assertResponseTypeSelectors();Also applies to: 470-475, 517-522, 559-564, 590-595, 623-628, 660-665, 685-690, 722-727
2 participants
Description
Abstract out the Bottom View in cypress tests so that it can be updated in the redesign tasks
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="@tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11855858863
Commit: 0cf1b0b
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Fri, 15 Nov 2024 12:48:55 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
BottomPanecomponent for improved response type management in tests.Bug Fixes
Documentation
Refactor
BottomPanecomponent.