chore: Reverted appsmith url #37267
chore: Reverted appsmith url #37267
Conversation
WalkthroughThe changes in this pull request involve modifications to several Cypress end-to-end test files, primarily updating URLs from "https://www.appsmith.com" to "https://www.google.com". This affects various test cases focused on user interactions, API configurations, and validation processes. A new test suite has also been introduced to handle user login and application import scenarios, emphasizing sequential execution and error management during CSV data parsing. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (9)
app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts (3)
Line range hint
24-24: Remove agHelper.Sleep() calls.Using
agHelper.Sleep()is against the coding guidelines. Replace with proper Cypress wait commands that wait for specific conditions.- agHelper.Sleep(1000); + cy.waitUntil(() => { + return cy.get('[data-cy="query-editor"]').should('be.visible'); + });Also applies to: 44-44
Line range hint
22-22: Use data- attributes for selectors.*Replace
cy.get("@dsName")with appropriate data-* attributes for better test stability.- cy.get("@dsName") + cy.get('[data-cy="datasource-name"]')Also applies to: 42-42
Line range hint
13-17: Enhance test assertions.The API run button test should include multiple assertions as per guidelines. Consider adding checks for button state, appearance, and tooltip.
apiPage.CreateApi("FirstAPI", "GET"); apiPage.AssertRunButtonDisability(true); +apiPage.AssertRunButtonState({ + isDisabled: true, + tooltipVisible: true, + tooltipText: "Please enter a valid URL" +}); apiPage.EnterURL(url); apiPage.AssertRunButtonDisability(false); +apiPage.AssertRunButtonState({ + isDisabled: false, + tooltipVisible: false +});app/client/cypress/e2e/Regression/ClientSide/Workspace/LoginAndImportApp_Spec.ts (1)
11-14: Add meaningful tags to the test suiteThe tags array is empty. Add appropriate tags for test categorization.
describe( "Create new workspace and invite user & validate all roles", - { tags: [""] }, + { tags: ["@workspace", "@regression"] }, () => {app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts (3)
Line range hint
33-33: Replace cy.wait with proper assertion.Using fixed wait times is discouraged in Cypress tests. Instead, use Cypress's built-in retry-ability and assertions.
- cy.wait(500); + cy.waitUntil(() => Cypress.$(oneClickBindingLocator.datasourceDropdownSelector).length > 0);
Line range hint
169-172: Use Cypress's built-in assertions instead of jQuery length checks.Replace jQuery-style length checks with Cypress's built-in assertions for better retry-ability and clearer error messages.
- agHelper - .GetElement(oneClickBindingLocator.datasourceQuerySelector()) - .then(($ele) => { - expect($ele.length).equals(upfrontContentCount); - }); + cy.get(oneClickBindingLocator.datasourceQuerySelector()) + .should('have.length', upfrontContentCount);Also applies to: 178-182, 189-193
Line range hint
23-24: Improve test isolation.The test cases appear to be dependent on each other's state. Each test should be independent and set up its own state to ensure reliability.
Consider:
- Moving common setup to beforeEach
- Resetting state between tests
- Breaking down large test cases into smaller, focused ones
Also applies to: 89-90, 149-150
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/uiToCode_spec.ts (2)
Line range hint
52-290: Consider using custom commands for repetitive test actionsThe test file contains multiple instances of similar action sequences (adding actions, validating JS field values). This can be refactored into custom Cypress commands for better maintainability.
Example custom command:
// In commands.ts Cypress.Commands.add('addNavigateAction', (url: string) => { propPane.SelectPlatformFunction("onClick", "Navigate to"); propPane.SelectActionByTitleAndValue("Navigate to", "Select page"); agHelper.GetNClick(propPane._navigateToType("URL")); agHelper.TypeText( propPane._actionSelectorFieldByLabel("Enter URL"), url ); agHelper.GetNClick(propPane._actionSelectorPopupClose); });
Line range hint
1-290: Add test data cleanup in afterEach hookThe test suite modifies button actions but doesn't clean up after each test. This could affect subsequent test runs.
Add cleanup:
afterEach(() => { EditorNavigation.SelectEntityByName("Button1", EntityType.Widget); propPane.EnterJSContext("onClick", ""); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/uiToCode_spec.ts(10 hunks)app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Workspace/LoginAndImportApp_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/uiToCode_spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/Workspace/LoginAndImportApp_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/ActionExecution/Block_Execution.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_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 (1)
app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts (1)
12-12: URL change needs verification with the issue.
The URL change from appsmith.com to google.com should be verified against issue #37266 to ensure this is the intended reversion.
✅ Verification successful
URL change is aligned with issue #37266
The URL change from appsmith.com to google.com is intentional and aligns with the issue #37266 "Update third party url to another domain". The change is part of a deliberate reversion commit (75893f5) to update third-party URLs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of google.com to ensure consistency
rg -l 'www\.google\.com' --type ts
Length of output: 1120
Script:
#!/bin/bash
# Check the git history of the specific file to understand the URL change context
git log -p app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts | grep -A 5 -B 5 "google.com\|appsmith.com"
# Check issue #37266 content
gh issue view 37266
Length of output: 2341
app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/APIOnLoad_Spec.ts
Show resolved
Hide resolved
| cy.readFile( | ||
| "/Users/sagarkhalasi/codebase/appsmith/app/client/cypress/fixtures/testuserimport.csv", | ||
| ).then((csvData) => { |
There was a problem hiding this comment.
Replace hardcoded file path with a relative path
The hardcoded absolute path will fail in different environments. Use a relative path from the fixtures directory.
- cy.readFile(
- "/Users/sagarkhalasi/codebase/appsmith/app/client/cypress/fixtures/testuserimport.csv",
- ).then((csvData) => {
+ cy.fixture("testuserimport.csv").then((csvData) => {📝 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.
| cy.readFile( | |
| "/Users/sagarkhalasi/codebase/appsmith/app/client/cypress/fixtures/testuserimport.csv", | |
| ).then((csvData) => { | |
| cy.fixture("testuserimport.csv").then((csvData) => { |
| it("1. Login as and import the app as per file", () => { | ||
| testData.forEach((user) => { | ||
| console.log("JsonFilePath:", user.jsonFilePath); // This should print the string value | ||
| const resolvedJsonFilePath = user.jsonFilePath; | ||
| console.log("Resolved JsonFilePath:", resolvedJsonFilePath); // Log resolved file path | ||
| //homePage.NavigateToHome(); | ||
| cy.go(-1) | ||
| agHelper.Sleep(5000); | ||
| cy.SignupFromAPI( | ||
| user.user_email, | ||
| user.password, | ||
| ); | ||
| cy.wait(5000); | ||
| cy.go(-1) | ||
| cy.wait(5000); | ||
| cy.reload(); | ||
| cy.wait(5000); | ||
| homePage.ImportApp(resolvedJsonFilePath); | ||
| cy.wait(5000); | ||
| cy.LogOut(false); | ||
| cy.wait(5000); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Remove cy.wait() and implement proper assertions
The test case violates multiple Cypress best practices:
- Uses cy.wait() with hardcoded times
- Uses agHelper.Sleep()
- Lacks proper assertions
- Uses direct navigation with cy.go()
- it("1. Login as and import the app as per file", () => {
+ it("should import apps for each user successfully", () => {
testData.forEach((user) => {
- console.log("JsonFilePath:", user.jsonFilePath);
- const resolvedJsonFilePath = user.jsonFilePath;
- console.log("Resolved JsonFilePath:", resolvedJsonFilePath);
- cy.go(-1)
- agHelper.Sleep(5000);
cy.SignupFromAPI(
user.user_email,
user.password,
- );
- cy.wait(5000);
- cy.go(-1)
- cy.wait(5000);
- cy.reload();
- cy.wait(5000);
- homePage.ImportApp(resolvedJsonFilePath);
- cy.wait(5000);
+ ).should('be.ok');
+
+ homePage.ImportApp(user.jsonFilePath)
+ .should('be.visible')
+ .and('contain', 'Import complete');
+
cy.LogOut(false);
- cy.wait(5000);
+ .should('be.ok');
});
});Committable suggestion skipped: line range outside the PR's diff.
| let testData: any; | ||
|
|
||
| // Read the CSV file before running the tests | ||
| before(() => { | ||
| cy.readFile( | ||
| "/Users/sagarkhalasi/codebase/appsmith/app/client/cypress/fixtures/testuserimport.csv", | ||
| ).then((csvData) => { | ||
| const rows = csvData.split("\n"); // Split by newlines to get each row | ||
| const headers = rows[0].split(","); // The first row contains headers | ||
| console.log("Headers:", headers); // Debug headers | ||
| testData = []; | ||
| rows.slice(1).forEach((row) => { | ||
| const values = row.split(","); | ||
| console.log("Row data:", values); // Debug row data | ||
| const userObject: any = {}; | ||
| headers.forEach((header, index) => { | ||
| const value = values[index]?.trim() || ""; // Avoid undefined value by providing a default empty string | ||
| userObject[header.trim()] = value; | ||
| }); | ||
| testData.push(userObject); // Add user object to the data array | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper type definitions and error handling for CSV parsing
The CSV parsing logic lacks proper TypeScript types and error handling.
+ interface UserData {
+ user_email: string;
+ password: string;
+ jsonFilePath: string;
+ }
- let testData: any;
+ let testData: UserData[];
before(() => {
- cy.readFile(
+ cy.fixture("testuserimport.csv").then((csvData: string) => {
const rows = csvData.split("\n");
const headers = rows[0].split(",");
- console.log("Headers:", headers);
testData = [];
- rows.slice(1).forEach((row) => {
+ rows.slice(1).filter(Boolean).forEach((row) => {
const values = row.split(",");
- console.log("Row data:", values);
- const userObject: any = {};
+ const userObject: Partial<UserData> = {};
headers.forEach((header, index) => {
const value = values[index]?.trim() || "";
userObject[header.trim()] = value;
});
- testData.push(userObject);
+ testData.push(userObject as UserData);
});
});
});📝 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.
| let testData: any; | |
| // Read the CSV file before running the tests | |
| before(() => { | |
| cy.readFile( | |
| "/Users/sagarkhalasi/codebase/appsmith/app/client/cypress/fixtures/testuserimport.csv", | |
| ).then((csvData) => { | |
| const rows = csvData.split("\n"); // Split by newlines to get each row | |
| const headers = rows[0].split(","); // The first row contains headers | |
| console.log("Headers:", headers); // Debug headers | |
| testData = []; | |
| rows.slice(1).forEach((row) => { | |
| const values = row.split(","); | |
| console.log("Row data:", values); // Debug row data | |
| const userObject: any = {}; | |
| headers.forEach((header, index) => { | |
| const value = values[index]?.trim() || ""; // Avoid undefined value by providing a default empty string | |
| userObject[header.trim()] = value; | |
| }); | |
| testData.push(userObject); // Add user object to the data array | |
| }); | |
| }); | |
| }); | |
| interface UserData { | |
| user_email: string; | |
| password: string; | |
| jsonFilePath: string; | |
| } | |
| let testData: UserData[]; | |
| // Read the CSV file before running the tests | |
| before(() => { | |
| cy.fixture("testuserimport.csv").then((csvData: string) => { | |
| const rows = csvData.split("\n"); // Split by newlines to get each row | |
| const headers = rows[0].split(","); // The first row contains headers | |
| testData = []; | |
| rows.slice(1).filter(Boolean).forEach((row) => { | |
| const values = row.split(","); | |
| const userObject: Partial<UserData> = {}; | |
| headers.forEach((header, index) => { | |
| const value = values[index]?.trim() || ""; // Avoid undefined value by providing a default empty string | |
| userObject[header.trim()] = value; | |
| }); | |
| testData.push(userObject as UserData); | |
| }); | |
| }); | |
| }); |
app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/PropertyControl_spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/uiToCode_spec.ts
Show resolved
Hide resolved
|
@ApekshaBhosale Please review |
github-actions
bot
added
Stability Issue
## Description Updating url Fixes appsmithorg#37266 _or_ Fixes `Issue URL` > [!WARNING] > _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 <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11716775460> > Commit: ba22e75 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11716775460&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 07 Nov 2024 08:19:42 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new test suite for creating workspaces, inviting users, and validating roles. - **Bug Fixes** - Updated URLs in various test cases to ensure accurate validation of API interactions and user flows. - **Tests** - Enhanced test coverage by modifying existing tests to reflect new API endpoints and validating expected behaviors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Updating url
Fixes #37266
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/11716775460
Commit: ba22e75
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 07 Nov 2024 08:19:42 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit