fix: Updated the depreciated RunQueryNVerifyResponseViews to use runQueryAndVerifyResponseViews #37945
Conversation
…eryAndVerifyResponseViews
albinAppsmith
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe changes in this pull request focus on updating the Cypress end-to-end test suite for Google Sheets functionality. The primary modification involves renaming the method Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts (1)
Line range hint
1-350: Remove cy.wait and agHelper.Sleep callsThe test contains multiple instances of
agHelper.Sleep()and implicit waits. This makes tests flaky and slower.Replace these with proper Cypress wait strategies:
- agHelper.Sleep(500); + cy.get('[data-cy="dropdown-option"]').should('be.visible'); - cy.wait('@postExecute') + cy.wait('@postExecute').its('response.statusCode').should('eq', 200)app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (2)
Line range hint
1-350: Refactor test assertions for better maintainabilityConsider extracting common assertion messages and test data into constants or fixtures.
+ const ERROR_MESSAGES = { + INSUFFICIENT_SCOPE: "Request had insufficient authentication scopes.", + INVALID_RANGE: "Unable to parse range: 'Sheet 2'!1:1" + }; - expect(interception.response.body.data.body).to.deep.equal( - "Request had insufficient authentication scopes." - ); + expect(interception.response.body.data.body).to.deep.equal( + ERROR_MESSAGES.INSUFFICIENT_SCOPE + );
Line range hint
1-1: Consider implementing shared test utilitiesMultiple test files share similar patterns for:
- Query verification
- Error message assertions
- Data setup and teardown
Consider creating shared test utilities to reduce duplication and improve maintainability.
app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts (2)
245-245: Method signature updates look good, but avoid using Sleep/wait.While the method updates are correct, there are instances of
agHelper.Sleep()andcy.wait()in this test file which should be avoided per coding guidelines.Consider replacing
agHelper.Sleep()andcy.wait()with Cypress's built-in retry-ability and assertions:// Instead of: agHelper.Sleep(); // Use: cy.get('selector').should('exist'); // Instead of: cy.wait('@networkCall') // Use: cy.intercept('POST', '/api/endpoint').as('networkCall'); cy.get('selector').should('exist'); cy.wait('@networkCall').its('response.statusCode').should('eq', 200);Also applies to: 278-278
Line range hint
1-400: Consider additional improvements for test reliability.While the method signature updates are good, there are several areas for improvement in the test file:
- Replace all instances of
agHelper.Sleep()andcy.wait()with proper Cypress assertions- Consider using data-* attributes for selectors instead of class names
- Consider moving test data to fixtures instead of inline JSON
Would you like assistance in implementing these improvements?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts(8 hunks)app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts(2 hunks)app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts(7 hunks)app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts(7 hunks)app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts(8 hunks)app/client/cypress/e2e/GSheet/WidgetBinding_AllAccess_Spec.ts(1 hunks)app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts(7 hunks)app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/WidgetBinding_AllAccess_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🔇 Additional comments (12)
app/client/cypress/e2e/GSheet/WidgetBinding_AllAccess_Spec.ts (1)
60-60: Method signature update looks good
The update to use runQueryAndVerifyResponseViews with an object parameter improves code clarity and maintainability.
app/client/cypress/e2e/GSheet/WidgetBinding_SelectedAccess_Spec.ts (1)
60-60: Method signature update looks good
The update maintains consistency with the new method signature pattern.
app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts (2)
107-107: Method signature update looks good
The update maintains consistency with the new method signature pattern throughout the test suite.
93-93: Method signature update looks good
The update to use object parameter improves readability. Consider verifying that GSHEET_DATA is properly loaded.
✅ Verification successful
Let me verify the test data structure further to ensure it's properly defined and used.
GSHEET_DATA is properly defined and used correctly
The constant is well-structured with test data containing all required fields (rowIndex, uniq_id, etc.) and is used consistently throughout the test file for verifying response counts and specific field values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify GSHEET_DATA constant is properly defined
rg -A 5 "GSHEET_DATA.*=.*\[" app/client/cypress/fixtures/
Length of output: 519
Script:
#!/bin/bash
# Check how GSHEET_DATA is used in the test file
rg -B 2 -A 2 "GSHEET_DATA" app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts
# Check the complete test data structure
cat app/client/cypress/fixtures/test-data-gsheet.ts
Length of output: 12196
app/client/cypress/e2e/GSheet/SelectedSheet_Access_Spec.ts (1)
117-117: Replace deprecated method calls with updated version
The changes from RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews are correct, improving consistency across the codebase.
Also applies to: 126-126, 136-136, 152-152, 172-172, 190-190, 240-240, 275-275
app/client/cypress/e2e/GSheet/AllAccess_Spec.ts (2)
129-129: Replace deprecated method calls with updated version
The changes from RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews are correct, improving consistency across the codebase.
Also applies to: 138-138, 148-148, 164-164, 184-184, 202-202, 249-249, 284-284
Line range hint 15-15: Verify if describe.skip is intentional
The test suite is marked with describe.skip. Please verify if this is intentional or if the tests should be enabled.
✅ Verification successful
The skip is intentional and consistent across GSheet test files
All GSheet test files in the e2e directory are currently skipped:
- app/client/cypress/e2e/GSheet/WidgetBinding_AllAccess_Spec.ts
- app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts
- app/client/cypress/e2e/GSheet/GsheetMisc_Spec.ts
- app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts
- app/client/cypress/e2e/GSheet/AllAccess_Spec.ts
This appears to be a deliberate decision to skip the entire GSheet test suite.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other GSheet test files are also skipped
rg "describe\.skip.*GSheet" -A 1
Length of output: 34
Script:
#!/bin/bash
# Let's try a broader search for skipped tests in the GSheet directory
fd ".*Spec\.ts$" --exec rg -l "describe\.skip|it\.skip" {}
# Also get the content of the specific file to verify the skip
cat "app/client/cypress/e2e/GSheet/AllAccess_Spec.ts"
Length of output: 13026
app/client/cypress/e2e/GSheet/ReadOnly_Access_Spec.ts (1)
139-139: Replace deprecated method calls with updated version
The changes from RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews are correct, improving consistency across the codebase.
Also applies to: 147-147, 156-156, 171-171, 190-190, 207-207, 227-227, 260-260
app/client/cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts (1)
241-241: Method calls updated correctly to use object parameter
The updates to runQueryAndVerifyResponseViews are consistent across all occurrences, properly using the new object parameter format.
Also applies to: 275-275, 337-337, 340-340, 378-378, 407-407, 442-442, 452-452
app/client/cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts (1)
143-143: Method call updated correctly to use object parameter
The update to runQueryAndVerifyResponseViews follows the new signature pattern consistently.
app/client/cypress/e2e/GSheet/ReadNWrite_Access_Spec.ts (2)
131-131: LGTM! Method signature update looks good.
The changes consistently update the deprecated method calls to use the new signature with object parameters.
Also applies to: 139-139, 148-148, 163-163
182-182: LGTM! Consistent method updates in filtering tests.
The method signature updates are properly applied in the filtering and cell range verification sections.
Also applies to: 199-199
|
Closing this PR. These changes are already available in #37897 |
Description
This PR removes the deprecated RunQueryNVerifyResponseViews and implemented runQueryAndVerifyResponseViews instead.
Fixes
Automation
/ok-to-test tags="@tag.Sanity, @tag.Datasource"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12163035711
Commit: aaa6bcd
Cypress dashboard.
Tags: @tag.Sanity, @tag.Datasource
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/ApiTests/API_MultiPart_Spec.ts
List of identified flaky tests.Wed, 04 Dec 2024 16:36:04 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor