feat: Enable new toolbar for cypress #37148
Conversation
albinAppsmith
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe pull request introduces a new feature flag, Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
app/client/cypress/support/Objects/FeatureFlags.ts (4)
8-8: LGTM! Consider adding a comment describing the flag's purpose.The new feature flag follows the established pattern.
+ // Controls the visibility of the redesigned actions panel in the editor release_actions_redesign_enabled: true,
Line range hint
11-28: Consider adding response type definition.The function correctly intercepts the feature flags API, but could benefit from explicit typing of the response object.
interface FeatureFlagResponse { responseMeta: { status: number; success: boolean; }; data: Record<string, boolean>; errorDisplay: string; }
Line range hint
30-52: Enhance error handling and simplify the response transformation.The nested callback structure could be simplified, and error cases should be handled explicitly.
export const getConsolidatedDataApi = ( flags: Record<string, boolean> = {}, reload = true, ) => { cy.intercept("GET", "/api/v1/consolidated-api/*?*", (req) => { - req.reply((res: any) => { + req.reply((res) => { + if (!res) return req.continue(); if ( - res.statusCode === 200 || - res.statusCode === 401 || - res.statusCode === 500 + ![200, 401, 500].includes(res.statusCode) ) { + return req.continue(); + } const originalResponse = res?.body; const updatedResponse = produce(originalResponse, (draft: any) => { draft.data.featureFlags.data = { ...flags, }; }); return res.send(updatedResponse); - } }); }).as("getConsolidatedData");
Consider extracting common license flag filtering logic to reduce duplication
The ripgrep results confirm that the license flag filtering logic is duplicated only within this file and follows the same pattern in both intercepts. The suggested refactoring would improve maintainability.
- Extract common filtering logic into a reusable function in
app/client/cypress/support/Objects/FeatureFlags.ts- Apply the extracted function to both intercept handlers
const filterLicenseFlags = (flags: Record<string, boolean>) => { return Object.entries(flags).reduce((acc, [flag, value]) => { if (LICENSE_FEATURE_FLAGS.includes(flag)) { acc[flag] = value; } return acc; }, {} as Record<string, boolean>); };🔗 Analysis chain
Line range hint
54-119: Refactor to reduce code duplication and improve maintainability.The function contains repeated logic for filtering license flags. Consider extracting common functionality.
const filterLicenseFlags = (flags: Record<string, boolean>) => { return Object.entries(flags).reduce((acc, [flag, value]) => { if (LICENSE_FEATURE_FLAGS.includes(flag)) { acc[flag] = value; } return acc; }, {} as Record<string, boolean>); };Then use it in both intercepts:
export const featureFlagInterceptForLicenseFlags = () => { cy.intercept( { method: "GET", url: "/api/v1/users/features", }, (req) => { req.reply((res) => { if (res) { const originalResponse = res.body; - let modifiedResponse: any = {}; - Object.keys(originalResponse.data).forEach((flag) => { - if (LICENSE_FEATURE_FLAGS.includes(flag)) { - modifiedResponse[flag] = originalResponse.data[flag]; - } - }); + const modifiedResponse = filterLicenseFlags(originalResponse.data);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other potential duplicate flag filtering logic rg -A 5 "LICENSE_FEATURE_FLAGS.includes"Length of output: 1091
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/cypress/support/Objects/FeatureFlags.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/support/Objects/FeatureFlags.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.
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (103)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/constants.ts (1)
8-10: Consider standardizing the test locator pattern.The existing
runJSActiondoesn't follow the "t--" prefix pattern used byrunJSActionTestID. Consider updating it for consistency.export const testLocators = { - runJSAction: "run-js-action", + runJSAction: "t--run-js-action", runJSActionTestID: "t--run-js-action", };app/client/cypress/locators/JSEditor.json (1)
Line range hint
3-11: Consider standardizing remaining selectors.Several selectors in this file don't follow our Cypress guidelines:
- Using class selectors (
.bp3-*,.t--*)- Using XPath selector for popover
- Using plain text selectors with
containsConsider updating these to use data-testid attributes. Example refactor:
{ "runButton": "[data-testid='t--run-js-action']", - "editNameField": ".bp3-editable-text-input", + "editNameField": "[data-testid='t--js-editor-name-input']", - "outputConsole": ".CodeEditorTarget", + "outputConsole": "[data-testid='t--js-editor-console']", - "jsObjectName": ".t--action-name-edit-field", + "jsObjectName": "[data-testid='t--js-object-name']", - "jsPage": ".form-row-header", + "jsPage": "[data-testid='t--js-page-header']", - "jsPageProperty": ".entity-context-menu", + "jsPageProperty": "[data-testid='t--js-property-menu']", - "propertyList": ".t--entity-property", + "propertyList": "[data-testid='t--js-property-list']", - "delete": ".single-select >div:contains('Delete')", + "delete": "[data-testid='t--js-delete-btn']", - "deleteConfirm": ".single-select >div:contains('Are you sure?')", + "deleteConfirm": "[data-testid='t--js-delete-confirm']", - "popover": "//*[local-name()='g' and @id='Icon/Outline/more-vertical']" + "popover": "[data-testid='t--js-more-actions']" }app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug19893_spec.ts (1)
Line range hint
1-24: Consider moving selectors to ObjectsCore.While the test follows most Cypress best practices, consider:
- Moving the
cy.get("@guid")selector to ObjectsCore for better maintainability- Creating a constant for the hardcoded "localhost" and 9 values in AssertCursorPositionForTextInput
Example refactor:
// In ObjectsCore.ts export const guidSelector = "@guid"; export const defaultHost = "localhost"; export const defaultCursorPosition = 9; // In test file _.agHelper.GetElement(_.locators.guidSelector).then((uid) => { // ... rest of the code _.dataSources.AssertCursorPositionForTextInput( _.dataSources._urlInputControl, "{moveToStart}", _.constants.defaultHost, _.constants.defaultCursorPosition, ); });app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug25148_Spec.ts (1)
Line range hint
1-27: Consider restructuring the test for better organization.While the test follows Cypress best practices, consider these improvements:
- Move UUID generation to a before block to keep the test focused on behavior
- Combine the two assertions on
apiPage._saveAsDSinto a single expect statementHere's how you could refactor:
describe( "Bug 25148 - Edit Datasource button was disabled on Authentication tab of Api action", { tags: ["@tag.Datasource", "@tag.Git", "@tag.AccessControl"] }, () => { let dsName: string; let uid: string; before(() => { agHelper.GenerateUUID(); cy.get("@guid").then((generatedUid) => { uid = generatedUid; dsName = "AuthAPI " + uid; }); }); it("should keep Edit datasource button disabled in Authentication tab", () => { dataSources.NavigateToDSCreateNew(); dataSources.CreatePlugIn("Authenticated API"); agHelper.RenameDatasource(dsName); dataSources.FillAuthAPIUrl(); dataSources.SaveDatasource(); apiPage.CreateApi("API" + uid, "GET", true); apiPage.SelectPaneTab("Authentication"); // Assert both save buttons are disabled cy.get(apiPage._saveAsDS).each(($el) => { cy.wrap($el).should('be.disabled'); }); }); }, );app/client/cypress/locators/QueryEditor.json (1)
Line range hint
1-26: Consider standardizing all selectors to use data-testid attributes.Many selectors in this file use XPath, text content, or class selectors which are fragile and not recommended for Cypress tests. Consider refactoring them to use data-testid attributes consistently.
Examples of selectors to update:
- "runQuery": "//span[text()='Run']/ancestor::button", + "runQuery": "[data-testid=t--run-query-button]", - "settings": "span:contains('Settings')", + "settings": "[data-testid=t--settings-button]", - "queryEditorIcon": ".t--nav-link-query-editor", + "queryEditorIcon": "[data-testid=t--query-editor-icon]"app/client/cypress/locators/ApiEditor.js (1)
Line range hint
1-38: Consider standardizing remaining selectors to use data-testid.Many selectors still use class-based or CSS path selectors. For better maintainability and alignment with testing best practices, consider updating them to use data-testid attributes.
Example refactoring for a few selectors:
export default { - curlImportBtn: ".t--importBtn", + curlImportBtn: "[data-testid=\"t--import-btn\"]", - createBlankApiCard: ".t--createBlankApiCard", + createBlankApiCard: "[data-testid=\"t--create-blank-api-card\"]", - nameOfApi: ".t--nameOfApi", + nameOfApi: "[data-testid=\"t--name-of-api\"]", // ... apply similar changes to other selectors };app/client/cypress/e2e/Sanity/Datasources/MongoDatasourceURI_spec.ts (3)
Line range hint
52-52: Avoid direct selector usage.Replace the direct selector access
dataSources._portwith a data-* attribute based selector encapsulated in a helper function.-agHelper.AssertAttribute(dataSources._port, "value", "27017"); +agHelper.AssertAttribute(dataSources.getPortInputSelector(), "value", "27017");
Line range hint
7-9: Add authentication setup using API calls.According to best practices, the test should use LoginFromAPI for authentication setup.
Add this before the first test:
beforeEach(() => { agHelper.LoginFromAPI(); });
Line range hint
6-7: Track the TODO with a GitHub issue.The TODO comment references an external issue. Consider creating an internal issue to track this enhancement.
Would you like me to help create a GitHub issue to track the pending test case for URI with username and password?
app/client/cypress/e2e/Sanity/Datasources/ArangoDataSourceStub_spec.js (3)
Line range hint
32-35: Replace cy.wait with better assertionsUsing
cy.waitwith@createNewApiis discouraged. Instead, use route aliases with proper assertions.- cy.wait("@createNewApi").should( - "have.nested.property", - "response.body.responseMeta.status", - 201, - ); + cy.get("@createNewApi").should((interception) => { + expect(interception.response.statusCode).to.equal(201); + expect(interception.response.body.responseMeta.status).to.equal(201); + });
Line range hint
1-63: Enhance test reliability with data- selectors*Several improvements needed to align with best practices:
- Use data-* attributes for selectors
- Consider using API calls for setup/teardown instead of UI interactions
- Add multiple assertions for better test reliability
Would you like me to provide specific examples for implementing these improvements?
Line range hint
13-17: Use API calls for datasource testingReplace UI-based datasource testing with API calls for better performance and reliability.
// Example API-based approach: cy.request({ method: 'POST', url: '/api/v1/datasources/test', body: { // datasource config } }).then((response) => { expect(response.status).to.equal(200); expect(response.body.data.success).to.be.true; });app/client/cypress/e2e/Sanity/Datasources/DSAutosaveImprovements_spec.ts (1)
Line range hint
1-89: Add authentication via API for better test reliability.The test suite should use
LoginFromAPIfor authentication instead of UI-based login to improve test reliability and performance.Add this at the beginning of the describe block:
describe( "Datasource Autosave Improvements Tests", { tags: ["@tag.Datasource", "@tag.Sanity", "@tag.Git", "@tag.AccessControl"], }, function () { + before(() => { + agHelper.LoginFromAPI(); + });app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/False_Spec.ts (4)
Line range hint
44-44: Remove agHelper.Sleep(2000)Using
Sleep()is against the coding guidelines. Replace with appropriate Cypress wait commands or assertions.- agHelper.Sleep(2000); + agHelper.WaitUntilToastDisappear(); // If waiting for toast // Or use appropriate assertion/wait based on your specific need
Line range hint
63-69: Remove commented code blockClean up the test file by removing the commented-out code block in the after hook.
- // ["falseCases", "createTable"].forEach((type) => { - // entityExplorer.ActionContextMenuByEntityName( - // type, - // "Delete", - // "Are you sure?", - // ); - // });
Line range hint
37-45: Enhance test assertionsThe test lacks multiple assertions for the error cases. Consider adding specific error message validations.
dataSources.RunQuery({ expectedStatus: false }); + cy.get('@postExecute').should('have.property', 'status', 400); + cy.get('@postExecute').its('response.body.responseMeta.error').should('exist');
Line range hint
41-42: Use data- attributes for dynamic queries*Replace string concatenation with data attributes for better maintainability.
- `INSERT INTO ${inputData.tableName} (${inputData.inputFieldName[i]}) VALUES ({{"${value}"}})` + `INSERT INTO [data-table-name=${inputData.tableName}] ([data-field-name=${inputData.inputFieldName[i]}]) VALUES ({{"${value}"}})`app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
Line range hint
89-94: Consider consolidating test selectors.The Button component has both a className and data-testid for testing purposes. Consider standardizing on data-testid for Cypress tests to avoid selector redundancy.
<Button - className={testLocators.runJSAction} data-testid={testLocators.runJSActionTestID} isDisabled={props.disabled} isLoading={props.isLoading} onClick={props.onButtonClick}app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/CopyQuery_RenameDatasource_spec.js (4)
Line range hint
19-23: Replace wait patterns with better alternativesThe test uses implicit waits through Cypress aliases (@saveDatasource, @getPluginForm). Consider using Cypress's built-in retry-ability with assertions instead.
Replace wait patterns with assertions:
cy.get('@saveDatasource').should('exist').then((httpResponse) => { expect(httpResponse.response.body.data).to.exist; datasourceName = httpResponse.response.body.data.name; // ... rest of the code });
Line range hint
52-61: Improve selector usage and combine assertionsThe test uses non-recommended selectors and separate expect statements.
- Replace apiwidget.propertyList with a data-* attribute:
cy.get('[data-cy=query-binding-list]').should('exist').and(($lis) => { expect($lis).to.satisfy((elements) => { return elements.length === 5 && elements[0].textContent.includes('{{Query1.isLoading}}') && elements[1].textContent.includes('{{Query1.data}}') && elements[2].textContent.includes('{{Query1.responseMeta}}') && elements[3].textContent.includes('{{Query1.run()}}') && elements[4].textContent.includes('{{Query1.clear()}}'); }); });
Line range hint
78-88: Refactor test case to follow best practicesThe test case has similar issues with selectors and assertions, plus potential wait issues with cy.runQuery().
Consider refactoring to:
- Replace cy.runQuery() with a more reliable approach
- Use data attributes for selectors
- Combine assertions:
cy.intercept('POST', '/api/v1/actions/execute').as('queryExecution'); // Trigger query execution cy.wait('@queryExecution').its('response.statusCode').should('eq', 200); cy.get('[data-cy=query-binding-list]').should('exist').and(($lis) => { expect($lis).to.satisfy((elements) => { return elements.length === 5 && elements.every((el, idx) => { const expectedBindings = ['isLoading', 'data', 'responseMeta', 'run()', 'clear()']; return el.textContent.includes(`{{Query1.${expectedBindings[idx]}}}`); }); }); });
Line range hint
1-5: Add missing authentication and rename fileThe test file needs structural improvements to align with best practices.
- Add API-based authentication:
before(() => { cy.LoginFromAPI(Cypress.env("USERNAME"), Cypress.env("PASSWORD")); }); after(() => { cy.LogOutviaAPI(); });
- Rename the file to remove "RenameDatasource" since no such test exists, or add the missing test case.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionRun.tsx (1)
106-106: LGTM! Good addition of the test identifier.The component maintains both className and data-testid selectors, providing flexible options for test targeting.
Consider standardizing on either className or data-testid for test selection to maintain consistency. If both are needed, documenting the use case for each would be helpful.
app/client/cypress/locators/apiWidgetslocator.json (1)
Line range hint
1-64: Consider updating other selectors to use data-testid attributes.Several locators in this file use XPath, class selectors, or text-based selectors which are fragile and harder to maintain. For example:
searchInputPlaceholder: uses XPathmoveTo: uses text selector with contains()resourceUrl: uses class selectorConsider gradually migrating these to data-testid attributes. Example pattern:
- "searchInputPlaceholder": "//div[contains(@class, 't--dataSourceField')]//div//input", + "searchInputPlaceholder": "[data-testid='t--datasource-search-input']", - "moveTo": ".single-select >div:contains('Move to')", + "moveTo": "[data-testid='t--move-to-action']", - "resourceUrl": ".t--dataSourceField", + "resourceUrl": "[data-testid='t--datasource-field']"app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Query_Datasource_spec.js (3)
Line range hint
73-83: Use data- attributes for selectors*Replace API widget selectors with data-* attributes for better maintainability.
Create constants for selectors:
// In locators file export const QUERY_EDITOR = { queryInput: '[data-cy="query-input"]', // ... other selectors };Then use these constants instead of direct selectors.
Line range hint
85-95: Improve assertion structure and avoid string literalsWhile multiple assertions are good, string literals in assertions should be constants.
Extract expected values to constants:
const QUERY_BINDINGS = { isLoading: "{{Query1.isLoading}}", data: "{{Query1.data}}", // ... other bindings }; // Then use in assertions expect($lis.eq(0)).to.contain(QUERY_BINDINGS.isLoading);
Replace hardcoded waits with route assertions and conditional waits
The codebase shows extensive use of hardcoded
cy.wait()calls which can make tests flaky and slow. For this specific test file, we should:
- Replace
cy.wait(2000)withcy.wait('@getPage')after setting up route interception- For UI interactions, use Cypress' built-in retry-ability with assertions like
cy.get().should('be.visible')instead of arbitrary waitsExample fix:
// Before cy.generateUUID().then((uid) => { datasourceName = uid; cy.wait(2000); }); // After cy.intercept('GET', '/api/v1/pages*').as('getPage'); cy.generateUUID().then((uid) => { datasourceName = uid; cy.wait('@getPage'); });🔗 Analysis chain
Line range hint
21-24: Replace hardcoded waits with route assertionsThe test contains multiple instances of
cy.wait(2000)which can make tests flaky. Instead, wait for specific routes or elements to be ready.Replace with route assertions:
- cy.wait(2000); + cy.wait("@getPage");🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other instances of hardcoded waits rg "cy\.wait\(\d+\)" --type jsLength of output: 98678
app/client/cypress/support/Pages/GSheetHelper.ts (1)
Line range hint
94-94: Remove agHelper.Sleep() callUsing Sleep() is against Cypress best practices as it introduces flaky tests. Instead, use Cypress's built-in waiting mechanisms.
Replace with appropriate wait:
- this.agHelper.Sleep(500); + this.agHelper.WaitUntilAllPopToastsDisappear();app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js (3)
Line range hint
23-24: Replace cy.wait with better alternatives.Using explicit
cy.waitcommands is discouraged as per the coding guidelines. Consider using Cypress's built-in retry-ability and assertions instead.Replace the wait commands with proper assertions:
- cy.wait(1000); - cy.wait("@createNewJSCollection"); - cy.wait(1000); + cy.wait("@createNewJSCollection").its('response.statusCode').should('eq', 201);- cy.wait(1000); + cy.get('p:contains("Import from CURL")').should('be.visible');- cy.wait(1000); - cy.wait(2000); + cy.get(omnibar.globalSearchInput).should('be.visible').and('have.value', 'vnjkv');Also applies to: 73-73, 89-89
Line range hint
44-44: Use data- attributes for selectors.*Replace class-based selectors with data-* attributes as per the coding guidelines.
Replace the following selectors:
- cy.get(".t--js-action-name-edit-field") + cy.get("[data-cy=js-action-name-field]") - cy.get(".no-data-title") + cy.get("[data-cy=no-data-title]")Also applies to: 91-91
Line range hint
108-127: Replace XPath selectors with data- attributes.*Using XPath selectors is discouraged as per the coding guidelines.
Replace the XPath selectors in the recently opened items verification:
- cy.xpath(omnibar.recentlyopenItem) + cy.get('[data-cy=recently-opened-item]')Consider updating the locator in the
Omnibar.jsonfile to use data-* attributes instead of XPath.app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (3)
Line range hint
43-43: Replace cy.wait with better alternativesMultiple instances of
cy.wait("@updateLayout")found. According to Cypress best practices and our coding guidelines, we should avoid using cy.wait.Instead of waiting for a specific time, intercept the request and wait for the response:
-cy.wait("@updateLayout"); +cy.intercept('POST', '/api/v1/layouts/update').as('updateLayout'); +cy.wait('@updateLayout').its('response.statusCode').should('eq', 200);Also applies to: 44-44, 45-45, 46-46, 47-47, 48-48
Line range hint
144-144: Replace hardcoded selectors with data- attributes*Using CSS selectors like
.t--list-widget-next-pageand.t--widget-containerwidgetviolates our selector guidelines.Replace with data-* attributes:
-cy.get(".t--list-widget-next-page").find("button") +cy.get("[data-testid=list-widget-next-page]").find("[data-testid=next-page-button]") -cy.get(".t--widget-containerwidget") +cy.get("[data-testid=container-widget]")Also applies to: 145-145, 146-146, 147-147, 148-148, 149-149, 150-150
Line range hint
39-39: Improve test structure and assertionsThe test structure could be improved in several ways:
- Missing API-based login/logout as per guidelines
- Using string literals in assertions
- Multiple assertions could be combined
Consider refactoring the test setup:
describe("List widget V2 page number and page size", () => { before(() => { _.agHelper.AddDsl("listv2PaginationDsl"); // Add LoginFromAPI here }); after(() => { // Add LogOutviaAPI here }); it("1. List widget V2 with client side pagination", () => { // Store expected values in constants const expectedPageSize = 4; const expectedPageNumber = 1; cy.openPropertyPane("listwidgetv2"); cy.testJsontext("items", JSON.stringify(listData)); // Combine assertions cy.get(commonlocators.bodyTextStyle).first() .should("have.text", `PageSize ${expectedPageSize}`) .and("be.visible"); }); });Also applies to: 40-40, 41-41, 42-42, 43-43, 44-44, 45-45, 46-46, 47-47, 48-48, 49-49, 50-50, 51-51, 52-52
app/client/cypress/e2e/Regression/ClientSide/BugTests/DatasourceSchema_spec.ts (2)
Line range hint
42-42: Remove agHelper.Sleep() call.Using
Sleep()is discouraged as per the coding guidelines. Consider using Cypress's built-in waiting mechanisms or assertions instead.- agHelper.Sleep(1500); + agHelper.WaitUntilEleAppear(dataSources._datasourceStructure);
Line range hint
89-89: Remove agHelper.Sleep() call.Another instance of
Sleep()that should be replaced with proper Cypress waiting mechanisms.- agHelper.Sleep(1500); + agHelper.WaitUntilAllToastsDisappear();app/client/cypress/e2e/Regression/ServerSide/QueryPane/DSDocs_Spec.ts (2)
Line range hint
141-156: Avoid using afterEach in test cases.Per our Cypress guidelines, we should avoid using
afterEach. Consider moving the cleanup logic into each test case or use API calls for cleanup.- afterEach(() => { - agHelper.PressEscape(); - agHelper.ActionContextMenuWithInPane({ - action: "Delete", - entityType: entityItems.Query, - }); - dataSources.DeleteDatasourceFromWithinDS(dsName); - }); + // In each test case: + it("test case", () => { + // ... test logic ... + // Cleanup using API calls + cy.request('DELETE', '/api/v1/datasources/${dsName}'); + });
Line range hint
1-174: Refactor suggestion: Parameterize similar test cases.The test file contains multiple similar test cases that differ mainly in the datasource type and documentation URL. Consider parameterizing these tests for better maintainability.
const testCases = [ { name: "Postgres", plugin: DataSourceKVP["Postgres"], docPath: "querying-postgres#create-crud-queries" }, // ... other test cases ]; testCases.forEach(({ name, plugin, docPath }) => { it(`Verify ${name} documentation opens`, function() { CreateDummyDSNSave(plugin); cy.get("@dsName").then(($dsName) => { dsName = $dsName; dataSources.CreateQueryAfterDSSaved(); deployMode.StubWindowNAssert( dataSources._queryDoc, docPath, "getPluginForm" ); }); }); });app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/Basic_Spec.ts (3)
Line range hint
89-89: Remove explicit sleep statements.According to the coding guidelines, we should avoid using
agHelper.Sleep(). Instead, use Cypress's built-in waiting mechanisms or wait for specific elements/states.Replace the sleep statements with appropriate wait conditions:
- agHelper.Sleep(2000); + agHelper.WaitUntilElemDisappear(locators._modal); - agHelper.Sleep(3000); + agHelper.WaitUntilAllToastsDisappear();Also applies to: 102-102
Line range hint
73-77: Replace cy.wait with better assertions.According to the coding guidelines, we should avoid using
cy.wait. Instead, wait for specific conditions or states.Replace the wait with a more specific assertion:
- cy.wait(2000); + agHelper.WaitUntilTableLoad();
Line range hint
134-143: Use API-based logout in the cleanup.According to the coding guidelines, we should use
LogOutviaAPIfor logging out.Add the API-based logout before cleaning up:
after( "Verify Drop of tables & Deletion of the datasource after all created queries are deleted", () => { + agHelper.LogOutviaAPI(); EditorNavigation.SelectEntityByName("dropTable", EntityType.Query); dataSources.RunQuery();app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/PostgresConnections_spec.ts (2)
Line range hint
1-200: Multiple test best practices violations found.The test file has several violations of the coding guidelines:
- Uses
afterhook which should be avoided- Contains
agHelper.Sleep(10000)which should be replaced with proper wait conditions- Uses string-based assertions in multiple places
Consider these changes:
- Replace
afterhook withbeforeEach/afterEachfor cleanup- Replace
agHelper.Sleep(10000)with appropriate wait conditions- Use constants for assertion values instead of strings
Line range hint
115-120: Improve selector and assertion patterns.
- Query assertions directly access table cells. Consider using Cypress best practices:
// Instead of dataSources.ReadQueryTableResponse(0).then(($cellData) => { expect(Number($cellData)).to.eq(0); }); // Consider cy.get('[data-cy="query-table-response"]') .first() .should('have.text', '0');
- Use data-* attributes for more reliable selectors.
Also applies to: 150-155
app/client/cypress/support/ApiCommands.js (1)
Line range hint
1-231: Multiple guideline violations need attentionThe file contains several patterns that should be updated according to the coding guidelines:
Replace cy.wait calls with proper intercepts:
- Line 31: cy.wait(2000)
- Line 89: cy.wait("@postExecute")
- Multiple other instances
Replace XPath selectors with data-testid:
- Line 34: apiwidget.responseStatus
- Line 142: apiwidget.popover
Remove explicit waits:
- Line 31: cy.wait(2000)
- Line 89: cy.wait(3000)
Would you like me to provide specific refactoring suggestions for each of these patterns?
app/client/cypress/e2e/Regression/ClientSide/BugTests/DSDiscardBugs_spec.ts (2)
Line range hint
1-182: Improve test structure and assertions.Several improvements can be made to align with best practices:
- Consider using
beforeEachto set up common test data- Use data-* attributes for selectors instead of class-based selectors
- Add multiple assertions to validate the state after each action
Example refactor for a test case:
beforeEach(() => { _.agHelper.GenerateUUID(); cy.get("@guid").then((uid) => { dsName = `Mongo${uid}`; }); }); it("should show discard popup when changes are made", () => { // Setup _.dataSources.NavigateToDSCreateNew(); _.dataSources.CreatePlugIn("MongoDB"); _.agHelper.RenameDatasource(dsName); // Action _.dataSources.FillMongoDSForm(); _.dataSources.SaveDatasource(); _.agHelper.WaitUntilToastDisappear(); // Multiple assertions cy.get('[data-testid="t--datasource-name"]').should('have.value', dsName); cy.get('[data-testid="t--datasource-saved"]').should('be.visible'); // Cleanup _.dataSources.DeleteDatasourceFromWithinDS(dsName); });
Line range hint
147-152: Use data-testid for element selection.Replace class-based selector with data-testid attribute for better maintainability.
-_.agHelper.GetNClick( - _.dataSources._cancelEditDatasourceButton, - 0, - true, - 200, -); +cy.get('[data-testid="t--datasource-cancel-edit"]').click();app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js (2)
Line range hint
38-41: Replace cy.wait with better alternativesInstead of using
cy.wait(2000), use Cypress's built-in retry-ability withcy.get()and assertions.-cy.wait(2000); +cy.get(publishLocators.containerWidget).should('be.visible');
Line range hint
71-77: Remove explicit wait and use proper assertionsThe comment about isLoading suggests a race condition. Instead of using wait, properly handle the loading state.
-cy.wait(3000); +cy.get('[data-testid="list-loading-state"]').should('not.exist'); +cy.get(publishLocators.containerWidget).should('have.length', 2).and('be.visible');app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts (1)
Line range hint
23-24: Remove cy.wait and use better waiting strategies.Using arbitrary wait times with
cy.wait(500)is discouraged. Instead, use Cypress's built-in retry-ability and wait for specific conditions.Replace with a more reliable waiting strategy:
- cy.wait(500); + cy.waitUntil(() => Cypress.$(oneClickBindingLocator.datasourceDropdownSelector).length > 0);Or better yet, wait for the specific condition you're expecting:
- cy.wait(500); + agHelper.AssertElementExist(oneClickBindingLocator.datasourceDropdownSelector);app/client/cypress/support/Pages/JSEditor.ts (2)
Line range hint
210-214: Remove Sleep() call and add better assertions.The
RenameJSObjFromPanemethod could be improved by:
- Using better assertions instead of relying on blur()
- Adding error handling for the rename operation
Apply this diff to improve the implementation:
public RenameJSObjFromPane(renameVal: string) { cy.get(this._jsObjName).dblclick({ force: true }); cy.get(this._jsObjTxt) .clear() .type(renameVal, { force: true }) - .should("have.value", renameVal) - .blur(); + .should("have.value", renameVal); + cy.focused().blur(); + // Verify the rename was successful + cy.get(this._jsObjName).should("contain.text", renameVal); PageLeftPane.assertPresence(renameVal); }
Line range hint
1-324: Multiple improvements needed across the file.
- Several XPath selectors are used throughout the file (e.g.,
_settingsTab,_codeTab, etc.) which should be replaced with data-testid attributes.- Multiple instances of
agHelper.Sleep()are used which should be replaced with proper Cypress wait commands.- Consider extracting common locators to a shared constants file.
Would you like me to provide specific refactoring suggestions for these improvements?
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts (3)
Line range hint
147-147: Remove usage of agHelper.Sleep()The test contains multiple instances of
agHelper.Sleep(). This violates the coding guidelines and can make tests flaky. Instead, use Cypress's built-in waiting mechanisms that wait for elements or network requests to complete.Replace the sleep calls with appropriate wait conditions:
- agHelper.Sleep(); + agHelper.WaitUntilEleAppear(locators._jsonFormWidget); - agHelper.Sleep(2500); //for search to load + agHelper.WaitUntilTableLoad();Also applies to: 196-196, 271-271
Line range hint
367-374: Replace cy.xpath with data- attributes*Using XPath selectors is discouraged in Cypress tests. The button validation should use data-* attributes instead.
Replace the XPath selector:
- cy.xpath(locators._buttonByText("Update")).then((selector) => { - cy.wrap(selector) - .invoke("attr", "class") - .then((classes) => { - expect(classes).not.contain("bp3-disabled"); - }); - }); + cy.get('[data-cy="update-button"]') + .invoke("attr", "class") + .should("not.contain", "bp3-disabled");Please add the corresponding data-* attribute to the button in the component code.
Line range hint
1-400: Use data- attributes for all selectors*The test uses various string-based selectors throughout the code. This makes the tests brittle and harder to maintain.
Examples of selectors that need to be updated:
locators._buttonByText("Submit")locators._buttonByText("Delete")locators._buttonByText("Confirm")locators._buttonByText("Got it")Please update these to use data-* attributes in both the test and component code:
// Example format cy.get('[data-cy="submit-button"]') cy.get('[data-cy="delete-button"]') cy.get('[data-cy="confirm-button"]') cy.get('[data-cy="got-it-button"]')app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/BooleanEnum_Spec.ts (2)
Line range hint
203-203: Remove agHelper.Sleep() calls.Using
agHelper.Sleep()is explicitly forbidden in the coding guidelines. Replace with appropriate wait conditions.- agHelper.Sleep(2500); //Allwowing time for delete to be success + agHelper.WaitUntilToastDisappear(); // Wait for delete success toast - agHelper.Sleep(2000); + agHelper.WaitUntilAllToastsDisappear();Also applies to: 213-213
Line range hint
251-282: Avoid using after blocks in test cases.The coding guidelines explicitly discourage the use of after blocks. Consider:
- Moving cleanup logic to a separate test case
- Using beforeEach/afterEach at the describe level if absolutely necessary
- after( - "Verify Deletion of the datasource after all created queries are deleted", - () => { - //Drop table logic... - } - ); + it("9. Cleanup - Delete datasource and queries", () => { + //Drop table logic... + });app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts (2)
Line range hint
77-86: Consolidate assertions for better test efficiency.Multiple individual assertions on table data can be consolidated using a single assertion with an expected object:
-table.ReadTableRowColumnData(0, 0, "v2").then((cellData) => { - expect(cellData).to.eq(data.uniq_id); -}); -table.ReadTableRowColumnData(0, 1, "v2").then((cellData) => { - expect(cellData).to.eq(data.japanese_name); -}); +cy.get('@tableData').then((tableData) => { + expect(tableData).to.deep.equal({ + '0,0': data.uniq_id, + '0,1': data.japanese_name, + }); +});Also applies to: 146-155, 189-198
Test suite needs targeted improvements for stability and maintainability
- Remove
.skipor add a comment explaining the skip reason- Replace
timeout: Cypress.config("pageLoadTimeout")withcy.wait("@postExecute")for more reliable network request handling- Keep the existing
afterhook as it's properly cleaning up test data- Add data-* selectors for button interactions:
// Before locators._buttonByText("Update") // After locators._dataTestId("update-button")🔗 Analysis chain
Line range hint
14-269: Test suite needs improvements to align with best practices.Several areas need attention to comply with our Cypress testing guidelines:
- Test suite is currently skipped. Please either remove the skip or add a comment explaining why it's skipped.
- Replace class-based selectors with data-* attributes:
-locators._buttonByText("Update") +locators._dataAttribute("update-button")
- Replace cy.get timeout pattern with better waiting strategies:
-cy.get("@postExecute", { - timeout: Cypress.config("pageLoadTimeout"), -}).then((interception: any) => { +cy.wait("@postExecute").then((interception: any) => {
Move setup logic from before hook to a separate helper function for better maintainability.
Replace after hook with cy.cleanup() command for better test isolation.
Let's verify the selector usage:
Let me verify the test suite structure and skip reason:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all class-based selectors in Cypress tests rg -l "buttonByText|visibleTextSpan" app/client/cypress/Length of output: 3798
Script:
#!/bin/bash # Check for skip reason comments and test suite structure rg -A 5 "describe\.skip" app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts # Check for cleanup patterns rg "after\(" app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts # Check for existing data-* selectors rg "data-.*=\"" app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts # Check for timeout patterns rg "timeout:" app/client/cypress/e2e/GSheet/GsheetMisc_Spec.tsLength of output: 630
app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts (1)
Line range hint
1-314: Consider improving test reliability and maintainability.While the overall test structure is good, there are several areas for improvement:
- The tests appear to use implicit waits through helper methods. Consider using Cypress's built-in retry-ability and assertions instead.
- The cleanup in afterEach could be moved to before to ensure a clean state for each test.
- Some test cases contain commented code that should be either removed or fixed.
Consider these improvements:
// Instead of using agHelper.WaitUntilAllToastsDisappear(): cy.get('[data-cy=toast-message]').should('not.exist'); // Instead of afterEach, use before: before(() => { dataSources.DeleteDSDirectly(200, false); }); // Remove commented code or add TODO with issue reference: // TODO: Uncomment and fix after resolving issue #XXXX // dataSources.ValidateNSelectDropdown(...)app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts (5)
Line range hint
41-44: Replace Sleep with proper Cypress wait commands.Using
agHelper.Sleep(500)is against best practices as it introduces flaky tests. Instead, wait for the actual condition you're expecting.- agHelper.Sleep(500); + cy.get('[data-cy="spreadsheet-dropdown"]').should('exist').and('be.visible');
Line range hint
264-264: Remove explicit sleep after delete operation.Using
agHelper.Sleep(500)after delete operation can make tests flaky. Wait for the actual deletion confirmation instead.- agHelper.Sleep(500); + cy.get('[data-cy="delete-success-message"]').should('exist');
Line range hint
308-312: Replace Sleep with proper Cypress wait commands in import flow.Using
agHelper.Sleep()after network calls is not recommended. Instead, wait for specific UI elements or network responses.- agHelper.Sleep(); + cy.get('[data-cy="table-widget"]').should('exist');
Line range hint
332-337: Remove explicit sleep in app-level import.Using
agHelper.Sleep()after import is not recommended. Wait for specific UI elements or network responses instead.- agHelper.Sleep(); + cy.get('[data-cy="page-loaded"]').should('exist');
Line range hint
1-365: Consider using data-cy attributes for selectors.Several selectors in the test file could be improved by using data-* attributes instead of class names or other attributes. This would make the tests more resilient to UI changes.
Examples of selectors to update:
- Add data-cy attributes to dropdown elements
- Add data-cy attributes to table elements
- Add data-cy attributes to form elements
Would you like me to provide specific examples for each case?
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts (2)
Line range hint
54-54: Remove usage of agHelper.Sleep().The coding guidelines explicitly prohibit the use of
agHelper.Sleep(). Consider using Cypress's built-in waiting mechanisms or commands that wait for specific conditions instead.Replace sleep calls with appropriate wait conditions:
- agHelper.Sleep(500); + cy.waitUntil(() => /* condition */);Also applies to: 371-371, 436-436
Line range hint
15-15: Test structure needs revision.Two structural issues need attention:
- The test suite is marked as skipped (
describe.skip). Either remove the skip or add a comment explaining why it's skipped.- The use of
afterhook is against the coding guidelines. Consider moving the cleanup logic into individual test cases or refactor to use API calls for cleanup.Also applies to: 391-411
app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (5)
Line range hint
54-57: Replace hardcoded strings with constantsUse constants for dropdown values to improve maintainability and reduce typos.
+ const OPERATIONS = { + FETCH_MANY: "Fetch Many", + FETCH_DETAILS: "Fetch Details" + }; + const ENTITY_TYPES = { + SPREADSHEET: "Spreadsheet" + }; dataSources.ValidateNSelectDropdown( - "Operation", - "Fetch Many", - "Fetch Details", + "Operation", + OPERATIONS.FETCH_MANY, + OPERATIONS.FETCH_DETAILS, ); dataSources.ValidateNSelectDropdown( - "Entity", - "Spreadsheet" + "Entity", + ENTITY_TYPES.SPREADSHEET );
Line range hint
58-58: Remove agHelper.Sleep callReplace
agHelper.Sleep(500)with proper Cypress waiting mechanisms likecy.wait('@apiCall')orcy.intercept().- agHelper.Sleep(500); + cy.intercept('GET', '**/api/v1/spreadsheets/**').as('getSpreadsheets'); + cy.wait('@getSpreadsheets');
Line range hint
61-66: Enhance assertion coverageAdd multiple assertions to validate the response comprehensively.
cy.get("@postExecute").then((interception: any) => { + expect(interception.response.status).to.equal(200); expect(interception.response.body.data.body.name).to.deep.equal( spreadSheetName, ); + expect(interception.response.body.data.body).to.have.property('spreadsheetId'); + expect(interception.response.body.data.body).to.have.property('properties'); });
Line range hint
386-402: Replace after hook with beforeEach/afterEach patternUsing
afterhook for cleanup is discouraged. Consider usingbeforeEach/afterEachfor better test isolation.- after("Delete spreadsheet and app", function () { + beforeEach(function() { + // Setup test data + }); + afterEach(function() { // Delete spreadsheet and app homePage.EditAppFromAppHover(appName); gsheetHelper.DeleteSpreadsheetQuery( dataSourceName.allAccess, spreadSheetName, ); cy.get("@postExecute").then((interception: any) => { expect(interception.response.body.data.body.message).to.deep.equal( "Deleted spreadsheet successfully!", ); }); homePage.NavigateToHome(); homePage.DeleteApplication(appName); + });
Line range hint
1-402: General improvements needed across the test suiteSeveral improvements are needed to align with Cypress best practices:
- Replace all instances of
agHelper.Sleep()with proper Cypress waiting mechanisms- Use data-* attributes for selectors instead of class names or XPaths
- Add type definitions for response data to improve type safety
- Consider breaking down large test files into smaller, more focused test suites
Would you like me to help create a GitHub issue to track these improvements?
app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts (2)
Line range hint
58-59: Remove agHelper.Sleep() callAccording to the coding guidelines, avoid using
agHelper.Sleep(). Instead, wait for specific elements or state changes.- agHelper.Sleep(500); + agHelper.WaitUntilElemAppear("[data-cy=spreadsheet-dropdown]");
Line range hint
371-372: Avoid using after hook for cleanupAccording to the coding guidelines, avoid using
afterhooks. Consider moving the cleanup logic to a separate test case or using API calls directly.- after("Delete spreadsheet and app", function () { + it("11. Cleanup: Delete spreadsheet and app", function () {Also applies to: 373-374, 375-376
app/client/cypress/e2e/Regression/ClientSide/Linting/BasicLint_spec.ts (2)
Line range hint
26-27: Remove usage of agHelper.Sleep()According to the coding guidelines, we should avoid using
agHelper.Sleep(). Consider using Cypress's built-in waiting mechanisms or commands that automatically wait for elements to be actionable.Replace:
- agHelper.Sleep(2000); + cy.get(locators._lintErrorElement).should('not.exist');
Line range hint
1-450: Consider enhancing test reliability and maintainabilityA few suggestions to align with Cypress best practices:
- Replace
agHelper.Sleep()calls with Cypress's built-in waiting mechanisms- Consider using data-* attributes for selectors
- Strengthen multiple assertions in expect statements
Example improvements:
// Add data attributes to elements cy.get('[data-cy="lint-error"]').should('exist').and('be.visible'); // Replace sleep with proper waiting cy.get('[data-cy="button"]').should('be.enabled').click(); // Multiple assertions cy.get('[data-cy="alert"]') .should('be.visible') .and('contain', successMessage) .and('have.class', 'success');app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Numeric_Spec.ts (5)
Line range hint
214-215: Remove usage of agHelper.Sleep.Replace
agHelper.Sleep()calls with proper Cypress wait commands or assertions. Using arbitrary sleep times can make tests flaky and slow.- agHelper.Sleep(2500); //Allwowing time for delete to be success + agHelper.WaitUntilToastDisappear(); + table.WaitForTableLoad();Also applies to: 249-250, 284-285
Line range hint
19-24: Use API for login instead of UI interaction.According to guidelines, use
LoginFromAPIinstead of UI interactions for login operations.- appSettings.OpenPaneAndChangeTheme("Moon"); - dataSources.CreateDataSource("Postgres"); + agHelper.LoginFromAPI(); + dataSources.CreateDataSourceFromApi("Postgres");
Line range hint
116-117: Use data- attributes for button selectors.*Replace button text selectors with data-* attributes for more reliable test selection.
- locators._buttonByText("Run InsertQuery") + locators._button("data-cy=insert-query-btn")Also applies to: 173-174, 234-235
Line range hint
126-143: Use constants for test assertions.Replace string literals in assertions with constants to improve maintainability.
const EXPECTED_VALUES = { SERIAL_ID: "1", BIGINT_ID: "922337203685477", DECIMAL_ID: "865456.987654567", NUMERIC_ID: "2147483647.2147484" }; table.ReadTableRowColumnData(0, 0, "v1", 2000).then(($cellData) => { expect($cellData).to.eq(EXPECTED_VALUES.SERIAL_ID); });
Line range hint
1-15: Add JSDoc comments for the test suite.Add descriptive JSDoc comments to explain the purpose and requirements of the test suite.
/** * Test suite for PostgreSQL Numeric Data Types * * Validates the following data types: * - SERIAL (auto-incrementing integer) * - BIGINT * - DECIMAL * - NUMERIC * * @group regression * @group postgres */app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/DateTime_Spec.ts (1)
Line range hint
1-400: Consider improving test reliability and performance
- Remove
agHelper.Sleep()calls (lines 297, 386) and replace with proper Cypress wait commands or assertions- Consider using data-* attributes for selectors instead of text-based ones (e.g.,
locators._buttonByText)- Consider moving the table creation and cleanup to
before()andafter()hooks instead of separate test casesExample for removing Sleep:
- agHelper.Sleep(2500); //Allwowing time for delete to be success + cy.wait("@postExecute").should("have.property", "status", 200); + table.WaitForTableEmpty();app/client/cypress/support/Pages/ApiPage.ts (1)
Line range hint
1-500: Consider updating remaining selectors to follow best practices.Many selectors in the file still use class selectors and XPaths, which could be brittle. Consider:
- Replace class selectors (e.g.,
.t--createBlankApiCard,.t--dataSourceField) with data-testid attributes- Replace XPath selectors with data-testid attributes
- Remove usage of
agHelper.Sleep()and replace with proper Cypress waiting strategiesExample improvements:
- _createapi = ".t--createBlankApiCard"; + _createapi = "[data-testid='create-blank-api']"; - private _bodyTypeSelect = `//div[@data-testid="t--api-body-tab-switch"]`; + private _bodyTypeSelect = "[data-testid='api-body-tab-switch']";app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Character_Spec.ts (1)
Line range hint
252-252: RemoveagHelper.Sleep()calls.Replace Sleep calls with proper Cypress wait conditions:
- agHelper.Sleep(2500); //Allwowing time for delete to be success + cy.waitUntil(() => { + return cy.get('your-selector').should('have.length', expectedLength); + }); - agHelper.Sleep(2000); + cy.waitUntil(() => { + return table.isTableEmpty(); + });The use of explicit sleep calls is discouraged as it makes tests flaky and slower. Instead, use Cypress's built-in retry-ability and wait conditions that check for specific state changes.
Also applies to: 347-347
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Binary_Spec.ts (3)
Line range hint
1-450: Remove commented-out after block and update test cleanup strategy.The commented-out after block at the end of the file should be either implemented or removed to maintain clean test code.
Consider implementing the cleanup using API calls:
- // after( - "Validate Drop of the Newly Created - binarytype...", - () => { - //Drop table... - }, - ); + after(() => { + // Use API calls for cleanup + cy.request('POST', '/api/v1/datasources/...', { + query: 'DROP TABLE IF EXISTS public.binarytype' + }); + });
Line range hint
1-450: Replace wait/sleep calls with better assertions.The test uses implicit waits through helper methods which is against best practices:
table.WaitUntilTableLoad()agHelper.WaitUntilEleDisappear()Replace waits with explicit assertions:
// Instead of table.WaitUntilTableLoad() cy.get('table').should('exist').and('be.visible'); // Instead of agHelper.WaitUntilEleDisappear() cy.get(locators._btnSpinner).should('not.exist');
Line range hint
1-450: Add login/logout via API calls.The test should use API calls for authentication operations as per guidelines.
Add these at the beginning and end of the test:
before(() => { cy.LoginFromAPI(Cypress.env('USERNAME'), Cypress.env('PASSWORD')); }); after(() => { cy.LogOutviaAPI(); });app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts (3)
Line range hint
191-191: Replace agHelper.Sleep() with proper Cypress wait conditionsUsing fixed timeouts with
agHelper.Sleep()makes tests flaky and violates the coding guidelines. Replace these with proper wait conditions:- agHelper.Sleep(2000); //for table selection to be captured + agHelper.WaitUntilAllToastsDisappear(); + table.WaitUntilTableLoad(); - agHelper.Sleep(5000); //some more time for rows to rearrange for CI flakyness! + table.WaitUntilTableLoad(); - agHelper.Sleep(2000); // Above entensions settling time + agHelper.WaitUntilAllToastsDisappear(); + dataSources.WaitForQueryEditor(); - agHelper.Sleep(2000); + table.WaitForTableEmpty();Also applies to: 207-207, 307-307, 420-420
Line range hint
19-29: Add cleanup in after() hookWhile the setup in
before()is good, consider adding cleanup in anafter()hook to ensure the database is left in a clean state, even if tests fail:+ after(() => { + // Clean up the database + dataSources.DeleteDatasourceFromWithinDS(dsName, 200); + });
Line range hint
101-101: Use data- attributes for selectors*Replace text-based button selectors with data-* attributes for better maintainability:
- locators._buttonByText("Run InsertQuery") + locators._button("[data-cy=insert-query-btn]") - table.ReadTableRowColumnData(0, 0) + table.ReadTableRowColumnData("[data-row-id=0]", "[data-col-id=serialid]")Also applies to: 102-102, 103-103, 104-104, 105-105
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Array_Spec.ts (3)
Line range hint
155-155: Remove agHelper.Sleep() callsAccording to the coding guidelines, we should avoid using
agHelper.Sleep(). Consider using Cypress's built-in waiting mechanisms or retry-ability instead.Replace the sleep calls with appropriate Cypress commands:
- agHelper.Sleep(5000); //some more time for rows to rearrange! + cy.waitUntil(() => table.ReadTableRowColumnData(1, 0, "v1", 2000).then($cellData => $cellData === "3"));
Line range hint
495-495: Replace remaining Sleep() calls with Cypress wait mechanismsMultiple instances of
agHelper.Sleep()found in the test file. These should be replaced with Cypress's built-in waiting mechanisms.Replace the sleep calls with appropriate wait conditions:
- agHelper.Sleep(2500); //Allwowing time for delete to be success + cy.waitUntil(() => table.ReadTableRowColumnData(1, 0, "v1", 2000).then($cellData => $cellData !== "3")); - agHelper.Sleep(2000); + table.WaitForTableEmpty();Also applies to: 507-507, 557-557
Line range hint
12-24: Consider using API calls for test data setupThe test setup is using UI interactions to create test data. According to the coding guidelines, we should use API calls for data setup.
Consider refactoring the setup to use API calls:
before("Create DS, Add DS & setting theme", () => { cy.request({ method: 'POST', url: '/api/v1/datasources', body: { name: 'PostgresDS', type: 'postgres', // ... other configuration } }).then(response => { dsName = response.body.data.name; }); // Add DSL via API cy.request('POST', '/api/v1/pages/dsl', { dsl: "Datatypes/ArrayDTdsl" }); // Set theme via API cy.request('PUT', '/api/v1/themes', { colors: { primary: -31, secondary: -27 } }); });app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts (6)
Line range hint
1-24: Remove unnecessary imports.Several imported modules are not used in the test file:
assertHelper,draggableWidgets,entityItems,locators,propPane,table,Widgets,OneClickBinding,EntityType,PageList.
Line range hint
32-38: Avoid using cy.wait and sleep in tests.The test uses
agHelper.Sleep()which is against the coding guidelines. Instead, use Cypress's built-in retry-ability and wait for specific elements or network requests.Replace sleep with proper assertions:
- agHelper.Sleep(2000); + cy.get('@postExecute').should('have.property', 'status', 200);Also applies to: 73-74
Line range hint
146-196: Use data- attributes for selectors.*The test uses complex CSS selectors and text content for element selection. According to the coding guidelines, use data-* attributes instead.
Example refactor:
- agHelper.GetNClick(table._searchInput) + agHelper.GetNClick('[data-cy=table-search-input]')
Line range hint
198-255: Improve error validation test structure.The error validation test case could be split into smaller, focused test cases for each type of error (missing endpoint, missing username, missing password).
Consider restructuring like this:
it("should validate missing endpoint error", () => { // Test only endpoint missing scenario }); it("should validate missing username error", () => { // Test only username missing scenario }); it("should validate missing password error", () => { // Test only password missing scenario });
Line range hint
324-331: Add cleanup in after hook.The after hook only deletes the datasource but doesn't clean up the created tables. Add cleanup for all created database objects.
Add table cleanup:
after("Verify Deletion of the datasource", () => { cy.intercept("DELETE", "/api/v1/datasources/*").as("deleteDatasource"); dataSources.DeleteDatasourceFromWithinDS(dsName, 409); + // Clean up created tables + query = "DROP TABLE IF EXISTS amazon_sales; DROP TABLE IF EXISTS Simpsons;"; + dataSources.EnterQuery(query); + dataSources.RunQuery(); });
Add tests for CRUD operations in the generated page
The current test only verifies the initial table load and JSON form presence. Add test cases to verify:
- Create operation by adding a new record
- Update operation by modifying an existing record
- Delete operation by removing a record
- Verify the table state after each operation
Example locations to add tests:
- After line 379: Test create operation
- After line 382: Test update operation
- After line 385: Test delete operation
🔗 Analysis chain
Line range hint
257-322: Add assertions for CRUD operations.The CRUD template generation test lacks assertions for create, update, and delete operations. Add comprehensive checks for each operation.
Let me gather more information about the CRUD template generation test to verify if there are any assertions for create, update, and delete operations.
Let me search for any actual CRUD operation tests in the test file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if all CRUD operations are properly tested rg -A 5 "create|update|delete.*assert" app/client/cypress/e2e/Sanity/Datasources/Length of output: 18489
Script:
#!/bin/bash # Search for the GenerateCRUDNValidateDeployPage function implementation ast-grep --pattern 'function GenerateCRUDNValidateDeployPage($$$) { $$$ }' # Also search for any CRUD-related assertions in the test file rg -A 5 "GenerateCRUDNValidateDeployPage" app/client/cypress/e2e/Sanity/Datasources/Length of output: 34835
Script:
#!/bin/bash # Search for create/update/delete operations in the test file rg -B 2 -A 5 "create|update|delete.*test|it\(" app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.tsLength of output: 2926
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Json_Spec.ts (3)
Line range hint
171-172: ReplaceagHelper.Sleep()with proper Cypress wait conditions.Using explicit sleep statements can make tests flaky and slow. Consider using Cypress's built-in wait mechanisms or retry-ability instead.
-agHelper.Sleep(5000); //Allowing time for update to be success for CI flaky behavior +agHelper.WaitUntilToastDisappear(); +agHelper.WaitUntilAllToastsDisappear();Also applies to: 425-426, 516-517
Line range hint
171-174: Avoid hardcoded values in assertions.Store expected values in constants or test fixtures to improve maintainability.
+const EXPECTED_VALUES = { + FIRST_ROW: "1", + SECOND_ROW: "2", + THIRD_ROW: "3", +}; -expect($cellData).to.eq("3"); +expect($cellData).to.eq(EXPECTED_VALUES.THIRD_ROW);Also applies to: 425-429
Line range hint
4-4: Consider using data-testid attributes for selectors.The test file imports locators but still uses some class-based selectors. Consider using data-testid attributes for more reliable test selectors.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (2)
Line range hint
831-844: Replace Sleep() with Cypress built-in waiting mechanisms.Using
agHelper.Sleep()and arbitrary wait times can make tests flaky. Instead, use Cypress's built-in waiting mechanisms.- agHelper.Sleep(2000); //for documents value to settle! + agHelper.WaitUntilAllToastsDisappear(); + cy.waitUntil(() => { + return cy.get('@postExecute').then((response) => { + return response.status === 200; + }); + });
Line range hint
1-894: Consider refactoring test file to follow Cypress best practices.The test file contains multiple instances of
cy.wait()andagHelper.Sleep(). Consider refactoring to:
- Use
cy.intercept()with waiting for network requests- Use Cypress's built-in retry-ability
- Wait for elements to reach actionable states instead of using arbitrary timeouts
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (51)
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts(1 hunks)app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts(1 hunks)app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts(1 hunks)app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts(1 hunks)app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/BugTests/DSDiscardBugs_spec.ts(6 hunks)app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug19893_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug25148_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/BugTests/DatasourceSchema_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/CopyQuery_RenameDatasource_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Query_Datasource_spec.js(2 hunks)app/client/cypress/e2e/Regression/ClientSide/Linting/BasicLint_spec.ts(2 hunks)app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts(2 hunks)app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts(5 hunks)app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/Basic_Spec.ts(3 hunks)app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/False_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/PostgresConnections_spec.ts(2 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Array_Spec.ts(2 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Binary_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/BooleanEnum_Spec.ts(2 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Character_Spec.ts(6 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/DateTime_Spec.ts(7 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Json_Spec.ts(5 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Numeric_Spec.ts(6 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/DSDocs_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts(4 hunks)app/client/cypress/e2e/Sanity/Datasources/ArangoDataSourceStub_spec.js(2 hunks)app/client/cypress/e2e/Sanity/Datasources/DSAutosaveImprovements_spec.ts(1 hunks)app/client/cypress/e2e/Sanity/Datasources/MongoDatasourceURI_spec.ts(2 hunks)app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts(1 hunks)app/client/cypress/locators/ApiEditor.js(1 hunks)app/client/cypress/locators/JSEditor.json(1 hunks)app/client/cypress/locators/QueryEditor.json(1 hunks)app/client/cypress/locators/apiWidgetslocator.json(1 hunks)app/client/cypress/support/ApiCommands.js(2 hunks)app/client/cypress/support/Objects/CommonLocators.ts(2 hunks)app/client/cypress/support/Pages/AggregateHelper.ts(1 hunks)app/client/cypress/support/Pages/ApiPage.ts(3 hunks)app/client/cypress/support/Pages/DataSources.ts(7 hunks)app/client/cypress/support/Pages/GSheetHelper.ts(3 hunks)app/client/cypress/support/Pages/JSEditor.ts(3 hunks)app/client/src/PluginActionEditor/components/PluginActionToolbar.tsx(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionRun.tsx(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/constants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/PluginActionEditor/components/PluginActionToolbar.tsx
🧰 Additional context used
📓 Path-based instructions (47)
app/client/cypress/e2e/GSheet/AllAccess_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/GSheet/GsheetMisc_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/GSheet/ReadNWrite_Access_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/GSheet/ReadOnly_Access_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/GSheet/SelectedSheet_Access_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/DSDiscardBugs_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/DS_Bug19893_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/DS_Bug25148_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/DatasourceSchema_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/ExplorerTests/CopyQuery_RenameDatasource_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/ClientSide/ExplorerTests/Query_Datasource_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/ClientSide/Linting/BasicLint_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/OneClickBinding/PropertyControl_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/OtherUIFeatures/Omnibar_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/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_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/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_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/ClientSide/Widgets/TableV2/columnTypes/Select1_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/Datasources/ConnectionErrors_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/GenerateCRUD/MongoURI_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/MySQL_Datatypes/Basic_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/MySQL_Datatypes/False_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/PostgresConnections_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/Postgres_DataTypes/Array_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/Postgres_DataTypes/Binary_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/Postgres_DataTypes/BooleanEnum_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/Postgres_DataTypes/Character_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/Postgres_DataTypes/DateTime_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/Postgres_DataTypes/Json_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/Postgres_DataTypes/Numeric_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/Postgres_DataTypes/UUID_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/DSDocs_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/Sanity/Datasources/ArangoDataSourceStub_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/Sanity/Datasources/DSAutosaveImprovements_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/Sanity/Datasources/MongoDatasourceURI_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/Sanity/Datasources/MsSQL_Basic_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/locators/ApiEditor.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/locators/JSEditor.json (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/locators/QueryEditor.json (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/locators/apiWidgetslocator.json (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/ApiCommands.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/Objects/CommonLocators.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/AggregateHelper.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/ApiPage.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/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/GSheetHelper.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/JSEditor.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 (72)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/constants.ts (1)
10-10: LGTM! The new test locator follows the standard "t--" prefix pattern.
app/client/cypress/locators/JSEditor.json (1)
2-2: LGTM! Selector change follows best practices.
The update to use data-testid attribute is aligned with our guidelines for Cypress selectors.
app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug19893_spec.ts (1)
15-15: LGTM! Method rename aligns with new standards.
The change from RenameWithInPane to RenameDatasource follows the standardization effort.
app/client/cypress/e2e/Regression/ClientSide/BugTests/DS_Bug25148_Spec.ts (1)
16-16: LGTM: Method name change improves clarity.
The change from RenameWithInPane to RenameDatasource better reflects the specific action being performed.
app/client/cypress/locators/QueryEditor.json (1)
7-7: LGTM! The selector update follows best practices.
The change to use data-testid attribute is correct and aligns with Cypress best practices.
app/client/cypress/locators/ApiEditor.js (1)
7-7: LGTM! Good transition to data-testid selectors.
The changes align with Cypress best practices by using data-testid attributes instead of class-based selectors.
Also applies to: 9-9
app/client/cypress/e2e/Sanity/Datasources/MongoDatasourceURI_spec.ts (1)
25-25: LGTM: Helper method update.
The change from RenameWithInPane to RenameDatasource aligns with the standardization efforts across test files.
Also applies to: 41-41
app/client/cypress/e2e/Sanity/Datasources/ArangoDataSourceStub_spec.js (1)
12-12: LGTM: Method signature simplification
The change from RenameWithInPane to RenameDatasource improves code clarity by using a more descriptive name and simpler signature.
Also applies to: 24-24
app/client/cypress/e2e/Sanity/Datasources/DSAutosaveImprovements_spec.ts (1)
43-43: LGTM! Method name standardization looks good.
The change from RenameWithInPane to RenameDatasource aligns with the codebase-wide standardization effort.
app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/False_Spec.ts (1)
35-35: LGTM: Query renaming method updated
The change from RenameWithInPane to RenameQuery aligns with the standardization efforts across test files.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (2)
89-89: LGTM: Test selector addition aligns with Cypress best practices.
The addition of the data-testid attribute enhances the component's testability in Cypress.
Line range hint 42-44: Verify feature flag implementation across the codebase.
The component uses the release_actions_redesign_enabled feature flag to toggle between old and new versions.
✅ Verification successful
Let me verify if there are any old components by searching for files in a different location pattern.
Feature flag implementation is correctly structured
The feature flag release_actions_redesign_enabled is properly implemented with:
- Old component exists at
components/old/JSFunctionRun.tsx - New component correctly imports the old version as
OldJSFunctionRun - Feature flag is consistently used across related components in the editor
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of the feature flag across the codebase
# and ensure the old version exists
# Check for feature flag usage
rg -A 2 "release_actions_redesign_enabled"
# Verify the existence of the old component
fd "old/JSFunctionRun" --type f
Length of output: 4236
Script:
#!/bin/bash
# Search for any JSFunctionRun files that might be in an "old" directory structure
fd "JSFunctionRun" --type f
# Check if there are any imports or references to old JSFunctionRun components
rg -A 2 "import.*JSFunctionRun.*old"
rg -A 2 "from.*old.*JSFunctionRun"
Length of output: 1132
app/client/cypress/locators/apiWidgetslocator.json (1)
63-63: LGTM! Good improvement in selector strategy.
The change from class selector to data-testid attribute follows Cypress best practices.
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Query_Datasource_spec.js (1)
66-70: LGTM: Correct usage of RenameDatasource
The datasource renaming implementation follows the recommended patterns.
app/client/cypress/support/Pages/GSheetHelper.ts (3)
39-39: LGTM: Method rename implemented correctly
The change from RenameWithInPane to RenameQuery aligns with the standardization effort.
61-61: LGTM: Method rename implemented correctly
The change from RenameWithInPane to RenameQuery maintains consistency.
128-130: LGTM: Method rename implemented correctly
The change from RenameWithInPane to RenameQuery is properly implemented with the operation name formatting.
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js (1)
69-69: LGTM: Method rename aligns with the codebase changes.
The change from RenameWithInPane to RenameQuery is consistent with the modifications across other test files.
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (1)
165-165: LGTM: Improved query renaming implementation
The change from manual click operations to using agHelper.RenameQuery is a good improvement that follows our helper function guidelines.
app/client/cypress/e2e/Regression/ClientSide/BugTests/DatasourceSchema_spec.ts (2)
32-32: LGTM! Method rename improves clarity.
The change from RenameWithInPane to RenameDatasource makes the intention clearer and aligns with similar changes across other test files.
Line range hint 47-47: Address skipped test for bug #36348.
The test is currently skipped due to bug #36348. Please verify if the bug has been resolved and update the test accordingly.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/DSDocs_Spec.ts (2)
168-168: LGTM! Method rename improves clarity.
The rename from RenameWithInPane to RenameDatasource better reflects the method's purpose.
Line range hint 158-174: Use data- attributes for selectors in CreateDummyDSNSave.*
The function might be using implicit selectors through the helper methods. Ensure all underlying selectors use data-* attributes as per our guidelines.
app/client/cypress/e2e/Regression/ServerSide/MySQL_Datatypes/Basic_Spec.ts (1)
53-53: LGTM: Query renaming implementation changes.
The changes from RenameWithInPane to RenameQuery are consistent with the codebase-wide standardization effort.
Also applies to: 62-62, 71-71
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/PostgresConnections_spec.ts (1)
28-28: LGTM: Method name changes are consistent.
The replacement of RenameWithInPane with RenameDatasource aligns with the standardization effort and provides better semantic clarity.
Also applies to: 59-59
app/client/cypress/support/ApiCommands.js (1)
14-14: LGTM: Good practice using centralized locators
The addition of CommonLocators from ObjectsRegistry aligns with best practices for maintainable selectors.
app/client/cypress/e2e/Regression/ClientSide/BugTests/DSDiscardBugs_spec.ts (5)
44-45: Remove redundant sleep call after save operation.
Similar to the previous instance, replace the explicit sleep with a proper wait condition.
67-68: Remove explicit sleep after datasource save.
Another instance of using Sleep() that should be replaced with proper wait conditions.
91-92: Remove sleep call after save operation.
Another instance of using Sleep() that should be replaced.
115-116: Remove explicit sleep call.
Another instance of using Sleep() that should be replaced.
138-139: Remove explicit sleep call.
Another instance of using Sleep() that should be replaced.
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js (2)
193-193: LGTM: Proper usage of helper function
The change to use _.agHelper.RenameQuery() is correct and follows best practices.
Line range hint 1-234: Multiple improvements needed for test reliability
Several issues need to be addressed to align with coding guidelines:
- Replace CSS selectors with data-testid attributes
- Use LoginFromAPI instead of UI interactions
- Remove all cy.wait() calls
- Improve selector usage
Here are some examples of selectors that need to be updated:
-cy.get(".t--list-widget-next-page.rc-pagination-next")
+cy.get('[data-testid="list-next-page"]')
-cy.get(".t--draggable-buttonwidget")
+cy.get('[data-testid="button-widget"]')
-cy.get(".rc-pagination-item")
+cy.get('[data-testid="pagination-item"]')Let's verify if these selectors are consistently used across other test files:
app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts (2)
112-112: LGTM: Datasource renaming implementation looks good.
The usage of RenameDatasource follows the new naming convention consistently.
139-139: LGTM: Consistent usage of RenameDatasource method.
The implementation correctly uses template literals for dynamic naming.
app/client/cypress/support/Pages/JSEditor.ts (1)
34-34: Good update to use data-testid!
The change from class selector to data-testid for _runButton aligns with best practices.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts (1)
33-33: LGTM! Improved method usage for renaming datasource
The change to use RenameDatasource instead of RenameWithInPane is more specific and clearer in its intent.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/BooleanEnum_Spec.ts (2)
70-70: LGTM: Query rename implementation looks good.
The change from RenameWithInPane to RenameQuery aligns with the current API.
172-172: LGTM: Query rename implementation looks good.
The change from RenameWithInPane to RenameQuery aligns with the current API.
app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts (1)
69-69: LGTM: Method rename aligns with codebase refactoring.
The change from RenameWithInPane to RenameQuery is consistent with the broader refactoring effort.
app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts (2)
25-25: LGTM: Consistent method naming improves maintainability.
The change from RenameWithInPane to RenameDatasource standardizes the datasource renaming method across all test cases, making the code more maintainable and the intent clearer.
Also applies to: 84-84, 136-136, 237-237, 258-258
Line range hint 13-22: Verify test isolation and setup.
The test suite uses a shared dataSourceName variable and relies on afterEach for cleanup. This could lead to test interdependence if cleanup fails.
Let's verify the cleanup reliability:
✅ Verification successful
Let me analyze the test isolation concern with one more verification to check the cleanup implementation:
Test cleanup is properly implemented with error handling
The cleanup implementation is reliable for the following reasons:
- DeleteDSDirectly has proper error handling with expected response validation
- The cleanup uses AssertNetworkStatus to verify deletion success
- Each test assigns a unique ID to dataSourceName, preventing test interference
- The afterEach hook ensures consistent cleanup regardless of test outcome
The shared variable usage is intentional for cleanup purposes and doesn't impact test isolation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential cleanup issues in other test files
rg -A 2 "DeleteDSDirectly" "app/client/cypress"
# Search for shared variable usage patterns
rg "dataSourceName.*=.*" "app/client/cypress"
Length of output: 3035
Script:
#!/bin/bash
# Check the DeleteDSDirectly implementation for error handling
ast-grep --pattern 'DeleteDSDirectly($$$) {
$$$
}'
# Check if there are any test failures related to cleanup
rg -B2 "failed.*DeleteDSDirectly" "app/client/cypress"
Length of output: 109
Script:
#!/bin/bash
# Let's check the actual implementation in DataSources.ts
rg -A 10 "public DeleteDSDirectly" "app/client/cypress/support/Pages/DataSources.ts"
# Check for any retry or error handling patterns in the test files
rg "cy\.(wait|intercept).*DeleteDSDirectly" "app/client/cypress"
Length of output: 626
Script:
#!/bin/bash
# Check the ValidateDSDeletion implementation
rg -A 5 "ValidateDSDeletion" "app/client/cypress/support/Pages/DataSources.ts"
# Check if dataSourceName is properly scoped in the test file
rg -A 10 "describe.*ConnectionErrors_Spec" "app/client/cypress/e2e/Regression/ServerSide/Datasources/ConnectionErrors_Spec.ts"
Length of output: 506
app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts (1)
37-37: LGTM: Method rename is consistent with the codebase changes.
The change from RenameWithInPane to RenameQuery aligns with the standardization effort across test files.
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts (1)
49-49: Verify the method rename impact.
The change from RenameWithInPane to RenameQuery appears to be part of a broader API update. Please ensure all test files are updated consistently with this change.
✅ Verification successful
The method rename is consistent across the codebase
The change from RenameWithInPane to RenameQuery is valid as:
RenameWithInPaneis not used anywhere in the codebaseRenameQueryis properly defined inAggregateHelperclass and used consistently across test files- The method is used in similar contexts for renaming queries, APIs, and database operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old method name
rg "RenameWithInPane" --type ts
Length of output: 31
Script:
#!/bin/bash
# Let's verify the new method name exists in agHelper
rg "RenameQuery" -A 2 -B 2 --type ts
# Also check for any potential interface or type definitions
ast-grep --pattern 'interface $_ {
$$$
RenameQuery($$$): $$$
$$$
}'
Length of output: 37741
app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (1)
53-53: LGTM: Method rename looks correct
The change from RenameWithInPane to RenameQuery appears to be part of a consistent API update.
app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts (2)
53-53: LGTM: Method rename is consistent with codebase changes
The rename from RenameWithInPane to RenameQuery aligns with the broader codebase changes for method naming standardization.
Line range hint 1-386: Verify test stability without explicit waits
The test contains multiple instances of agHelper.Sleep() and cy.wait(). Consider replacing them with more reliable waiting strategies:
- Line 371:
agHelper.Sleep() - Line 306:
agHelper.Sleep() - Line 328:
agHelper.Sleep()
✅ Verification successful
Based on the search results and the learnings from user feedback, I can now provide a final response:
Replace explicit waits with Cypress built-in retry-ability and assertions
The test contains multiple instances of agHelper.Sleep() and cy.wait() which should be replaced with Cypress's built-in retry-ability and assertions:
- Use
cy.get()with assertions instead of arbitrary waits - Replace
agHelper.Sleep()withcy.should()assertions on element states - For network requests, use
cy.wait('@alias')only when absolutely necessary - For animations/transitions, use
cy.should('be.visible')or similar assertions
Example:
// Instead of:
agHelper.Sleep(500);
cy.get(selector).click();
// Use:
cy.get(selector).should('be.visible').click();🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for explicit wait patterns in Cypress tests
echo "Searching for explicit waits in Cypress tests..."
rg -n "agHelper\.Sleep|cy\.wait" "app/client/cypress/e2e/"
Length of output: 229950
app/client/cypress/e2e/Regression/ClientSide/Linting/BasicLint_spec.ts (2)
118-118: LGTM: Method rename changes look good
The replacement of RenameWithInPane with RenameQuery is consistent with the API entity renaming operations.
Also applies to: 123-123
240-240: LGTM: Method rename changes look good
The replacement of RenameWithInPane with RenameQuery is consistent with the Query entity renaming operations.
Also applies to: 246-246
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Numeric_Spec.ts (1)
50-50: LGTM: Method renaming is consistent.
The renaming from RenameWithInPane to RenameQuery is applied consistently across all query renames.
Also applies to: 62-62, 75-75, 84-84, 93-93, 103-103
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/DateTime_Spec.ts (7)
62-62: LGTM!
The method update from RenameWithInPane to RenameQuery is correct.
74-74: LGTM!
The method update to RenameQuery is correct for the insert query.
86-86: LGTM!
The method update to RenameQuery is correct for the update query.
96-96: LGTM!
The method update to RenameQuery is correct for the delete all records query.
105-105: LGTM!
The method update to RenameQuery is correct for the drop table query.
115-115: LGTM!
The method update to RenameQuery is correct for the delete record query.
130-130: LGTM!
The method update to RenameQuery is correct for the interval records query.
app/client/cypress/support/Pages/ApiPage.ts (2)
56-56: LGTM! Selector change follows best practices.
The change from class selector to data-testid improves test reliability and maintainability.
131-131: LGTM! Method rename improves clarity.
The consistent renaming from RenameWithInPane to RenameQuery across the file enhances code readability.
Also applies to: 464-464
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Character_Spec.ts (1)
54-54: LGTM: Method rename changes are consistent.
The changes from RenameWithInPane to RenameQuery are consistently applied across all query renames.
Also applies to: 66-66, 80-80, 89-89, 98-98, 107-107
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Binary_Spec.ts (1)
53-53: LGTM: Method rename aligns with new naming convention.
The change from RenameWithInPane to RenameQuery is consistent with the broader refactoring effort across test files.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts (1)
65-65: LGTM: Query renaming changes are consistent
The changes from RenameWithInPane to RenameQuery are consistent and properly implemented across all query renaming operations.
Also applies to: 72-72, 77-77, 82-82, 87-87, 92-92
app/client/cypress/support/Objects/CommonLocators.ts (1)
73-73: LGTM! Selector follows data-testid convention
The updated selector uses data-testid attribute which aligns with our Cypress testing guidelines.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Array_Spec.ts (1)
70-70: LGTM: Query renaming method update
The change from RenameWithInPane to RenameQuery is consistent with the codebase's evolution.
Also applies to: 183-183
app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts (1)
255-255: LGTM: Method rename is consistent with the PR objectives.
The change from RenameWithInPane to RenameDatasource improves clarity and follows the naming convention updates.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/Json_Spec.ts (2)
58-58: LGTM! Method renames look consistent.
The changes from RenameWithInPane to RenameQuery are applied consistently across all query operations.
Also applies to: 71-71, 77-77, 83-83, 88-88, 93-93, 359-359, 369-369, 382-382, 388-388, 394-394, 399-399, 404-404, 409-409, 414-414
Line range hint 1-600: Verify test coverage for edge cases.
The test suite covers basic CRUD operations but could benefit from additional edge cases:
- Invalid JSON/JSONB input
- Boundary conditions for nested JSON structures
- Error handling scenarios
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (1)
745-745: LGTM! Query renaming changes look good.
The renaming of queries follows a consistent pattern and provides clear, descriptive names that reflect their purpose.
Also applies to: 765-765, 831-831, 876-876
app/client/cypress/support/Pages/AggregateHelper.ts (1)
265-271: LGTM!
The method is simple and focused. The default dblClick: true parameter makes sense for query renaming.
app/client/cypress/support/Pages/DataSources.ts (2)
121-121: Improved selector reliability by using data-testid
The change from class-based selector to data-testid attribute improves test reliability and maintainability.
730-730: Consistent method naming with RenameQuery
The standardization of rename method from RenameWithInPane to RenameQuery improves code clarity and maintains consistency across the codebase.
Also applies to: 767-767, 909-909, 1186-1186, 1244-1244, 1576-1576
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/Select1_spec.ts (1)
413-414:
Replace plain string selectors with locator variables
Please use locator variables instead of plain string selectors for better maintainability and consistency.
Apply this diff to use locator variables:
- cy.get(".t--property-control-serversidefiltering input").click();
+ cy.get(_.locators._propertyControlInput("serversidefiltering")).click();
- cy.updateCodeInput(
- ".t--property-pane-section-selectproperties",
- `
- {{Query1.data.map((d) => ({
- label: d.name,
- value: d.name
- }))}}
- `);
+ cy.updateCodeInput(
+ _.locators._propertyControl("selectproperties"),
+ `
+ {{Query1.data.map((d) => ({
+ label: d.name,
+ value: d.name
+ }))}}
+ `);Likely invalid or redundant comment.
| _.agHelper.Sleep(); | ||
| _.dataSources.EditDatasource(); |
There was a problem hiding this comment.
Remove explicit sleep calls.
Replace _.agHelper.Sleep() with proper Cypress commands that wait for specific conditions. Cypress has built-in retry-ability and automatic waiting.
-_.dataSources.SaveDatasource();
-_.agHelper.Sleep();
+_.dataSources.SaveDatasource();
+_.agHelper.WaitUntilToastDisappear();Committable suggestion skipped: line range outside the PR's diff.
| private _jsObjName = ".editor-tab.active > .ads-v2-text"; | ||
| public _jsObjTxt = ".editor-tab.active > .ads-v2-text input"; |
There was a problem hiding this comment.
Replace class selectors with data-testid attributes.
The locators _jsObjName and _jsObjTxt are using class selectors which are brittle and could break with UI changes.
Apply this diff to use data-testid instead:
- private _jsObjName = ".editor-tab.active > .ads-v2-text";
- public _jsObjTxt = ".editor-tab.active > .ads-v2-text input";
+ private _jsObjName = "[data-testid='t--js-editor-name']";
+ public _jsObjTxt = "[data-testid='t--js-editor-name-input']";📝 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.
| private _jsObjName = ".editor-tab.active > .ads-v2-text"; | |
| public _jsObjTxt = ".editor-tab.active > .ads-v2-text input"; | |
| private _jsObjName = "[data-testid='t--js-editor-name']"; | |
| public _jsObjTxt = "[data-testid='t--js-editor-name-input']"; |
| dataSources.ValidateNSelectDropdown( | ||
| "Operation", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using data-cy attributes for dropdown selections
Replace string selectors with data-cy attributes for better maintainability.
- dataSources.ValidateNSelectDropdown(
- "Operation",
+ dataSources.ValidateNSelectDropdown(
+ "[data-cy=operation-dropdown]",Also applies to: 56-57
| _queryName = ".editor-tab.active > .ads-v2-text"; | ||
| _queryNameTxt = ".editor-tab.active > .ads-v2-text input"; |
There was a problem hiding this comment.
Use data- attributes instead of CSS selectors*
The selectors .editor-tab.active > .ads-v2-text are brittle as they depend on CSS classes. According to our Cypress guidelines, we should use data-* attributes for more reliable test selectors.
Consider updating the selectors to use data attributes:
- _queryName = ".editor-tab.active > .ads-v2-text";
- _queryNameTxt = ".editor-tab.active > .ads-v2-text input";
+ _queryName = "[data-testid='t--query-name']";
+ _queryNameTxt = "[data-testid='t--query-name-input']";📝 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.
| _queryName = ".editor-tab.active > .ads-v2-text"; | |
| _queryNameTxt = ".editor-tab.active > .ads-v2-text input"; | |
| _queryName = "[data-testid='t--query-name']"; | |
| _queryNameTxt = "[data-testid='t--query-name-input']"; |
| public RenameDatasource(renameVal: string) { | ||
| this.rename({ | ||
| nameLocator: this.locator._dsName, | ||
| textInputLocator: this.locator._dsNameTxt, | ||
| renameVal, | ||
| }); | ||
| this.AssertElementVisibility(this.locator._editIcon); | ||
| this.Sleep(); // wait for url update | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating assertion logic
The visibility assertion after rename could be moved into the rename method to avoid duplication across callers.
public RenameDatasource(renameVal: string) {
this.rename({
nameLocator: this.locator._dsName,
textInputLocator: this.locator._dsNameTxt,
renameVal,
+ assertSelector: this.locator._editIcon
});
- this.AssertElementVisibility(this.locator._editIcon);
}Committable suggestion skipped: line range outside the PR's diff.
| private rename(args: { | ||
| nameLocator: string; | ||
| textInputLocator: string; | ||
| renameVal: string; | ||
| dblClick?: boolean; | ||
| }) { | ||
| const { | ||
| nameLocator, | ||
| textInputLocator, | ||
| renameVal, | ||
| dblClick = false | ||
| } = args; | ||
|
|
||
| this.Sleep(300); | ||
|
|
||
| if (dblClick) { | ||
| cy.get(nameLocator).dblclick({ force: true }); | ||
| } else { | ||
| this.GetNClick(nameLocator, 0, true); | ||
| } | ||
|
|
||
| cy.get(textInputLocator) | ||
| .clear({ force: true }) | ||
| .type(renameVal, { force: true, delay: 0 }) | ||
| .should("have.value", renameVal) | ||
| .blur(); | ||
| this.PressEnter(); //allow lil more time for new name to settle | ||
|
|
||
| this.PressEnter(); | ||
|
|
||
| this.Sleep(); | ||
| } |
There was a problem hiding this comment.
Avoid using Sleep() and force: true in tests
The rename method has potential reliability issues:
- Using
Sleep()makes tests flaky and slow. Replace with Cypress's built-in retry-ability and waiting mechanisms. - Using
force: truebypasses Cypress's actionability checks. Consider making elements properly actionable instead.
Consider this more reliable implementation:
private rename(args: {
nameLocator: string;
textInputLocator: string;
renameVal: string;
dblClick?: boolean;
}) {
const {
nameLocator,
textInputLocator,
renameVal,
dblClick = false
} = args;
- this.Sleep(300);
+ cy.get(nameLocator).should('be.visible');
if (dblClick) {
- cy.get(nameLocator).dblclick({ force: true });
+ cy.get(nameLocator).should('be.visible').dblclick();
} else {
- this.GetNClick(nameLocator, 0, true);
+ cy.get(nameLocator).should('be.visible').click();
}
cy.get(textInputLocator)
+ .should('be.visible')
- .clear({ force: true })
+ .clear()
- .type(renameVal, { force: true, delay: 0 })
+ .type(renameVal)
.should('have.value', renameVal)
.blur();
this.PressEnter();
- this.Sleep();
}📝 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.
| private rename(args: { | |
| nameLocator: string; | |
| textInputLocator: string; | |
| renameVal: string; | |
| dblClick?: boolean; | |
| }) { | |
| const { | |
| nameLocator, | |
| textInputLocator, | |
| renameVal, | |
| dblClick = false | |
| } = args; | |
| this.Sleep(300); | |
| if (dblClick) { | |
| cy.get(nameLocator).dblclick({ force: true }); | |
| } else { | |
| this.GetNClick(nameLocator, 0, true); | |
| } | |
| cy.get(textInputLocator) | |
| .clear({ force: true }) | |
| .type(renameVal, { force: true, delay: 0 }) | |
| .should("have.value", renameVal) | |
| .blur(); | |
| this.PressEnter(); //allow lil more time for new name to settle | |
| this.PressEnter(); | |
| this.Sleep(); | |
| } | |
| private rename(args: { | |
| nameLocator: string; | |
| textInputLocator: string; | |
| renameVal: string; | |
| dblClick?: boolean; | |
| }) { | |
| const { | |
| nameLocator, | |
| textInputLocator, | |
| renameVal, | |
| dblClick = false | |
| } = args; | |
| cy.get(nameLocator).should('be.visible'); | |
| if (dblClick) { | |
| cy.get(nameLocator).should('be.visible').dblclick(); | |
| } else { | |
| cy.get(nameLocator).should('be.visible').click(); | |
| } | |
| cy.get(textInputLocator) | |
| .should('be.visible') | |
| .clear() | |
| .type(renameVal) | |
| .should('have.value', renameVal) | |
| .blur(); | |
| this.PressEnter(); | |
| } |
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (6)
app/client/src/PluginActionEditor/components/PluginActionSettings/index.tsx (1)
21-24: Consider adding TypeScript prop types for the components.The dataTestId prop is being added without visible type definitions. Consider defining proper TypeScript interfaces for the component props.
interface ApiSettingsProps { dataTestId: string; formName: string; } interface QuerySettingsProps { dataTestId: string; docsLink: string; formName: string; }Also applies to: 27-29
app/client/cypress/support/Pages/IDE/PluginEditorToolbar.ts (1)
1-35: Add JSDoc documentation for the class and methods.The class lacks documentation explaining its purpose and usage. Consider adding JSDoc comments.
Example documentation:
/** * Represents the toolbar in the Plugin Editor. * Provides methods to interact with toolbar elements in Cypress tests. */ export class PluginEditorToolbar { /** * Creates a new instance of PluginEditorToolbar. * @param runButton - data-testid selector for the run button * @param settingsButton - data-testid selector for the settings button * @param contextMenuTrigger - data-testid selector for the context menu trigger * @param dropdownTrigger - Optional data-testid selector for the dropdown trigger */ constructor(/*...*/) {/*...*/}app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx (1)
79-79: Consider moving the default test ID to constantsThe default test ID could be moved to a constants file for better maintainability and reuse.
+// In a constants file +export const TEST_IDS = { + TOOLBAR_SETTINGS_POPOVER_TRIGGER: "t--toolbar-settings-popover-trigger", +}; - dataTestId={props.dataTestId || "t--toolbar-settings-popover-trigger"} + dataTestId={props.dataTestId || TEST_IDS.TOOLBAR_SETTINGS_POPOVER_TRIGGER}app/client/cypress/e2e/Regression/ServerSide/JsFunctionExecution/JSFunctionExecution_spec.ts (3)
Line range hint
32-38: Strengthen assertions in test casesThe test cases could benefit from stronger assertions. For example, in the dot product calculation:
Add multiple assertions to verify:
- The function result type
- The error message format
- The vector lengths
expect(asyncFunctionLength).to.equal(functionsLength); +expect(typeof result).to.equal('number'); +expect($lis).to.have.length.greaterThan(0); +expect(functionSetting.name).to.match(/^[a-zA-Z]/);
Line range hint
403-414: Use data-cy attributes for selectorsThe code uses direct selectors which is against the coding guidelines. Replace with data-* attributes.
Update selectors to use data attributes:
- jsEditor._settingsTab + '[data-cy="js-editor-settings-tab"]' - jsEditor._asyncJSFunctionSettings + '[data-cy="js-editor-async-settings"]'
Line range hint
32-38: Add API-based authentication methodsThe test suite should use API methods for authentication operations.
Add before/after hooks:
before(() => { agHelper.AddDsl("tablev1NewDsl"); LoginFromAPI(); // Add this }); after(() => { LogOutviaAPI(); // Add this });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (12)
app/client/cypress/e2e/Regression/ServerSide/JsFunctionExecution/JSFunctionExecution_spec.ts(4 hunks)app/client/cypress/support/Pages/ApiPage.ts(5 hunks)app/client/cypress/support/Pages/DataSources.ts(10 hunks)app/client/cypress/support/Pages/IDE/PluginEditorToolbar.ts(1 hunks)app/client/cypress/support/Pages/JSEditor.ts(7 hunks)app/client/cypress/support/Pages/PluginActionForm.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx(2 hunks)app/client/src/PluginActionEditor/components/PluginActionSettings/index.tsx(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorContextMenu.tsx(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx(1 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/client/src/pages/Editor/JSEditor/JSEditorContextMenu.tsx
- app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/cypress/support/Pages/ApiPage.ts
- app/client/cypress/support/Pages/JSEditor.ts
- app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx
🧰 Additional context used
📓 Path-based instructions (4)
app/client/cypress/e2e/Regression/ServerSide/JsFunctionExecution/JSFunctionExecution_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/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/PluginEditorToolbar.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/PluginActionForm.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 (10)
app/client/cypress/support/Pages/PluginActionForm.ts (3)
1-1: LGTM!
Clean and properly structured import statement.
3-9: Well-structured locators using data-testid!
Good implementation following Cypress best practices:
- Using data-testid attributes for selectors
- Clear and descriptive test IDs
- Proper encapsulation of locators as private members
11-15: LGTM!
Clean initialization of the toolbar with proper parameter passing.
app/client/src/PluginActionEditor/components/PluginActionSettings/index.tsx (1)
21-24: LGTM! Adding test IDs improves testability.
The addition of data-testid attributes aligns well with testing best practices and the PR's objective.
Also applies to: 27-29
app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx (1)
23-23: LGTM: Clean interface extension
The optional dataTestId property is well-typed and maintains backward compatibility.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx (1)
61-64: Test ID updates align with the new naming convention.
The change from t--js-editor-SETTINGS to t--js-settings-trigger follows a more descriptive naming pattern.
Let's verify the consistency of this test ID across the codebase:
Also applies to: 75-76
✅ Verification successful
Test ID update is consistently applied across the codebase
The new test ID t--js-settings-trigger is consistently used in:
- Component implementation (JSEditorToolbar.tsx)
- Test file (JSEditorToolbar.test.tsx)
- Cypress test support (JSEditor.ts)
And the old test ID t--js-editor-SETTINGS has been completely removed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining old test IDs
rg "t--js-editor-SETTINGS"
# Verify the new test ID usage
rg "t--js-settings-trigger"
Length of output: 583
app/client/cypress/support/Pages/DataSources.ts (4)
14-14: LGTM! Clean dependency injection.
The addition of PluginActionForm import and private instance follows good encapsulation practices.
Also applies to: 58-58
123-123: LGTM! Improved selector robustness.
Changed from class-based selector to data-testid, which is more resilient to styling changes.
732-732: LGTM! Consistent method usage.
Standardized usage of RenameQuery across multiple locations for better consistency.
Also applies to: 769-769, 911-911, 1188-1188, 1246-1246
Line range hint 1120-1124: LGTM! Enhanced toolbar interaction.
Refactored to use pluginActionForm.toolbar for better encapsulation of toolbar-related actions.
| <ApiSettings | ||
| dataTestId="t--action-settings-trigger" | ||
| formName={API_EDITOR_FORM_NAME} | ||
| /> |
There was a problem hiding this comment.
💡 Codebase verification
Test ID is duplicated between ApiSettings and QuerySettings components
The review comment is correct. The same dataTestId="t--action-settings-trigger" is used in both:
<ApiSettings>component<QuerySettings>component
This could lead to ambiguous test selectors. Consider using distinct identifiers like:
t--api-settings-triggerfor ApiSettingst--query-settings-triggerfor QuerySettings
🔗 Analysis chain
Verify unique test IDs for different components.
Both ApiSettings and QuerySettings use the same dataTestId="t--action-settings-trigger". Consider using unique identifiers to distinguish between them in tests.
Also applies to: 27-29
The previous command failed due to incorrect file type specification. Let's try with glob patterns instead to search both TypeScript and TSX files, and also verify the review comment's claim about QuerySettings component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of this test ID to ensure uniqueness
rg "t--action-settings-trigger" --type ts --type tsx
Length of output: 80
Script:
#!/bin/bash
# Search for test ID occurrences using ripgrep with correct file type patterns
rg "t--action-settings-trigger" -g "*.ts" -g "*.tsx"
# Also search for QuerySettings component usage to verify the review comment
rg "QuerySettings" -g "*.ts" -g "*.tsx" -A 3
Length of output: 2026
| public openDropdownTrigger() { | ||
| if(!this.dropdownTrigger) { | ||
| throw new Error("Dropdown trigger not defined"); | ||
| } | ||
|
|
||
| cy.get(this.dropdownTrigger).click({ force: true }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding retry options instead of force clicks.
Using force: true can mask underlying issues. Instead, consider using Cypress's built-in retry and timeout mechanisms.
- cy.get(this.dropdownTrigger).click({ force: true });
+ cy.get(this.dropdownTrigger, { timeout: 10000 })
+ .should('be.visible')
+ .click();📝 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 openDropdownTrigger() { | |
| if(!this.dropdownTrigger) { | |
| throw new Error("Dropdown trigger not defined"); | |
| } | |
| cy.get(this.dropdownTrigger).click({ force: true }); | |
| } | |
| public openDropdownTrigger() { | |
| if(!this.dropdownTrigger) { | |
| throw new Error("Dropdown trigger not defined"); | |
| } | |
| cy.get(this.dropdownTrigger, { timeout: 10000 }) | |
| .should('be.visible') | |
| .click(); | |
| } |
| export class PluginEditorToolbar { | ||
|
|
||
| private dropdownTrigger: string | undefined; | ||
| private runButton: string; | ||
| private settingsButton: string; | ||
| private contextMenuTrigger: string; | ||
|
|
||
| constructor(runButton: string, settingsButton: string, contextMenuTrigger: string, dropdownTrigger?: string) { | ||
| this.dropdownTrigger = dropdownTrigger; | ||
| this.runButton = runButton; | ||
| this.settingsButton = settingsButton; | ||
| this.contextMenuTrigger = contextMenuTrigger; | ||
| } |
There was a problem hiding this comment.
Use data-testid attributes for selectors.
The constructor accepts string selectors but doesn't enforce the use of data-testid attributes. According to our Cypress guidelines, we should avoid using CSS selectors or XPaths.
Consider updating the constructor to enforce data-testid selectors:
- constructor(runButton: string, settingsButton: string, contextMenuTrigger: string, dropdownTrigger?: string) {
+ constructor(
+ runButton: `[data-testid="${string}"]`,
+ settingsButton: `[data-testid="${string}"]`,
+ contextMenuTrigger: `[data-testid="${string}"]`,
+ dropdownTrigger?: `[data-testid="${string}"]`
+ ) {Committable suggestion skipped: line range outside the PR's diff.
| @@ -400,7 +400,7 @@ describe( | |||
| jsObj = jsObjName; | |||
| }); | |||
| // Switch to settings tab | |||
| agHelper.GetNClick(jsEditor._settingsTab); | |||
| jsEditor.toolbar.toggleSettings(); | |||
There was a problem hiding this comment.
Replace Sleep() calls with proper Cypress wait conditions
The code uses agHelper.Sleep() in multiple places which is against the coding guidelines. Instead, wait for specific elements or conditions.
Consider replacing with proper wait conditions:
- agHelper.Sleep(2000);
+ cy.get(jsEditor._asyncJSFunctionSettings).should('be.visible');Similar replacements should be made for other Sleep() calls.
Also applies to: 414-414, 421-421, 466-466, 473-473, 503-503
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_spec.js (3)
Line range hint
1-14: Replace cy.wait with better alternativesThe file uses multiple
cy.waitcalls which violate the coding guidelines. Consider using Cypress's built-in retry-ability and better alternatives:
- Use
cy.intercept()to wait for network requests- Use
.should()with assertions to wait for elements- Use
cy.waitUntil()for complex conditionsExample refactor:
- cy.wait(1000); + cy.get(element).should('be.visible');
Line range hint
156-159: Use data- attributes instead of CSS selectors*The test uses CSS selectors which violate the coding guidelines:
cy.get(".ads-v2-listitem span:contains('omnibarApiDatasource')")Replace with data attributes:
- cy.get(".ads-v2-listitem span:contains('omnibarApiDatasource')") + cy.get('[data-cy="datasource-item"]')
Line range hint
15-24: Improve test structure and reliabilityConsider the following improvements:
- Add more descriptive test names (e.g., "Shows and hides global search using keyboard shortcuts")
- Replace hardcoded waits with proper assertions
- Consider grouping related tests using
describeblocksExample structure:
describe("Global Search", () => { describe("Keyboard Shortcuts", () => { it("shows search modal with CMD/CTRL + K", () => { // test code }); }); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js(3 hunks)app/client/cypress/support/ApiCommands.js(3 hunks)app/client/cypress/support/Pages/DataSources.ts(10 hunks)app/client/cypress/support/Pages/PluginActionForm.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Omnibar_spec.js
- app/client/cypress/support/ApiCommands.js
- app/client/cypress/support/Pages/PluginActionForm.ts
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_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.
🔇 Additional comments (5)
app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_spec.js (1)
149-149: LGTM: Good refactor to use the helper function
The change to use _.agHelper.RenameQuery is a good improvement as it:
- Standardizes the renaming approach
- Improves maintainability
- Reduces duplicate code
app/client/cypress/support/Pages/DataSources.ts (4)
14-14: LGTM: Good use of composition pattern
The addition of PluginActionForm import and instance follows good object-oriented practices by composing functionality.
Also applies to: 58-58
123-123: LGTM: Improved selector robustness
Changed from class-based selector .t--run-query to data-testid attribute selector [data-testid='t--run-action'], which is more maintainable and less prone to breakage.
732-732: LGTM: Improved method naming consistency
Replaced generic RenameWithInPane calls with more specific rename methods like RenameQuery and RenameDatasource. This improves code readability and maintainability.
Also applies to: 769-769, 911-911, 1120-1120, 1187-1187, 1245-1245, 1577-1577
1120-1123: LGTM: Simplified toggle implementation
Good refactoring to use the pluginActionForm's toolbar for toggling settings, making the code more maintainable.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad4_Spec.ts (2)
Line range hint
9-15: Improve test organization and setup.Consider restructuring the test setup:
- Move JS object creation to a beforeEach block for consistent state
- Make test case title more descriptive of what's being verified
Line range hint
71-77: Simplify multiple identical assertions.Replace multiple similar assertions with a more concise approach using array methods.
- expect($ele.eq(0).text()).to.be.oneOf(functions); - expect($ele.eq(1).text()).to.be.oneOf(functions); - expect($ele.eq(2).text()).to.be.oneOf(functions); - expect($ele.eq(3).text()).to.be.oneOf(functions); - expect($ele.eq(4).text()).to.be.oneOf(functions); - expect($ele.eq(5).text()).to.be.oneOf(functions); - expect($ele.eq(6).text()).to.be.oneOf(functions); + cy.wrap($ele).should('have.length', 7).each(($el) => { + expect($el.text()).to.be.oneOf(functions); + });app/client/cypress/support/Pages/JSEditor.ts (1)
Line range hint
219-224: Consider more robust element interaction.The double-click interaction might be brittle. Consider using a more explicit interaction pattern with proper assertions.
Apply this pattern:
- cy.get(this._jsObjName).dblclick({ force: true }); - cy.get(this._jsObjTxt) - .clear() - .type(renameVal, { force: true }) - .should("have.value", renameVal) - .blur(); + cy.get(this._jsObjName) + .should('be.visible') + .dblclick(); + cy.get(this._jsObjTxt) + .should('be.visible') + .clear() + .type(renameVal) + .should("have.value", renameVal) + .blur();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad4_Spec.ts(2 hunks)app/client/cypress/support/Pages/JSEditor.ts(9 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad4_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/support/Pages/JSEditor.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 (2)
app/client/cypress/support/Pages/JSEditor.ts (2)
33-44: LGTM! Well-structured toolbar integration.
The new locators properly use data-testid attributes and the PluginEditorToolbar integration is clean.
56-57: Skipping comment as past review is still valid.
A previous review comment already addresses the need to replace class selectors with data-testid attributes.
| @@ -66,7 +66,7 @@ describe( | |||
| }); | |||
|
|
|||
| function VerifyFunctionDropdown(functions: string[]) { | |||
| cy.get(jsEditor._funcDropdown).click(); | |||
| cy.get(jsEditor.runFunctionSelectLocator).click(); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant click operation.
The dropdown is clicked twice (open and close) without any operations in between. Consider removing the second click as it's unnecessary.
- cy.get(jsEditor.runFunctionSelectLocator).click();
cy.get(jsEditor._funcDropdownOptions).then(function ($ele) {
// ... assertions ...
});
- cy.get(jsEditor.runFunctionSelectLocator).click();Also applies to: 79-79
| cy.get(jsEditor.runFunctionSelectLocator).click(); | ||
| cy.get(jsEditor._funcDropdownOptions).then(function ($ele) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use data-testid attributes for selectors.
Replace jsEditor.runFunctionSelectLocator and jsEditor._funcDropdownOptions with data-testid attributes for better maintainability.
Example:
// In your component
<select data-testid="js-function-dropdown">
<option data-testid="js-function-option">
// In your test
cy.get('[data-testid="js-function-dropdown"]')
cy.get('[data-testid="js-function-option"]')
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (13)
app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_CurlPOSTImport_spec.js (3)
1-2: Standardize import statementsConsider using ES6 imports consistently across the file instead of mixing require and import statements.
-const apiwidget = require("../../../../locators/apiWidgetslocator.json"); +import apiwidget from "../../../../locators/apiWidgetslocator.json";
Line range hint
22-26: Simplify promise chain and avoid nested callbacksThe nested promise chain can be simplified using async/await for better readability.
- agHelper.GetObjectName() - .then((text) => { - const someText = text; - expect(someText).to.equal(response.response.body.data.name); - }); + const apiName = await agHelper.GetObjectName(); + expect(apiName).to.equal(response.response.body.data.name);
Line range hint
1-48: Address Cypress best practices complianceSeveral improvements needed to align with the provided guidelines:
- Replace AssertNetworkExecutionSuccess (which uses cy.wait) with Cypress intercept patterns
- Use data-* attributes instead of imported locators
- Consider consolidating network request assertions
Example intercept pattern to replace cy.wait:
cy.intercept('POST', '**/api/v1/import/curl').as('curlImport'); // ... your test code ... cy.wait('@curlImport').then((interception) => { expect(interception.response.statusCode).to.eq(200); expect(interception.response.body.responseMeta.success).to.eq(true); });app/client/cypress/e2e/Sanity/Datasources/SMTPDatasource_spec.js (6)
Line range hint
42-43: Replace direct selector interactions with helper methods.The code uses direct selector interactions with
cy.get()andagHelper.ClickOutside(). Consider creating helper methods for these repetitive patterns to improve maintainability.- cy.get(queryLocators.queryFromEmail).first().type("{{From.text}}", { parseSpecialCharSequences: false }); - agHelper.ClickOutside(); + agHelper.TypeIntoField(queryLocators.queryFromEmail, "{{From.text}}", { position: "first" });Also applies to: 46-47, 50-51, 54-55, 58-59
Line range hint
71-73: Replace xpath selectors with data- attributes.*Using xpath selectors is against the coding guidelines. Replace them with data-* attributes for better maintainability.
- cy.xpath("//input[@class='bp3-input']") + cy.get("[data-cy=input-field]")Also applies to: 82-84
Line range hint
74-77: Remove explicit wait commands.The code uses
cy.wait()which is against the guidelines. Replace with Cypress's built-in retry-ability and assertions.- .closest("div").click().wait(500); - cy.wait("@postExecute") + .closest("div").click(); + cy.intercept("POST", "**/postExecute").as("postExecute"); + cy.wait("@postExecute");
Line range hint
103-104: Remove hardcoded waiting time.Using explicit
cy.wait(2000)is against best practices. Replace with proper assertions or commands that wait for specific conditions.- cy.wait(2000); + cy.get("[data-cy=element-to-wait-for]").should("be.visible");
Line range hint
106-107: Use data attributes for file upload button.Replace the text-based button selector with a data attribute.
- agHelper.ClickButton("Select Files"); + agHelper.ClickButton("[data-cy=file-upload-button]");
Line range hint
109-110: Remove eslint-disable comment.Instead of disabling the eslint rule, fix the underlying issue by using proper waiting strategies.
- //eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(500); + cy.get("[data-cy=upload-status]").should("be.visible");app/client/cypress/support/ApiCommands.js (1)
117-118: Consider consolidating duplicate validation logicThe validation logic in
CreationOfUniqueAPIcheckandCreateApiAndValidateUniqueEntityNameis identical. Consider extracting this into a shared helper function.+const validateUniqueApiName = (apiname) => { + agHelper.RenameQuery(apiname); + cy.get(".ads-v2-tooltip .ads-v2-text").should(($x) => { + expect($x).contain( + apiname.concat(" is already being used or is a restricted keyword.") + ); + }); +}; Cypress.Commands.add("CreationOfUniqueAPIcheck", (apiname) => { dataSources.NavigateToDSCreateNew(); agHelper.GetNClick(apiwidget.createapi); cy.wait("@createNewApi"); cy.get(apiwidget.resourceUrl).should("be.visible"); - agHelper.RenameQuery(apiname); - cy.get(".ads-v2-tooltip .ads-v2-text").should(($x) => { - expect($x).contain( - apiname.concat(" is already being used or is a restricted keyword.") - ); - }); + validateUniqueApiName(apiname); });Also applies to: 141-142
app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncedApps_spec.js (2)
Line range hint
1-576: Multiple violations of Cypress best practices need to be addressed.
- Replace cy.wait with better alternatives:
- cy.wait("@saveDatasource") + cy.intercept("POST", "**/api/v1/datasources").as("saveDatasource") + cy.wait("@saveDatasource")
- Use data-* attributes instead of CSS selectors:
- cy.get(".t--widget-inputwidgetv2") + cy.get("[data-cy=input-widget]")
- Use API methods for authentication:
- // Current login implementation + cy.LoginFromAPI()
Line range hint
32-576: Consider breaking down the large test suite.The test suite contains multiple long test cases that handle different aspects of Git sync. Consider breaking these into smaller, focused test files for better maintainability:
- Git branch operations
- Page operations (clone, rename, delete)
- Data binding validations
app/client/cypress/support/commands.js (1)
1054-1057: LGTM! Good refactoring to use the helper function.The change from direct DOM query to using
agHelper.GetObjectName()improves maintainability by centralizing the entity name retrieval logic.Consider adding a JSDoc comment to document the return type and purpose of this command:
+/** + * Gets the name of the currently selected entity + * @returns {Cypress.Chainable<string>} The entity name + */ Cypress.Commands.add("getEntityName", () => { let entityName = agHelper.GetObjectName(); return entityName; });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Entity_Naming_conflict_spec.js(0 hunks)app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncedApps_spec.js(1 hunks)app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_spec.js(2 hunks)app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_CurlPOSTImport_spec.js(2 hunks)app/client/cypress/e2e/Sanity/Datasources/SMTPDatasource_spec.js(1 hunks)app/client/cypress/locators/JSEditor.json(1 hunks)app/client/cypress/locators/QueryEditor.json(1 hunks)app/client/cypress/locators/apiWidgetslocator.json(1 hunks)app/client/cypress/support/ApiCommands.js(5 hunks)app/client/cypress/support/commands.js(1 hunks)
💤 Files with no reviewable changes (1)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Entity_Naming_conflict_spec.js
🚧 Files skipped from review as they are similar to previous changes (4)
- app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/GlobalSearch_spec.js
- app/client/cypress/locators/JSEditor.json
- app/client/cypress/locators/QueryEditor.json
- app/client/cypress/locators/apiWidgetslocator.json
🧰 Additional context used
📓 Path-based instructions (5)
app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncedApps_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/ApiTests/API_CurlPOSTImport_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/Sanity/Datasources/SMTPDatasource_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/ApiCommands.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/commands.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.
🔇 Additional comments (3)
app/client/cypress/e2e/Sanity/Datasources/SMTPDatasource_spec.js (1)
39-39: LGTM! Good refactoring to use the helper method.
The change from direct selector interaction to agHelper.RenameQuery() improves maintainability.
app/client/cypress/support/ApiCommands.js (1)
10-10: LGTM: Good use of centralized locators
The introduction of centralized locators through ObjectsRegistry and PluginActionForm improves maintainability.
Also applies to: 15-16
app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/GitSyncedApps_spec.js (1)
Line range hint 157-161: LGTM! Good refactoring to use helper method.
The change to use agHelper.GetObjectName() instead of direct widget name access improves code maintainability.
| cy.get(locator._queryName).should("be.visible"); | ||
| expect(response.response.body.responseMeta.success).to.eq(true); | ||
| cy.get(ApiEditor.ApiNameField) | ||
| .click() | ||
| .invoke("text") | ||
| cy.get(locator._queryName) | ||
| .invoke('text') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test reliability by following Cypress best practices
The current implementation has a few areas for improvement:
- Replace direct text assertions with more robust data-testid selectors
- Add proper API response intercepts instead of relying on implicit waits
cy.get("@createNewApi").then((response) => {
- cy.get(locator._queryName).should("be.visible");
+ cy.get(locator._queryName)
+ .should("be.visible")
+ .and("have.attr", "data-testid", "t--query-name")
expect(response.response.body.responseMeta.success).to.eq(true);
- cy.get(locator._queryName)
- .invoke('text')
- .then((text) => {
- const someText = text;
- expect(someText).to.equal(response.response.body.data.name);
- });
+ cy.get(locator._queryName)
+ .should("have.text", response.response.body.data.name);
});Also applies to: 207-207
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
app/client/src/PluginActionEditor/components/PluginActionForm/components/ApiEditor/APIEditorForm.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx(2 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/GraphQLEditor/GraphQLEditorForm.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/UQIEditorForm.tsx(1 hunks)
🔇 Additional comments (5)
app/client/src/PluginActionEditor/components/PluginActionForm/components/UQIEditor/UQIEditorForm.tsx (1)
25-30: LGTM! Verify test coverage.
The addition of the data-testid attribute follows best practices for Cypress testing.
Let's verify the test identifier usage:
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/CommonEditorForm.tsx (2)
17-17: LGTM! Well-typed prop addition.
The optional dataTestId prop is properly typed and follows TypeScript best practices.
34-34: LGTM! Proper test attribute implementation.
The data-testid attribute is correctly implemented, following React testing best practices.
Let's verify the consistency of test IDs across the codebase:
✅ Verification successful
Let me verify the test ID implementation consistency by checking the actual props and their usage.
Test ID implementation is consistent across editor forms
The data-testid prop is correctly implemented in CommonEditorForm and consistently used across different editor form implementations (GraphQL, API, UQI) with proper naming convention t--*-editor-form.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of the new test IDs
# Look for the test IDs mentioned in the AI summary
rg -A 2 "t--.*-editor-form"
Length of output: 1317
Script:
#!/bin/bash
# Check the CommonEditorForm component's props and usage
ast-grep --pattern 'interface $_ {
$$$
dataTestId?: $_
$$$
}'
# Also check how these forms are rendered
rg -A 3 "CommonEditorForm"
Length of output: 10053
app/client/src/PluginActionEditor/components/PluginActionForm/components/ApiEditor/APIEditorForm.tsx (2)
44-44: LGTM! Test ID addition looks good.
The dataTestId prop follows the established naming convention with the "t--" prefix.
44-44: Verify test ID usage in Cypress tests.
Since this is part of the new toolbar feature for Cypress, let's verify the test ID is being used correctly.
| @@ -28,6 +28,7 @@ function GraphQLEditorForm() { | |||
| <CommonEditorForm | |||
| action={action} | |||
| bodyUIComponent={<PostBodyData actionName={action.name} />} | |||
| dataTestId="t--graphql-editor-form" | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent test selector attribute naming found
APIEditorForm.tsxandGraphQLEditorForm.tsxusedataTestIdUQIEditorForm.tsxusesdata-testid
🔗 Analysis chain
LGTM! Verify test selector naming consistency.
The addition of the dataTestId prop enhances test coverage and follows the expected pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent naming pattern for test selectors across editor forms
# Expected pattern: "t--*-editor-form"
# Search for similar test selectors in editor form components
rg -g '**/*EditorForm.tsx' 't--.*-editor-form'
Length of output: 478
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx (1)
1-8: Consider adding TypeScript interfaces for better type safety.The imports look clean and focused. However, since this is a TypeScript file, we should ensure proper type safety for the plugin object from context.
Add an interface for the plugin object:
interface PluginConfig { uiComponent: UIComponentTypes; // Add other required properties }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx(1 hunks)
🔇 Additional comments (2)
app/client/src/PluginActionEditor/components/PluginActionForm/PluginActionForm.tsx (2)
Line range hint 15-28: LGTM! Clean and maintainable conditional rendering.
The component's return statement is well-structured with clear conditions for rendering different editor forms. The use of UIComponentTypes enum provides good type safety for the conditionals.
11-13: Verify the removal of error handling.
The removal of editorConfig validation might lead to runtime errors if the plugin object is undefined or malformed.
…o action-redesign/toolbar-flag-lift
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/cypress/support/Pages/DataSources.ts (1)
Line range hint
4-4: Add unit tests as indicated by TODO commentThe TODO comment indicates missing test coverage for this module. Unit tests are essential for validating the behavior of mathematical operations.
Would you like me to help create a test suite for these functions?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts(4 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js(0 hunks)app/client/cypress/support/Objects/CommonLocators.ts(2 hunks)app/client/cypress/support/Pages/DataSources.ts(12 hunks)
💤 Files with no reviewable changes (1)
- app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts
- app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts
- app/client/cypress/support/Objects/CommonLocators.ts
🧰 Additional context used
📓 Path-based instructions (1)
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.
## Description This PR enabled IDE toolbar for cypress. Fixes appsmithorg#37217 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12115693818> > Commit: f2af870 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12115693818&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 02 Dec 2024 12:19:38 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new feature flag `release_actions_redesign_enabled` to enhance user experience. - **Bug Fixes** - Updated method calls in various test cases to improve consistency and reliability in interactions, specifically renaming methods related to data sources and queries. - Improved error handling and validation in tests for MongoDB query functionalities. - **Documentation** - Enhanced test coverage and assertions for various functionalities, including API actions, Google Sheets queries, and JavaScript function execution. - **Style** - Updated CSS selectors for improved consistency and maintainability across components and test cases. - **Tests** - Refactored multiple test cases to utilize new helper methods, improving code clarity and reducing direct DOM manipulation. - Enhanced visual tests for JSEditor and improved interaction with the run button. - Added new assertions and enhanced the structure of tests for various components, including API response handling and widget interactions. - Streamlined interaction with the settings toolbar in various test cases by encapsulating functionality within the `PluginActionForm` class. - Introduced new `data-testid` attributes across various components to enhance testability. - **Chores** - Removed unused imports and streamlined method calls for better performance and readability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
This PR enabled IDE toolbar for cypress.
Fixes #37217
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12115693818
Commit: f2af870
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 02 Dec 2024 12:19:38 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
release_actions_redesign_enabledto enhance user experience.PluginActionFormclass.data-testidattributes across various components to enhance testability.