chore: Updated js file to ts files #37312
Conversation
WalkthroughThe pull request includes modifications to several test specification files in a Cypress testing environment. The primary change across these files involves updating import statements for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.ts (3)
Line range hint
28-34: Replace cy.wait with proper assertions.Using hardcoded waits is discouraged in Cypress. Instead, use explicit assertions or wait for specific elements/states.
Consider refactoring to:
- cy.wait(3000); //wait for text field to alter + cy.get(`${widgetsPage.textWidget} .bp3-ui-text`) + .should('not.have.value', fieldValue) + .should('exist');
Line range hint
22-23: Update selectors to use data- attributes.*Current selectors rely on CSS classes which are prone to changes. Use data-* attributes for more reliable element selection.
Consider updating:
- cy.get(`${fieldPrefix}-${fieldName}`) + cy.get(`[data-testid="jsonform-field-${fieldName}"]`)Also applies to: 37-38
Line range hint
19-65: Add error handling for JSON parsing.The function assumes JSON.parse will always succeed. Consider adding proper error handling for malformed JSON data.
Consider wrapping JSON operations in try-catch:
cy.get(`${widgetsPage.textWidget} .bp3-ui-text`).then(($el) => { + try { const formData = JSON.parse($el.text()); const formDataValue = resolveFieldValue ? resolveFieldValue(formData) : formData[fieldName]; cy.wrap(formDataValue).should("be.undefined"); + } catch (e) { + cy.fail('Invalid JSON data in form'); + } });app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.ts (1)
Line range hint
31-31: Replace cy.wait with proper Cypress assertionsUsing hardcoded waits (
cy.wait(3000)) is discouraged as it makes tests brittle and slower. Instead, leverage Cypress's built-in retry-ability.Replace the waits with proper assertions:
- cy.wait(3000); //wait for text field to alter + cy.get(`${widgetsPage.textWidget} .bp3-ui-text`) + .should('not.be.empty') + .should((elem) => { + expect(JSON.parse(elem.text())).to.exist; + });Also applies to: 47-47
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
app/client/cypress/e2e/Regression/ClientSide/Templates/CreateNewAppFromTemplates_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Templates/Filtering/TemplatesModal_filtering_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/client/cypress/e2e/Regression/ClientSide/Templates/CreateNewAppFromTemplates_spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Templates/Filtering/TemplatesModal_filtering_spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_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/JSONForm/JSONForm_HiddenFields_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.
🔇 Additional comments (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_1_spec.ts (1)
19-19: LGTM: Function signature change is well implemented.
The optional parameter addition improves function flexibility while maintaining backward compatibility.
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_HiddenFields_2_spec.ts (2)
19-19: LGTM: TypeScript optional parameter addition
The function signature change to make resolveFieldValue optional is a good improvement that maintains backward compatibility.
Line range hint 89-93: LGTM: Well-structured test suite
The test suite follows good practices:
- Clear test descriptions
- Proper test tagging
- Multiple assertions
| @@ -16,7 +16,7 @@ import { | |||
| const fieldPrefix = ".t--jsonformfield"; | |||
| const backBtn = "[data-testid='t--property-pane-back-btn']"; | |||
|
|
|||
| function hideAndVerifyProperties(fieldName, fieldValue, resolveFieldValue) { | |||
| function hideAndVerifyProperties(fieldName, fieldValue, resolveFieldValue?) { | |||
| // Check if visible | |||
| cy.get(`${fieldPrefix}-${fieldName}`).should("exist"); | |||
|
|
|||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use data- attributes for selectors*
Current selectors don't follow best practices. Replace class-based and attribute selectors with data-* attributes.
Examples:
- .bp3-ui-text
+ [data-testid="form-text"]
- ${fieldPrefix}-${fieldName}
+ [data-testid="json-form-field-${fieldName}"]Also, consider creating a dedicated locators file for JSON form-specific selectors to improve maintainability.
Also applies to: 39-39, 47-47, 66-66
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.ts (4)
Line range hint
1-400: Refactor test structure to follow best practicesThe test structure needs improvements in several areas:
- Replace cy.wait() calls with proper Cypress commands like cy.intercept() or cy.waitUntil()
- Avoid xpath selectors and use data-cy attributes instead
- Split this large test into smaller, focused test cases
Example refactor for a wait statement:
-cy.wait("@importNewApplication").then(() => { +cy.intercept('POST', '**/api/v1/applications/import').as('importNewApplication'); +cy.waitUntil(() => + cy.get('@importNewApplication').then((xhr) => xhr.response.statusCode === 200) +).then(() => {
133-133: LGTM with minor suggestionThe change from
vartoletimproves variable scoping. Consider using a more descriptive name likecardNumberFormatfor better clarity.-let format = /^\d{4}-\d{4}-\d{4}(-\d{4})?$/; +let cardNumberFormat = /^\d{4}-\d{4}-\d{4}(-\d{4})?$/;
Line range hint
250-280: Improve test reliability by removing fixed timeoutsThe test uses fixed timeouts which could lead to flaky tests. Replace them with proper Cypress commands that wait for specific conditions.
Example refactor:
-cy.wait(2000); +cy.get(selector).should('be.visible').and('be.enabled');
Line range hint
70-90: Improve test assertionsThe test assertions could be more maintainable:
- Move string literals to constants
- Use Cypress's built-in assertions instead of .then() blocks where possible
- Group related assertions using .should() chains
Example refactor:
-cy.readTabledataPublish("0", "1").then((cellData) => { - expect(cellData).to.be.equal("100"); -}); +const EXPECTED_VALUES = { + FIRST_ROW: "100", +}; +cy.readTabledataPublish("0", "1") + .should("equal", EXPECTED_VALUES.FIRST_ROW);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_Spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Migration_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.
sagar-qa007
added
ok-to-test
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
We currently maintain two separate file types across specifications, locators, and commands in our Cypress codebase. To streamline and standardise the code, improve TypeScript support, enhance readability, and simplify the process of writing new test cases, I have updated all JavaScript files to TypeScript.
Fixes #37311
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/11790965886
Commit: 4f902e0
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 12 Nov 2024 06:21:59 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
reconnectDatasourceLocatorsmodule in multiple test files to ensure proper functionality without file extension issues.Improvements
resolveFieldValueparameter optional in thehideAndVerifyPropertiesfunction, enhancing flexibility in form data verification for JSON forms.