chore: removed old flags for airgap instances #36609
Conversation
AmanAgarwal041
requested review from
ApekshaBhosale,
sagar-qa007 and
a team
as code owners
WalkthroughThe changes in this pull request enhance the Cypress end-to-end tests for the Community Issues page and the Table widget. Key improvements include expanded checks for pagination, row selection, and validation of search functionality. Additionally, comprehensive testing of filtering logic and CRUD operations related to issues has been refined. The modifications aim to bolster the robustness of the test suite by ensuring thorough validation of user interactions and data integrity. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Integrations Pod General
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11102991010. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (48)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js (2)
Line range hint
14-26: Class, let's improve our test reliability by avoiding cy.wait.In our first test case, we're using
cy.wait("@updateLayout"). While this works, it's like watching the clock instead of checking if your experiment is actually complete. Let's consider using Cypress's built-in retry-ability for a more robust approach. For example:cy.get('@updateLayout').its('response.body.responseMeta.status').should('eq', 200);This way, we're actually checking for the condition we care about, rather than just waiting for a fixed amount of time.
Line range hint
14-45: Let's make our test descriptions more informative, students!Our current test case descriptions are a bit like vague essay titles. They tell us what we're testing, but not the specific scenario or expected outcome. Let's make them more descriptive. For example:
- "Input widget displays default value from table widget v2 correctly"
- "Input widget updates correctly when a different row is selected in the table"
Remember, good test descriptions are like good essay titles - they should give a clear idea of what's being tested and what the expected outcome is.
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js (2)
Line range hint
41-43: Let's clean up our work, class!The
cy.log()statement here is like writing notes in the margin of your test book - it's helpful while you're working, but not necessary for the final product. We can remove this line to keep our test clean and focused:cy.readTableV2dataPublish("0", "0").then((tabData) => { const tabValue = tabData; expect(tabValue).to.be.equal("2736212"); // Remove the following line // cy.log("the value is" + tabValue); cy.get(publish.inputWidget + " " + "input") .first() .invoke("attr", "value") .should("contain", tabValue); });
Line range hint
43-47: Let's make our test more robust, like a well-built science project!Instead of using complex selectors, we should use data-* attributes. This makes our tests more resilient to changes in the UI. Here's how we could improve this part:
cy.get('[data-cy="input-widget"]') .first() .invoke("attr", "value") .should("contain", tabValue);Remember to add the corresponding data-cy attribute to your input widget in the actual application code.
app/client/cypress/limited-tests.txt (1)
Line range hint
26-29: Let's improve our documentation, shall we?Class, I appreciate the inclusion of instructions for running all specs and the note about ci-test-limit. However, to make this file even more user-friendly, consider the following suggestions:
- Add a brief explanation of when and why one might want to run all specs.
- Provide more context about the ci-test-limit command. What does it do, and when should it be used?
- Consider adding a section with examples of how to use this file in different scenarios.
Remember, clear documentation is like a good lesson plan – it helps everyone understand and follow along more easily!
Here's a suggested improvement:
# For running all specs - uncomment below: #cypress/e2e/**/**/* -#ci-test-limit uses this file to run minimum of specs. Do not run entire suite with this command. +# Note: The ci-test-limit command uses this file to run a minimum set of specs. +# It should not be used to run the entire test suite. + +# Usage examples: +# 1. To run specific tests: Use the lists above +# 2. To run all tests: Uncomment the line above +# 3. For CI with limited tests: Use the ci-test-limit commandapp/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js (2)
Line range hint
33-39: Well done, class! Let's review the changes.I'm pleased to see the expansion of the pagination section and the verification of pagination functionality. However, we can make a small improvement to follow best practices.
Let's replace the static wait with a more robust assertion:
- agHelper.WaitUntilToastDisappear("done"); + cy.get(commonlocators.toastMsg).should('not.exist');This change will make our test more reliable and less prone to timing issues. Remember, in testing, we always want to avoid unnecessary waits!
Line range hint
41-58: Excellent work on addressing Bug #22477!Your test case effectively verifies the behavior of the next page button when the last page is reached. I'm particularly impressed with your attention to detail in checking both the disabled state and the absence of a toast message.
To make our test even more robust, let's add an assertion to verify that we're on the last page:
agHelper.GetNClick(table._nextPage("v2")); agHelper.AssertAttribute(table._nextPage("v2"), "disabled", "disabled"); agHelper.AssertElementAbsence(locators._toastMsg); // Add this line cy.get('.t--widget-tablewidgetv2 .page-item').last().should('contain', '2');This additional assertion will ensure that we're truly on the last page when the next button is disabled. Remember, in testing, we can never be too thorough!
app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js (2)
36-36: Excellent addition to our test, students!You've shown great attention to detail by expanding the row selection section before proceeding with the test. This ensures our test won't fail simply because a section is collapsed. Gold star for you!
However, let's make it even better. Can anyone tell me how we might improve this further? That's right! We should add a small wait after expanding the section to ensure it's fully loaded. Let's modify it like this:
table.ExpandIfCollapsedSection("rowselection"); cy.wait(500); // Short wait to ensure section is fully expandedThis will make our test even more robust. Remember, in testing as in life, patience is a virtue!
Line range hint
1-78: Class, let's review our work as a whole.You've done a commendable job creating tests for our input and table widget interactions. Your coverage of default values, sorting, and column identification shows good thinking. However, we have some areas for improvement:
I notice we're using
cy.wait()in several places. Remember our lesson on avoiding explicit waits? Instead, let's use Cypress's built-in retry-ability. For example, replace:cy.wait(1000); cy.readTableV2dataPublish("0", "0")with:
cy.readTableV2dataPublish("0", "0", { timeout: 10000 })We're using attribute selectors like
publish.inputWidget + " " + "input". Let's improve this by using data attributes. Modify your app code to include data attributes, then update the selectors:cy.get('[data-cy="input-widget"]')Lastly, let's enhance our assertions. Instead of separate
expectandcy.logstatements, combine them:cy.readTableV2dataPublish("0", "0").should((tabValue) => { expect(tabValue).to.equal("6788734"); cy.get('[data-cy="input-widget"]').should('have.value', tabValue); });By making these changes, we'll have more robust and maintainable tests. Remember, good testing is the foundation of reliable software!
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js (2)
Line range hint
70-70: Let's remove this wait, shall we?Class, I've spotted a
cy.wait(1000)on line 70. Remember our golden rule: avoid usingcy.waitin our Cypress tests. It's like watching the clock during an exam - it doesn't help and only makes time pass slower!Instead, let's think about what we're waiting for. Are we waiting for an element to appear? A network request to complete? Let's use Cypress's built-in retry-ability and wait for the specific condition we need.
For example, if we're waiting for rows to be selected, we could do something like:
cy.get('.selected-row').should('have.length', 3)This way, we're telling Cypress exactly what to expect, rather than arbitrarily waiting. It's like raising your hand when you're done with a question, instead of just waiting for a set amount of time!
Line range hint
33-35: Time to refine our selectors, class!I've noticed we're using CSS selectors in our
cy.get()calls. While these work, they're not the best practice according to our coding guidelines. It's like using a nickname instead of a student's proper name - it might work, but it's not always clear or consistent!Let's improve these by using
data-*attributes instead. For example, instead of:cy.get(`${widgetsPage.textWidget} .bp3-ui-text`)We could use something like:
cy.get('[data-testid="text-widget-content"]')This makes our tests more resilient to changes in the CSS and clearer to read. It's like using proper nouns in your essays - much more precise!
Remember to update the application code to include these
data-*attributes where needed. It's a bit more work now, but it'll make our tests much easier to maintain in the future!Also applies to: 44-46, 54-56, 65-67, 75-77
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js (2)
Line range hint
41-53: Impressive work on this test case, students!Your test demonstrates a thorough understanding of event handling and user feedback verification. The inclusion of toast message checks is particularly commendable.
However, I have a small suggestion to enhance your test:
Consider adding a negative test case to ensure that the toast message doesn't appear when a row is deselected. This would provide more comprehensive coverage of the
onRowSelectedbehavior.Here's an example of how you might implement this:
// Deselect the first row cy.get(".t--table-multiselect").first().click({ force: true }); // Ensure the toast message doesn't appear cy.get(commonlocators.toastmsg).should("not.exist");
Line range hint
55-73: Excellent work on this final test case, class!Your test thoroughly covers the behavior of default selected rows and their interaction with the header cell. You've maintained our best practices throughout, which is commendable.
To further enhance this test, I have a small suggestion:
Consider adding an assertion to verify the state of the header cell after selecting and deselecting all rows. This would ensure that the header cell's state is consistent with the row selection state.
Here's an example of how you might implement this:
// After selecting all rows cy.get(".t--table-multiselect-header").first().should("have.class", "selected"); // After deselecting all rows cy.get(".t--table-multiselect-header").first().should("not.have.class", "selected");app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js (1)
Line range hint
116-137: Well done, class! This new test case is a valuable addition.The new test case for verifying default row selection functionality is well-structured and comprehensive. It covers both the editor and deployed states of the application, which is excellent practice.
However, let's make a small improvement to enhance our test's reliability:
Consider replacing the hard-coded wait with a more robust solution:
-cy.wait("@updateLayout"); +cy.intercept("POST", "/api/v1/layouts/update").as("updateLayout"); +cy.wait("@updateLayout").its("response.statusCode").should("equal", 200);This change will make our test more resilient to timing issues and ensure that the layout update has completed successfully before proceeding.
app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (2)
Line range hint
1-24: Excellent work on improving thecomponentDidUpdatemethod, students!You've made some smart changes here:
- Adding a check for string or finite values in
defaultOptionValueimproves our type handling.- Using
xorWithandequalfor change detection is a more robust approach.These changes will make our component more reliable and easier to maintain. However, I have a small suggestion to make the code even clearer:
Consider extracting the change detection logic into a separate helper function. This would make the
componentDidUpdatemethod easier to read and test. Here's an example:private hasDefaultOptionValueChanged( currentValue: string[] | OptionValue[], prevValue: string[] | OptionValue[] ): boolean { const isStringArray = currentValue.some( (value: any) => isString(value) || isFinite(value) ); return isStringArray ? xorWith(currentValue as string[], prevValue as string[], equal).length > 0 : xorWith(currentValue as OptionValue[], prevValue as OptionValue[], equal).length > 0; }Then in
componentDidUpdate, you could simply call:if (this.hasDefaultOptionValueChanged(this.props.defaultOptionValue, prevProps.defaultOptionValue) && this.props.isDirty) { this.props.updateWidgetMetaProperty("isDirty", false); }This would make your code more modular and easier to understand at a glance. Keep up the good work!
Line range hint
1-13: A gold star for adding thesetSelectedOptionssetter!This is a valuable addition to our
MultiSelectWidget. It allows other parts of the application to programmatically set the selected options, which is very useful for creating dynamic user interfaces.Great job on enhancing the widget's functionality!
To make sure everyone understands how to use this new setter, let's add some documentation. Consider adding a comment above the
setSelectedOptionsconfiguration like this:/** * Sets the selected options of the MultiSelect widget. * @param {array} options - An array of option values to be set as selected. */ setSelectedOptions: { path: "defaultOptionValue", type: "array", accessor: "selectedOptionValues", },This will help other developers understand how to use this setter effectively. Remember, clear documentation is like a good lesson plan - it helps everyone learn and use the code correctly!
app/client/src/widgets/SelectWidget/widget/index.tsx (2)
Line range hint
165-171: Excellent simplification of our property updates!Students, observe how we've streamlined our
getPropertyUpdatesForQueryBindingmethod. By removing the conditional check and always updating thedynamicPropertyPathListwith 'sourceData', we've made our code more consistent and easier to understand.However, let's consider a small improvement in our code style:
Consider using the spread operator for clarity:
-dynamicPropertyPathList.push({ key: "sourceData" }); +dynamicPropertyPathList = [...dynamicPropertyPathList, { key: "sourceData" }];This approach creates a new array, which can be beneficial in certain React contexts and makes the addition of the new item more explicit.
Line range hint
1012-1023: Bravo on optimizing our filter change handling!Class, let's examine the improvements in our
onFilterChangemethod. We're now updating the widget's meta property 'filterText' and intelligently triggering server-side filtering only when necessary. This is an excellent optimization that will improve our application's performance.To further enhance this method, consider the following suggestion:
We could add a debounce mechanism to prevent rapid successive calls to
onFilterUpdate. This would be particularly beneficial for performance in cases of fast typing. Here's how you might implement it:import { debounce } from 'lodash'; // At the class level private debouncedFilterUpdate = debounce((value: string) => { if (this.props.onFilterUpdate && this.props.serverSideFiltering) { super.executeAction({ triggerPropertyName: "onFilterUpdate", dynamicString: this.props.onFilterUpdate, event: { type: EventType.ON_FILTER_UPDATE, }, }); } }, 300); // 300ms delay // In the onFilterChange method onFilterChange = (value: string) => { this.props.updateWidgetMetaProperty("filterText", value); this.debouncedFilterUpdate(value); };This change would make our widget even more efficient and responsive. What do you think about this enhancement?
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js (1)
Line range hint
29-29: Let's avoid usingcy.wait; use assertions or built-in retries insteadTo make our tests more efficient and less flaky, we should avoid using
cy.wait. Instead, we can use assertions or Cypress's built-in retry mechanisms to wait for elements or conditions.You can replace the
cy.wait(1000);with an assertion that checks for the desired state:-// Disable Client Search -_.agHelper.CheckUncheck(commonlocators.clientSideSearch, false); -cy.wait(1000); //wait & then read the table value +// Disable Client Search and wait for the table to update +_.agHelper.CheckUncheck(commonlocators.clientSideSearch, false); +cy.readTableV2dataPublish("0", "0").should("exist");This way, the test waits dynamically for the table to update without a fixed delay.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js (4)
Line range hint
130-131: Avoid Usingcy.waitStatementsIt's important to understand that using
cy.waitwith static wait times can lead to flaky tests. Instead, you should use assertions or commands that wait for certain conditions to be met, ensuring your tests are reliable and stable.Please remove the
cy.wait(100);statement and rely on an assertion to verify that the necessary conditions are met before proceeding. Here's how you can adjust your code:- cy.wait(100); agHelper.ValidateToastMessage("This is a test message");The
ValidateToastMessagefunction should handle the waiting internally by checking for the toast message's appearance.
Line range hint
76-77: Use Data Attributes Instead of CSS SelectorsWhen selecting elements for interaction, it's best to use
data-*attributes. This practice enhances the robustness of your tests by reducing their susceptibility to UI changes that might affect class names or structures.Please update your selectors to use
data-*attributes or locator variables. For example:- cy.get(".t--propertypane").contains("Style").click({ force: true }); + cy.get("[data-testid='t--propertypane']").contains("Style").click({ force: true });This change will make your test more resilient and easier to maintain.
Line range hint
78-80: Avoid Complex CSS Selectors; Utilize Locator VariablesUsing long or complex CSS selectors can make your tests fragile and harder to read. It's advisable to use locator variables, which are predefined selectors that improve code readability and maintainability.
Please modify your code to use locator variables from your locators file. For example:
- cy.get(".t--property-control-horizontalalignment .ads-v2-segmented-control__segments-container") + cy.get(commonLocators.horizontalAlignmentSegment)This adjustment will help ensure that your tests are less affected by changes in the UI structure.
Line range hint
125-126: Enhance Test Reliability Without Static WaitsUsing static waits like
cy.wait(100);doesn't guarantee that the application is ready for the next action, which can cause intermittent test failures.Instead of a static wait, consider using an assertion to confirm that the expected element or message is present before proceeding. For example:
- cy.wait(100); agHelper.ValidateToastMessage("This is a test message"); + agHelper.ValidateToastMessage("This is a test message");The
ValidateToastMessagefunction should inherently wait for the toast message to appear, so the explicit wait is unnecessary.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts (1)
Line range hint
38-43: Avoid hardcoded strings in assertionsTo make your tests more robust and easier to maintain, it's advisable to avoid hardcoding strings in your assertions. Instead, define expected values as constants or variables. This approach improves clarity and simplifies updates.
Consider applying this change:
+const expectedUserName = "Lindsay Ferguson"; table.ReadTableRowColumnData(1, 3, "v2").then((cellData) => { - expect(cellData).to.eq("Lindsay Ferguson"); + expect(cellData).to.eq(expectedUserName);app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js (12)
Line range hint
21-28: Avoid Using Plain Strings for SelectorsUsing class selectors like
.t--property-control-allowaddingarowis discouraged. Instead, utilize data attributes and locator variables for consistency and reliability.Refactor the selectors as follows:
-cy.get(".t--property-control-allowaddingarow").should("exist"); -cy.get(".t--property-control-allowaddingarow input").should("exist"); -cy.get(".t--add-new-row").should("not.exist"); -propPane.TogglePropertyState("Allow adding a row", "Off", null); -cy.get(".t--add-new-row").should("not.exist"); -cy.get(".t--property-control-onsave").should("not.exist"); -cy.get(".t--property-control-ondiscard").should("not.exist"); -cy.get(".t--property-control-defaultvalues").should("not.exist"); +cy.get(propPane._allowAddARow).should("exist"); +cy.get(propPane._allowAddARowToggle).should("exist"); +cy.get(table._addNewRow).should("not.exist"); +propPane.TogglePropertyState("Allow adding a row", "Off"); +cy.get(table._addNewRow).should("not.exist"); +cy.get(propPane._onSave).should("not.exist"); +cy.get(propPane._onDiscard).should("not.exist"); +cy.get(propPane._defaultValues).should("not.exist");Ensure that the locator variables like
propPane._allowAddARowandtable._addNewRoware defined in your locators file.
Line range hint
34-39: Avoid Disabling Tests Temporarily with CodeInstead of disabling the
add new rowlink by entering inline edit mode, consider programmatically setting the state or mocking the behavior. This avoids potential flakiness in tests.
Line range hint
47-49: Use Data Attributes for SelectorsSelecting elements using class names can be unreliable. Replace:
cy.get(".tableWrap .new-row").should("exist");With a selector that uses data attributes or locator variables.
Refactor as:
-cy.get(".tableWrap .new-row").should("exist"); +cy.get(table._newRow).should("exist");Define
table._newRowin your locators file.
Line range hint
54-56: Use Locator Variables Instead of Plain StringsIn the line:
cy.updateCodeInput( ".t--property-control-defaultvalues", "{{{step: 'newStepCell'}}}", );Replace the plain string selector with a locator variable.
Refactor as:
-cy.updateCodeInput( - ".t--property-control-defaultvalues", - "{{{step: 'newStepCell'}}}", -); +cy.updateCodeInput( + propPane._defaultValues, + "{{{step: 'newStepCell'}}}", +);Ensure
propPane._defaultValuesis defined.
Line range hint
62-65: Avoid Using Plain Strings in AssertionsWhen asserting table data, use locator variables and avoid hardcoded indices.
Refactor as:
-cy.readTableV2data(0, 0).then((val) => { +cy.readTableV2data(rowIndex, colIndex).then((val) => { expect(val).to.equal("newStepCell"); });Define
rowIndexandcolIndexas needed.
Line range hint
71-88: Encapsulate Repeated Selectors Using Locator VariablesThe repeated use of selectors like
.t--widget-tablewidgetv2 .t--search-inputshould be replaced with locator variables to improve test maintainability.Refactor as:
-cy.get(".t--widget-tablewidgetv2 .t--search-input").should("exist"); +cy.get(table._searchInput).should("exist");Repeat this change for all similar selectors, defining appropriate locator variables in your locators file.
Line range hint
92-109: Avoid Usingcy.getwith Complex SelectorsUsing complex CSS selectors can make tests brittle.
Refactor selectors to use data attributes or locator variables. For example:
-cy.get(".t--discard-new-row").click({ force: true }); +cy.get(table._discardNewRow).click();Define
table._discardNewRowaccordingly.
Line range hint
115-124: Use Locator Variables for Dynamic SelectorsWhen constructing selectors with variables, ensure you're using locator variables and data attributes.
Refactor:
-cy.get( - `[data-colindex=0][data-rowindex=0] .t--inlined-cell-editor`, -).should("exist"); +cy.get(table._inlinedCellEditor(0, 0)).should("exist");Define
table._inlinedCellEditoras a function that returns the selector string.
Line range hint
129-136: Avoid Hardcoding Values in TestsInstead of hardcoding indices and values, use constants or variables for better readability and flexibility.
Consider defining variables for row and column indices, and expected values.
Line range hint
142-146: Use Proper Widget Selection MethodsWhen adding widgets, use the appropriate helper functions to interact with the canvas, ensuring consistency across tests.
Refactor:
-cy.dragAndDropToCanvas("textwidget", { x: 300, y: 600 }); +entityExplorer.DragDropWidgetNVerify("textwidget", 300, 600);Ensure
entityExploreris correctly imported and used.
Line range hint
161-168: Use Exact Property Labels for TogglingWhen toggling properties, ensure the labels match exactly to avoid errors due to typos or formatting differences.
Verify that the property labels used in
propPane.TogglePropertyStatematch those in the UI.
Line range hint
172-174: Avoid Usingcy.waitin TestsThe explicit wait
cy.wait(1000);should be replaced with proper synchronization.Refactor as:
-cy.wait(1000); +agHelper.WaitUntilToastDisappear();Or wait for a specific element to appear or disappear.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (5)
Line range hint
141-141: Avoid usingagHelper.Sleep()to ensure test reliabilityUsing
agHelper.Sleep()can make our tests less reliable because it introduces arbitrary delays. Instead, let's wait for specific elements or conditions to ensure our application is ready before proceeding.
Line range hint
165-165: Avoid usingagHelper.Sleep()to enhance test efficiencyIncluding
agHelper.Sleep()can lead to inefficient and flaky tests. Let's use explicit waits or assertions to synchronize our test execution with the application's state.
Line range hint
182-183: Refrain from usingcy.wait()with fixed timeoutsRelying on
cy.wait(500)can cause our tests to be flaky. It's better to wait for specific elements or events. Let's find a more reliable synchronization method to improve test stability.
Line range hint
183-184: Usedata-*attributes and locator variables for selectorsIn lines 183-184, using complex CSS selectors like
".t--table-filter:nth-child(2) .t--table-filter-value-input input[type=text]"can make our tests brittle. Let's utilizedata-*attributes and locator variables to enhance the robustness and maintainability of our tests.
Line range hint
240-240: Avoid using fixed timeouts like.wait(500)Using
.wait(500)after typing can make tests flaky. Instead, consider waiting for specific elements or conditions to ensure the application is ready before proceeding.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js (2)
Line range hint
69-70: Use Data- Attributes Instead of CSS Selectors*When selecting elements in your tests, it's important to use
data-*attributes rather than CSS selectors. Using selectors likecy.contains('[role="columnheader"] .draggable-header', "userName")can make your tests brittle, as any change in the DOM structure or classes can break them. By usingdata-*attributes, you ensure that your tests are more stable and maintainable.Consider updating your code as follows:
- cy.contains('[role="columnheader"] .draggable-header', "userName") + cy.get("[data-colindex='3'] .draggable-header").click({ force: true });Ensure that you have the appropriate
data-*attributes added to your elements in the application code to support this change.
Line range hint
71-71: Avoid Using Fixed Delays Likecy.wait(1000);Relying on fixed delays such as
cy.wait(1000);can lead to unreliable tests. The application might take more or less time to reach the desired state, causing tests to fail intermittently. Instead, it's better to wait for a specific condition or element to appear before proceeding.Here's how you can improve your test:
- cy.wait(1000); + cy.get(".t--table-widget").should("be.visible");Replace the selector with the one that best indicates the table has finished sorting.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts (4)
Line range hint
41-41: Replace arbitrary waits with condition-based waitsAt line 41:
agHelper.GetNClick(table._filterInputValue, 0).type("19").wait(500);Using
.wait(500);introduces an unnecessary delay and can cause the test to be unreliable. It's better to wait for the expected change in the table data.You can remove the
.wait(500);and add an assertion or wait for a specific element to confirm the filter has applied:agHelper.GetNClick(table._filterInputValue, 0).type("19"); table.ReadTableRowColumnData(0, 3, "v2").then(($cellData) => { expect($cellData).to.eq("Tobias Funke"); });
Line range hint
96-96: Avoid using.wait(500);—wait for the expected condition insteadIn line 96, after modifying the filter input:
.wait(500);This fixed wait might not align with the actual time needed for the table to update.
Replace
.wait(500);with a check for the updated table data:table.ReadTableRowColumnData(0, 3, "v2").then(($cellData) => { expect($cellData).to.eq("Ryan Holmes"); });
Line range hint
147-147: Use dynamic waits rather than fixed delaysAt line 147:
.wait(500);Fixed waits can make tests slower and less reliable. It's better to wait for the action to complete by checking for a specific outcome.
After typing in the new filter value, add an assertion to wait for the table to reflect the changes:
table.ReadTableRowColumnData(0, 3, "v2").then(($cellData) => { expect($cellData).to.eq("Ryan Holmes"); });
Line range hint
26-26: Remember to avoid usingagHelper.Sleep();in testsAt line 26:
agHelper.Sleep(2000); //table to filter & records to disappearAgain, using fixed sleep periods isn't recommended. We should wait for a specific condition instead.
Use a method like
table.WaitForTableEmpty("v2");or check for the expected data.app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts (1)
Line range hint
43-43: Avoid usingagHelper.Sleep(1000)to improve test reliability.On line 43, the use of
agHelper.Sleep(1000)is discouraged. Relying on static waits can lead to flaky tests due to timing inconsistencies. Instead, consider using dynamic waits that respond to application state changes.Replace the sleep with a proper wait method, such as waiting for an element to be visible or a network call to complete. This ensures your test proceeds only when the application is ready, enhancing stability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (34)
- app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSlashCommand_spec.ts (0 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/JSEnabledByDefaultExperiment_spec.ts (0 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js (3 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts (1 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
- app/client/cypress/support/Objects/FeatureFlags.ts (0 hunks)
- app/client/cypress/support/Pages/Table.ts (1 hunks)
- app/client/cypress/support/e2e.js (0 hunks)
- app/client/src/ce/entities/FeatureFlag.ts (0 hunks)
- app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts (0 hunks)
- app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (0 hunks)
- app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx (1 hunks)
- app/client/src/constants/WalkthroughConstants.ts (0 hunks)
- app/client/src/pages/Editor/DatasourceInfo/QueryTemplates.tsx (1 hunks)
- app/client/src/pages/Editor/PropertyPane/PropertyControlsGenerator.tsx (1 hunks)
- app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (1 hunks)
- app/client/src/widgets/SelectWidget/widget/index.tsx (1 hunks)
- app/client/src/widgets/TableWidgetV2/widget/index.tsx (1 hunks)
🔥 Files not summarized due to errors (1)
- app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (8)
- app/client/cypress/e2e/Regression/ClientSide/Autocomplete/PropertyPaneSlashCommand_spec.ts
- app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/JSEnabledByDefaultExperiment_spec.ts
- app/client/cypress/support/Objects/FeatureFlags.ts
- app/client/cypress/support/e2e.js
- app/client/src/ce/entities/FeatureFlag.ts
- app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts
- app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts
- app/client/src/constants/WalkthroughConstants.ts
🧰 Additional context used
📓 Path-based instructions (20)
app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_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/Binding/TableV2Widget_selectedRow_Input_widget_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/Binding/TableV2_ClientSide_Search_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/Binding/TableV2_DefaultSearch_Input_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/Binding/TableV2_Widget_API_Pagination_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/Binding/TextTableV2_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/AddNewRow1_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/Edge_case_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/TableV2Filter1_1_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/Widgets/TableV2/TableV2Filter1_2_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/Widgets/TableV2/TableV2Filter2_1_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/Widgets/TableV2/TableV2Filter2_2_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/Widgets/TableV2/TableV2_DisplayText_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/Widgets/TableV2/TableV2_MultiRowSelect_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/TableV2_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/checkboxCell_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/switchCell_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/server_side_filtering_spec_1.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/limited-tests.txt (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/Table.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.
📓 Learnings (1)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js (1)
Learnt from: Aishwarya-U-R PR: appsmithorg/appsmith#29405 File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41 Timestamp: 2023-12-08T08:59:53.560Z Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
🔇 Additional comments (26)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js (1)
33-33: Well done, class! This addition improves our test reliability.The new line
_.table.ExpandIfCollapsedSection("rowselection");is a smart move. It ensures that the row selection section is expanded before we interact with it. This is like making sure your textbook is open to the right page before you start reading. It helps prevent potential errors and makes our test more robust.app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js (1)
30-30: Well done, class! This addition is a gold star moment.Adding this line to expand the search and filters section is like opening your textbook before starting your homework. It ensures we can see and interact with all the important parts of our Table widget before we begin our test. This is a smart move that will help our test run smoothly every time.
app/client/cypress/limited-tests.txt (2)
1-24: Well done on maintaining a clear and organized file structure, class!The file is neatly organized with clear sections for different categories of tests. This makes it easy for everyone to understand and maintain. Keep up the good work!
2-24: Class, let's discuss the importance of these new test specifications!I'm pleased to see the addition of new test specifications, especially those related to the TableV2 widget. These tests will help ensure the reliability and functionality of this important component.
However, I have a few questions for you to ponder:
- For the tests under "Not fixed", what steps are being taken to address and resolve these issues?
- For the "Fixed" tests, have you considered adding comments to explain what was fixed in each case?
- Have you thought about grouping similar tests together for better organization?
Remember, thorough testing is key to maintaining a robust application. Keep up the excellent work!
To ensure all these test files exist, let's run a quick check:
app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js (1)
10-10: Well done, class! You've added a new import.I'm pleased to see you've imported the
tableobject from ObjectsCore. This addition shows good foresight, as it will allow us to use table-related operations in our test cases. Remember, children, organized imports make for tidy code!app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js (2)
29-29: Good addition, class! Let's expand on this.The new line
table.ExpandIfCollapsedSection("rowselection");is a smart move. It ensures that the "rowselection" section is expanded before we proceed with our test. This is like making sure all your textbooks are open to the right page before we start our lesson!However, let's take a moment to reflect on our coding guidelines:
- We're using a locator variable (
table.ExpandIfCollapsedSection) instead of a plain string. Good job!- We're not using any
cy.wait,cy.pause, oragHelper.sleep(). Excellent!- We're avoiding XPaths and CSS selectors. Well done!
Remember, class, always strive for clarity and consistency in your code. It's like keeping your desk tidy - it makes everything easier to find and understand!
Line range hint
1-79: Class, let's sum up our lesson for today!Overall, this test file for the Table widget v2 edge cases is well-structured and covers important scenarios. The new addition on line 29 is a good improvement to ensure test reliability.
However, we have a few areas where we can make our code even better:
- Remove the use of
cy.wait()and replace it with more specific wait conditions.- Update our selectors to use
data-*attributes instead of CSS selectors.- Consider using API calls for login/logout/signup if authentication is needed in these tests.
Remember, writing good tests is like writing a good essay. We need to be clear, precise, and follow our guidelines. Keep up the good work, and let's strive to make our test suite the best it can be!
Don't forget to review these changes with your study group (team) and make the necessary updates. Class dismissed!
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js (4)
Line range hint
15-22: Well done, class! This test case is a shining example of good practices.I'm pleased to see that you've followed our Cypress best practices. You've used the recommended
data-*attributes for selectors and avoided any unnecessary waits. Keep up the excellent work!
Line range hint
24-29: Excellent job on this test case, students!Your attention to detail is commendable. You've correctly used the
data-*attributes for selectors and maintained a clean, efficient test structure. This is precisely what we aim for in our Cypress tests.
Line range hint
31-39: Another stellar example, class!Your test case demonstrates a clear understanding of the Table Widget V2's behavior. You've maintained our best practices by using
data-*attributes and avoiding any unnecessary waits. Your attention to the header cell's state change is particularly noteworthy.
Line range hint
1-73: Excellent work on this test suite, class!Your test cases for the Table Widget V2 row multi-select validation are comprehensive and well-structured. You've consistently followed our Cypress best practices, using appropriate selectors and avoiding unnecessary waits.
To further elevate your work, consider implementing the suggestions I've provided for the fourth and fifth test cases. These additions will enhance the robustness of your tests and provide even more comprehensive coverage.
Remember, continuous improvement is key in software testing. Keep up the great work!
app/client/src/pages/Editor/PropertyPane/PropertyControlsGenerator.tsx (1)
137-137: Class, let's examine this interesting change!Well done on simplifying the code by removing the feature flag dependency. This change will ensure consistent behavior across all instances.
However, I'd like you to raise your hand and answer these questions:
- Is this change intentional and approved by the product team?
- Have we considered how this might affect our users' experience with the property pane?
To make sure we're all on the same page, let's add a comment explaining this change:
+ // Note: All sections now default to expanded state as per the removal of the feature flag true,Remember, clear documentation is key to maintaining our codebase!
app/client/src/pages/Editor/DatasourceInfo/QueryTemplates.tsx (4)
Line range hint
1-25: Good job cleaning up the imports, class!I see you've removed the import for WalkthroughContext. That's excellent housekeeping! Remember, keeping our imports tidy is like keeping our desks clean - it helps us work more efficiently.
Line range hint
172-214: Let's wrap up our review of the QueryTemplates component, class!You've done a commendable job in removing the walkthrough-related code from this component. It's like decluttering our classroom - we've removed what we no longer need, but kept all the important learning materials.
I'm pleased to see that the core functionality of creating and updating query actions remains intact. The rendering of template options in the menu items is unchanged, which is excellent.
However, I want you all to think about this: How might removing the walkthrough feature affect new users? Are there other ways we can guide them through using these query templates? Perhaps we could add some helpful tooltips or improve our documentation?
Remember, good code is not just about what works now, but also about making it easy for others (including your future selves) to understand and use. Keep up the good work, and always think about the user experience!
Line range hint
119-170: Now, let's examine our updateQueryAction function, class!I'm glad to see you've tidied up this function too. Just like in createQueryAction, you've removed the walkthrough-related code. This is good consistency!
However, I want you to think about something: Are there any other parts of our application that might have been relying on these walkthrough features? It's like removing a chapter from a textbook - we need to make sure no other chapters were referencing it.
Also, notice how our analytics logging has changed. We're still tracking important information, but we've removed the walkthrough-specific data. This is a good example of how our code evolves as our application's features change.
Let's do a quick check to make sure we haven't missed anything:
#!/bin/bash # Description: Check for any remaining references to walkthrough in our codebase # Test: Search for any remaining walkthrough references rg --type typescript 'walkthrough' -iThis will help us ensure we've removed all references to the walkthrough feature. If we find any, we might need to update those areas too!
Line range hint
80-117: Let's review the changes in our createQueryAction function, students!I'm pleased to see that you've removed the walkthrough-related code. It's like taking training wheels off a bicycle - we're moving forward! However, let's make sure our bicycle still rides smoothly.
Can you double-check that removing the isWalkthroughOpened variable and popFeature function call doesn't affect the query creation process? It's important to ensure that our actions still work as expected, even without the walkthrough.
Also, notice how we've simplified our analytics event. It's like writing a more concise essay - we're only including the most relevant information now.
To make sure everything is working correctly, let's run a quick test:
This will help us see if there are any places where we might need to update how we're calling createQueryAction.
✅ Verification successful
Let's refine our approach to searching for
createQueryActionin TypeScript files!It seems that ripgrep doesn't recognize
typescriptas a file type. No worries—we can adjust our script to target.tsand.tsxfiles directly. This should help us locate all instances ofcreateQueryActioneffectively.
Great job! The
createQueryActionfunction is still being called correctly.All instances of
createQueryActioninQueryTemplates.tsxand other relevant files are intact, ensuring that the removal of walkthrough-related code doesn't disrupt the query creation process. Everything looks smooth!🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if createQueryAction is still being called correctly # Test: Search for createQueryAction usage rg --type typescript 'createQueryAction\(' -A 5Length of output: 72
Script:
#!/bin/bash # Description: Check if createQueryAction is still being called correctly # Search for createQueryAction usage in .ts and .tsx files rg 'createQueryAction\(' --glob '*.ts' --glob '*.tsx' -A 5Length of output: 2345
app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx (1)
196-196: Excellent simplification, class!I see you've removed the conditional check for the JavaScript-enabled one-click binding feature flag. This is a good step towards cleaning up our codebase and aligning with our lesson plan of removing old flags.
Let's break it down for the class:
- Before: We were checking a condition to decide whether to enable JavaScript bindings.
- After: We're always setting it to
true.This change suggests that the feature has been fully rolled out and is now a standard part of our application. Well done!
However, let's not forget our homework:
- Ensure all related code that might have depended on this flag has been updated.
- Update our documentation to reflect that this feature is now always enabled.
Keep up the good work, and remember: in coding, as in the classroom, simplicity is often the best teacher!
app/client/cypress/support/Pages/Table.ts (1)
891-891: Attention, class! Let's examine this new method.The
getFormattedTomorrowDatesmethod is a well-structured addition to our codebase. It provides a helpful utility for obtaining tomorrow's date in two different formats. This method demonstrates good use of JavaScript's Date object and formatting functions.app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (1)
183-183: Well done on simplifying the code, class!I see you've removed the conditional check for a feature flag and added 'sourceData' to the
dynamicPropertyPathList. This is a good step forward! It shows that we're confident in the feature and are treating 'sourceData' as a dynamic property. Remember, this means 'sourceData' will now be re-evaluated when dependencies change, so make sure it's handled efficiently in the rest of the codebase.app/client/src/widgets/SelectWidget/widget/index.tsx (2)
Line range hint
165-171: Well done on enhancing the query generation!Class, let's take a moment to appreciate the improvement in our
getQueryGenerationConfigmethod. By adding thewhereclause that references the widget'sfilterText, we've made our server-side filtering more robust. This change allows for more dynamic and responsive queries based on user input.
Line range hint
1-1023: Excellent work on improving our SelectWidget!Class, let's recap the enhancements we've seen in this file:
- We've improved our query generation capabilities.
- We've simplified our property update logic.
- We've optimized our filter change handling.
These changes collectively contribute to a more efficient, responsive, and maintainable SelectWidget. Keep up the good work, and remember to consider the suggestions for further improvements. Well done!
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js (1)
155-159:⚠️ Potential issueAvoid Using
cy.waitin TestsThe use of
cy.wait(1000);is discouraged as it can lead to flaky tests.Refactor to use assertions or other synchronization methods. For example, wait for a specific element to appear:
-cy.wait(1000); +agHelper.WaitUntilElementAppear(propPane._propertyControl("Show pagination"));Ensure
agHelper.WaitUntilElementAppearis implemented appropriately.Likely invalid or redundant comment.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js (2)
129-129: Verify the Escaping in the Selector StringIn the line
_.table.ExpandIfCollapsedSection("search\\&filters");, you're using double backslashes to escape the&character. This might not be necessary and can cause confusion or errors. Ensure that the escaping is intentional and functioning as expected.Please check if the double backslash is required. If not, you can simplify it:
- _.table.ExpandIfCollapsedSection("search\\&filters"); + _.table.ExpandIfCollapsedSection("search&filters");
128-128:⚠️ Potential issueAvoid Using Fixed Delays Like
cy.wait(500);Using arbitrary waits like
cy.wait(500);can slow down your test suite and still may not ensure that the application is ready for the next action. It's better to wait for a specific event or element to ensure synchronization.Consider this adjustment:
- cy.wait(500); + cy.get(".t--property-pane").should("be.visible");This change waits until the property pane is visible, ensuring that the application is ready for the next steps in your test.
Likely invalid or redundant comment.
app/client/src/widgets/TableWidgetV2/widget/index.tsx (2)
277-277: Well done on simplifying the dynamic property updates!By always pushing
tableDatatodynamicPropertyPathList, you've removed the unnecessary feature flag check and streamlined the code. This enhancement makes thegetPropertyUpdatesForQueryBindingmethod cleaner and more maintainable.
277-277: Ensure all remnants of the removed feature flag are eliminated.Since the feature flag
rollout_js_enabled_one_click_binding_enabledhas been retired, it's important to verify that there are no lingering references elsewhere in the codebase. Residual references could cause unexpected behavior or build issues.Please run the following script to check for any remaining references:
| ); | ||
| // validation of data displayed in input widgets based on search value set | ||
| EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Time for some homework revisions, students!
Let's improve our test by avoiding the use of cy.wait("@updateLayout"). Instead, we can use Cypress's built-in retry-ability to wait for elements to be in the correct state. Here's an example of how we could rewrite this:
cy.get(".t--property-control-allowsearching input").should('be.visible').click({ force: true });This way, we're letting Cypress automatically wait for the element to be visible before clicking, which is more reliable than using a fixed wait time.
Also applies to: 33-34
| public ExpandIfCollapsedSection(sectionName: string) { | ||
| cy.get(`.t--property-pane-section-collapse-${sectionName}`) | ||
| .scrollIntoView() | ||
| .then(($element) => { | ||
| cy.wrap($element) | ||
| .siblings(".bp3-collapse") | ||
| .then(($sibling) => { | ||
| const siblingHeight = $sibling.height(); // Get the height of the sibling element | ||
|
|
||
| if (!siblingHeight) { | ||
| $element.click(); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Now, let's turn our attention to the ExpandIfCollapsedSection method.
This method is designed to expand a collapsed section in the property pane if it's not already expanded. While the intention is good, there are a few areas where we can improve:
-
The method uses Cypress commands directly, which might not be consistent with the rest of the class that seems to use an
agHelperfor most Cypress interactions. -
The method doesn't handle potential errors, such as if the element is not found.
-
The logic for determining if a section is collapsed (by checking the height of a sibling element) might be fragile if the DOM structure changes.
Let's consider refactoring this method to make it more robust and consistent with the rest of the class. Here's a suggested improvement:
- public ExpandIfCollapsedSection(sectionName: string) {
- cy.get(`.t--property-pane-section-collapse-${sectionName}`)
- .scrollIntoView()
- .then(($element) => {
- cy.wrap($element)
- .siblings(".bp3-collapse")
- .then(($sibling) => {
- const siblingHeight = $sibling.height(); // Get the height of the sibling element
-
- if (!siblingHeight) {
- $element.click();
- }
- });
- });
- }
+ public ExpandIfCollapsedSection(sectionName: string) {
+ return this.agHelper.GetElement(`.t--property-pane-section-collapse-${sectionName}`)
+ .then(($element) => {
+ if ($element.siblings(".bp3-collapse").height() === 0) {
+ this.agHelper.ScrollTo(`.t--property-pane-section-collapse-${sectionName}`);
+ this.agHelper.ClickButton($element);
+ }
+ })
+ .catch((error) => {
+ this.agHelper.LogErrorAndFail(`Failed to expand section ${sectionName}: ${error}`);
+ });
+ }This refactored version:
- Uses the
agHelperfor consistency with the rest of the class. - Handles potential errors with a catch block.
- Simplifies the logic for checking if the section is collapsed.
- Uses more descriptive method names from the
agHelper.
Remember, class, consistency and error handling are key to maintaining a robust test suite!
Committable suggestion was skipped due to low confidence.
| // Input onsearchtextchanged control | ||
| cy.get(".t--property-control-onsearchtextchanged .t--js-toggle") | ||
| .first() |
There was a problem hiding this comment.
Remember to use locator variables and data- attributes instead of CSS selectors*
As we've learned, using locator variables and data-* attributes enhances the maintainability and reliability of our tests. Relying on plain strings and CSS paths can make the tests brittle and harder to maintain. Let's replace the CSS selectors with appropriate locator variables.
Here's how you might adjust the code:
-cy.get(".t--property-control-onsearchtextchanged .t--js-toggle")
- .first()
- .click();
+cy.get(commonlocators.onSearchTextChangedToggle)
+ .click();If commonlocators.onSearchTextChangedToggle doesn't exist, please consider adding it to your locators file.
📝 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.
| // Input onsearchtextchanged control | |
| cy.get(".t--property-control-onsearchtextchanged .t--js-toggle") | |
| .first() | |
| // Input onsearchtextchanged control | |
| cy.get(commonlocators.onSearchTextChangedToggle) | |
| .click(); |
| @@ -35,6 +35,7 @@ describe( | |||
| entityExplorer.DragDropWidgetNVerify(draggableWidgets.TABLE, 300, 300); | |||
|
|
|||
| // turn on filtering for the table - it is disabled by default in this PR(#34593) | |||
| table.ExpandIfCollapsedSection("search\\&filters"); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remember to use locator variables instead of plain strings.
To ensure consistency and maintainability, it's important to use locator variables for selectors. Instead of passing a plain string "search\\&filters", consider defining and using a locator variable for the "search&filters" section.
| @@ -35,6 +35,7 @@ describe( | |||
| before(() => { | |||
| _.entityExplorer.DragDropWidgetNVerify(_.draggableWidgets.TABLE); | |||
| // turn on filtering for the table - it is disabled by default in this PR(#34593) | |||
| _.table.ExpandIfCollapsedSection("search\\&filters"); | |||
There was a problem hiding this comment.
Remember to use locator variables instead of plain strings for locators
Dear student, to maintain code consistency and adhere to our best practices, please use locator variables instead of plain strings when specifying locators. This enhances the maintainability and readability of the code.
You can apply the following change to fix this:
- _.table.ExpandIfCollapsedSection("search\\&filters");
+ _.table.ExpandIfCollapsedSection(_.locators.searchAndFiltersSection);If a locator variable for "search\\&filters" does not exist yet, kindly define it accordingly.
Committable suggestion was skipped due to low confidence.
| @@ -237,6 +240,7 @@ describe( | |||
| it("7. should check that adding cyclic dependency in the table doesn't crash the app", () => { | |||
| //_.deployMode.NavigateBacktoEditor(); | |||
| cy.openPropertyPane("tablewidgetv2"); | |||
| _.table.ExpandIfCollapsedSection("rowselection"); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider Using Locator Variables Instead of Plain Strings
Using plain strings like _.table.ExpandIfCollapsedSection("rowselection"); can make your tests harder to maintain, especially if the string is used in multiple places or may change in the future. By defining locator variables or constants, you centralize these values and make updates easier.
For example:
const rowSelectionSection = "rowselection";
_.table.ExpandIfCollapsedSection(rowSelectionSection);This approach improves readability and maintainability of your tests.
| table.ExpandIfCollapsedSection("search\\&filters"); | ||
| agHelper.GetNClick(".t--property-control-allowfiltering input"); |
There was a problem hiding this comment.
Remember to use locator variables instead of plain strings for selectors
At line 20, we have:
agHelper.GetNClick(".t--property-control-allowfiltering input");Using plain string selectors can make our tests brittle and harder to maintain. It's important to use locator variables, which promote reuse and make changes easier to manage.
Here's how you can update the code:
+const allowFilteringToggle = ".t--property-control-allowfiltering input";
...
-agHelper.GetNClick(".t--property-control-allowfiltering input");
+agHelper.GetNClick(allowFilteringToggle);| @@ -16,6 +16,7 @@ | |||
| it("1. Verify Table Filter for 'empty'", function () { | |||
| entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250); | |||
| // turn on filtering for the table - it is disabled by default in this PR(#34593) | |||
| table.ExpandIfCollapsedSection("search\\&filters"); | |||
| agHelper.GetNClick(".t--property-control-allowfiltering input"); | |||
| table.AddSampleTableData(); | |||
| propPane.UpdatePropertyFieldValue( | |||
There was a problem hiding this comment.
Avoid using agHelper.Sleep();—wait for specific conditions instead
In line 22, the code uses:
agHelper.Sleep(2000); //table to filter & records to disappearRelying on fixed delays can lead to flaky tests because the actual load time may vary. Instead, we should wait for a specific element or condition that indicates the table has finished filtering.
Consider replacing the sleep with a wait for the table to become empty:
table.WaitForTableEmpty("v2");This ensures the test proceeds only after the table is fully filtered.
| @@ -17,6 +17,7 @@ describe( | |||
| entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250); | |||
| //propPane.EnterJSContext("Table data", JSON.stringify(this.dataSet.TableInput)); | |||
| // turn on filtering for the table - it is disabled by default in this PR(#34593) | |||
| table.ExpandIfCollapsedSection("search\\&filters"); | |||
| agHelper.GetNClick(".t--property-control-allowfiltering input"); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Let's improve selector usage by adopting locator variables
In line 21, you're directly using a CSS selector ".t--property-control-allowfiltering input". To make our tests more robust and maintainable, it's important to use locator variables and leverage data-* attributes for selectors, as highlighted in our coding guidelines. This practice enhances readability and eases future updates to the code.
To implement this improvement, let's define a locator variable using a data-* attribute:
Add the following in your locators file (e.g., locators.js):
export const allowFilteringToggle = '[data-cy="t--property-control-allowfiltering"] input';Then, update line 21 as shown below:
- agHelper.GetNClick(".t--property-control-allowfiltering input");
+ agHelper.GetNClick(allowFilteringToggle);| @@ -28,6 +28,7 @@ describe( | |||
| before(() => { | |||
| entityExplorer.DragDropWidgetNVerify("tablewidgetv2", 650, 250); | |||
| // turn on filtering for the table - it is disabled by default in this PR(#34593) | |||
| table.ExpandIfCollapsedSection("search\\&filters"); | |||
| agHelper.GetNClick(".t--property-control-allowfiltering input"); | |||
There was a problem hiding this comment.
Remember to use locator variables instead of plain strings for selectors.
In line 32, you've used a plain string selector in agHelper.GetNClick(".t--property-control-allowfiltering input"). It's important to use locator variables for selectors to maintain consistency and ease future updates.
Let's define a locator variable for .t--property-control-allowfiltering input and replace the plain string. This practice enhances code readability and adherence to our coding standards.
|
/ci-test-limit runId=11102991010 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11105673414. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
app/client/cypress/e2e/Regression/ClientSide/DynamicHeight/JsonForm_spec.ts (1)
Line range hint
1-65: Class, let's talk about good habits in our code!Now, children, I've noticed something in our test that we need to improve. Can anyone spot the line that appears several times and starts with
agHelper.Sleep? Yes, that's right!Using
agHelper.Sleep(5000)is like taking a nap in the middle of a test - it's not very efficient! Instead of sleeping, we should be actively waiting for something to happen. Here are some better ways to do this:
- Use
cy.wait('@apiCall')if you're waiting for a specific API call to complete.- Use
cy.get('element').should('be.visible')to wait for an element to appear.- If you're waiting for a change in an element's property, use something like
cy.get('element').should('have.css', 'height', expectedHeight).Remember, class, in testing as in life, it's better to be active and attentive rather than just waiting around!
Let's work on replacing these
agHelper.Sleep(5000)calls with more appropriate waiting mechanisms. Who wants to come to the board and show us how to do the first one?app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_spec.js (3)
Line range hint
96-97: Consider using data- attributes or locator variables instead of class selectors*Using class selectors like
".show-page-items"and".page-item"can make your tests fragile if class names change in the future. It's a good practice to use data-* attributes or predefined locator variables for more robust and maintainable selectors.You might update the selectors like this:
- cy.get(".show-page-items").should("contain", "20 Records"); + cy.get(locators._showPageItems).should("contain", "20 Records"); - cy.get(".page-item").next().should("contain", "of 2"); + cy.get(locators._pageItem).next().should("contain", "of 2");Ensure that the appropriate locator variables
locators._showPageItemsandlocators._pageItemare defined in your locators file.
Line range hint
99-99: Avoid using class selectors; prefer data- attributes or locator variables*Referencing elements by class names like
".t--table-widget-next-page"may lead to brittle tests if those class names change. It's advisable to use data-* attributes or locator variables for more stable and maintainable selectors.Consider updating your selectors:
- cy.get(".t--table-widget-next-page").should("not.have.attr", "disabled"); + cy.get(locators._tableWidgetNextPage).should("not.have.attr", "disabled"); ... - cy.get(".t--table-widget-next-page").should("have.attr", "disabled"); + cy.get(locators._tableWidgetNextPage).should("have.attr", "disabled");Make sure that
locators._tableWidgetNextPageis properly defined in your locators file.Also applies to: 103-103
Line range hint
102-102: Avoid using static waits likecy.wait(500)Including fixed-duration waits can make your tests less reliable and slower. It's better to wait for specific conditions or elements to ensure that the application is ready before proceeding.
Consider replacing the static wait with a dynamic wait. Since you're checking that the "Next Page" button has the
disabledattribute in the next line, you might not need the static wait at all.- cy.wait(500);If a wait is necessary, you can wait for a network call or an element state:
- cy.wait(500); + cy.wait("@postExecute");Or ensure the element is in the expected state:
- cy.wait(500); + cy.get(locators._tableWidgetNextPage).should("have.attr", "disabled");app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_2_spec.js (4)
Line range hint
16-17: Avoid Fixed Delays withcy.wait(5000);In lines 16-17, you're using
cy.wait(5000);to introduce a fixed delay in your test. Relying on fixed delays can lead to flaky tests because it doesn't guarantee that the desired state has been reached. Instead, you should use dynamic waits that wait for specific events or elements.You already have
cy.wait("@updateLayout");immediately after the fixed wait. This dynamic wait should be sufficient. Let's remove the unnecessary fixed delay:- // eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(5000);
65-65: Use Locator Variables Instead of Plain StringsIn line 65, you're using a plain string
"search\\&filters"in thetable.ExpandIfCollapsedSectionmethod. According to our coding guidelines, it's better to use locator variables or data-* attributes for locators rather than plain strings. This practice enhances maintainability and reduces the likelihood of errors when locators change.Consider defining a locator variable for
"search\\&filters"in your locators file and using that variable here.
Line range hint
70-70: Correct Typographical Errors in CommentsThe comment on line 70 contains typographical errors:
"// Chage deat search text value to \"data\"". Clear and correct comments improve code readability and maintainability.Please apply the following correction:
- // Chage deat search text value to "data" + // Change default search text value to "data"
Line range hint
91-94: Prefer Data- Attributes Over CSS Selectors*In lines 91-94, you are using a CSS selector
".t--property-control-propertyname pre span span"to select an element. While this works, using data-* attributes for selectors is a better practice. It makes your tests more robust and less likely to break with UI changes.Consider updating the selector to use a data-* attribute:
- cy.get(".t--property-control-propertyname pre span span").should( + cy.get("[data-testid='t--property-control-propertyname']").should( "have.text", "customColumn18", );This change will make your test more maintainable and aligned with best practices.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
- app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/DynamicHeight/JsonForm_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_PropertyPane_2_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js (3 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts (1 hunks)
- app/client/cypress/support/Pages/PropertyPane.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- app/client/cypress/e2e/Regression/ClientSide/Binding/InputWidget_TableV2_Sorting_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2Widget_selectedRow_Input_widget_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_ClientSide_Search_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_DefaultSearch_Input_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Binding/TextTableV2_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Edge_case_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_MultiRowSelect_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts
🧰 Additional context used
📓 Path-based instructions (4)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_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/DynamicHeight/JsonForm_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/Widgets/TableV2/TableV2_PropertyPane_2_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/PropertyPane.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.
📓 Learnings (1)
app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_spec.js (1)
Learnt from: Aishwarya-U-R PR: appsmithorg/appsmith#29405 File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41 Timestamp: 2023-12-08T08:59:53.560Z Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ClientSide/DynamicHeight/JsonForm_spec.ts (1)
19-19: Good addition, class! Let's discuss why this is important.Children, the new line
propPane.ExpandIfCollapsedSection("general");is like making sure your textbook is open to the right page before you start reading. It ensures that the "general" section of the properties pane is visible before we try to change any settings. This makes our test more reliable, just like how checking your pencil works before starting a test makes sure you can answer all the questions.app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2TextPagination_spec.js (1)
93-94: Excellent work on updating the pagination settings!You've correctly expanded the pagination section and updated the "Total Records" field to "20". This will help ensure that the table displays the correct number of records and pagination controls as intended.
|
|
||
| public ExpandIfCollapsedSection(sectionName: string) { | ||
| cy.get(`.t--property-pane-section-collapse-${sectionName}`) | ||
| .scrollIntoView() | ||
| .then(($element) => { | ||
| cy.wrap($element) | ||
| .siblings(".bp3-collapse") | ||
| .then(($sibling) => { | ||
| const siblingHeight = $sibling.height(); // Get the height of the sibling element | ||
|
|
||
| if (!siblingHeight) { | ||
| $element.click(); | ||
| } | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Class, let's review this new method together!
The ExpandIfCollapsedSection method is a good addition to our toolkit. It's like a helpful classroom assistant, making sure all our "learning sections" are open and ready for us to explore. However, we can make it even better!
-
Error Handling: We should add a safety net in case we can't find our section. It's like having a backup plan for when a student is absent.
-
Confirmation: Let's add a way to make sure our section actually opened. It's like double-checking that everyone has their textbooks open to the right page.
-
Neat Handwriting: Our code's indentation is a bit messy. Remember, neat handwriting makes it easier for everyone to read!
Here's how we could improve it:
public ExpandIfCollapsedSection(sectionName: string) {
cy.get(`.t--property-pane-section-collapse-${sectionName}`)
.scrollIntoView()
.then(($element) => {
cy.wrap($element)
.siblings(".bp3-collapse")
.then(($sibling) => {
const siblingHeight = $sibling.height();
if (!siblingHeight) {
cy.wrap($element).click().then(() => {
// Verify that the section is now expanded
cy.wrap($sibling).should('have.css', 'height').and('not.eq', '0px');
});
}
});
})
.should('exist')
.then(($element) => {
if (!$element.length) {
throw new Error(`Section "${sectionName}" not found`);
}
});
}This improved version is like a well-organized lesson plan. It checks for errors, confirms our actions, and keeps everything tidy. Great job on adding this helpful method to our class!
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11119739074. |
…airgap-remove-old-flags
…airgap-remove-old-flags
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/11137938998. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/client/cypress/e2e/Regression/Apps/CommunityIssues_Spec.ts (5 hunks)
- app/client/cypress/limited-tests.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/limited-tests.txt
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/Apps/CommunityIssues_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.
🔇 Additional comments (4)
app/client/cypress/e2e/Regression/Apps/CommunityIssues_Spec.ts (4)
77-78: Well done verifying the 'Server side pagination' settingYou've correctly expanded the "pagination" section and asserted that "Server side pagination" is enabled. This ensures the feature is functioning as expected.
Line range hint
80-84: Excellent validation of the default selected rowBy expanding the "rowselection" section and validating the "Default selected row", you've ensured that the table selection behaves correctly.
119-120: Good job toggling 'Server side pagination' offDisabling "Server side pagination" and verifying the application's response helps confirm the feature's behavior under different settings.
132-132: Appropriate update of 'Default selected row'By changing the "Default selected row" to "1", you've properly tested how the table handles different default selections.
| @@ -140,6 +144,7 @@ describe( | |||
|
|
|||
| it("5. Verify Default search text in table as per 'Default search text' property set + Bug 12228", () => { | |||
| EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); | |||
| propPane.ExpandIfCollapsedSection("search\\&filters"); | |||
There was a problem hiding this comment.
Please review the escape characters in 'search\&filters'
It appears you're using an extra backslash in "search\&filters". Ensure this is intentional, as unnecessary escape characters might lead to issues. If the section name is "search&filters", you can write it without the double backslash.
Proposed change:
-propPane.ExpandIfCollapsedSection("search\\&filters");
+propPane.ExpandIfCollapsedSection("search&filters");📝 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.
| propPane.ExpandIfCollapsedSection("search\\&filters"); | |
| propPane.ExpandIfCollapsedSection("search&filters"); |
| @@ -171,6 +176,7 @@ | |||
|
|
|||
| it("6. Validate Search table with Client Side Search enabled & disabled & onSearchTextChanged is set", () => { | |||
| EditorNavigation.SelectEntityByName("Table1", EntityType.Widget); | |||
| propPane.ExpandIfCollapsedSection("search\\&filters"); | |||
There was a problem hiding this comment.
Confirm the necessity of the escape character in 'search\&filters'
Double-check whether the backslash in "search\&filters" is necessary. Using unnecessary escape characters can cause confusion and potential errors.
Suggested change:
-propPane.ExpandIfCollapsedSection("search\\&filters");
+propPane.ExpandIfCollapsedSection("search&filters");📝 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.
| propPane.ExpandIfCollapsedSection("search\\&filters"); | |
| propPane.ExpandIfCollapsedSection("search&filters"); |
…airgap-remove-old-flags
…airgap-remove-old-flags
…airgap-remove-old-flags
…airgap-remove-old-flags
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11209651511. |
|
Deploy-Preview-URL: https://ce-36609.dp.appsmith.com |
Description
Removed all the occurrences of listed flags in the codebase:
Fixes #36256
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11177173738
Commit: bfbf6bb
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 04 Oct 2024 10:31:10 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores