-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Sign in and Sign up cases #39028
Conversation
WalkthroughThis pull request introduces a new Cypress test file for verifying sign-in and sign-up flows. It adds two automated tests: one for user registration via the sign-up form and one for user authentication via sign-in. In addition, locator files for both login and signup pages are modified to support email input and facilitate element selection, and a new tag ( Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Runner
participant API as API
participant App as Application
participant UI as User Interface
%% Sign Up Flow
T->>API: Request logout via API
T->>UI: Visit home page
UI-->>T: Display "Forget Password" and sign-up link
T->>UI: Click sign-up link
UI-->>T: Redirect to sign-up page
T->>UI: Enter email and temporary password
T->>App: Submit sign-up form
App-->>T: Return consolidated sign-up data
T->>UI: Verify welcome page and expected text presence
sequenceDiagram
participant T as Test Runner
participant API as API
participant App as Application
participant UI as User Interface
%% Sign In Flow
T->>API: Request logout via API
T->>App: Invoke custom login function with email & password
App-->>T: Return successful login confirmation
T->>UI: Verify successful sign-in on dashboard
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
🪧 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
app/client/cypress/locators/SignupPage.json (1)
4-4
: Use data-testid for button selectorReplace type attribute selector with data-testid.
- "submitBtn": "button[type='submit']", + "submitBtn": "[data-testid='t--submit-button']",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/cypress/e2e/Regression/ClientSide/Login/SignInSignUp_spec.ts
(1 hunks)app/client/cypress/locators/LoginPage.json
(1 hunks)app/client/cypress/locators/SignupPage.json
(1 hunks)app/client/cypress/tags.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test ...
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/tags.js
app/client/cypress/locators/SignupPage.json
app/client/cypress/locators/LoginPage.json
app/client/cypress/e2e/Regression/ClientSide/Login/SignInSignUp_spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/cypress/tags.js (1)
57-57
: LGTM!The new tag follows the existing pattern and maintains alphabetical order.
cy.get(loginPage.submitBtn).click(); | ||
cy.wait("@getConsolidatedData"); | ||
agHelper.AssertURL("/signup-success?"); | ||
agHelper.AssertContains("Brand New"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using string assertions
Replace direct string assertion with data-testid or constants.
- agHelper.AssertContains("Brand New");
+ agHelper.AssertElementVisible(welcomePage.welcomeMessage);
📝 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.
agHelper.AssertContains("Brand New"); | |
agHelper.AssertElementVisible(welcomePage.welcomeMessage); |
it("1. User can sign up to application with form.", () => { | ||
homePage.LogOutviaAPI(); | ||
agHelper.VisitNAssert("/"); | ||
agHelper.WaitUntilEleAppear(SignupPageLocators.forgetPasswordLink); | ||
agHelper.AssertElementExist(SignupPageLocators.signupLink); | ||
agHelper.GetNClick(SignupPageLocators.signupLink); | ||
agHelper.AssertURL("/user/signup"); | ||
cy.get(loginPage.username).type(emailOne); | ||
cy.get(loginPage.password).type(tempPassword, { log: false }); | ||
cy.get(loginPage.submitBtn).click(); | ||
cy.wait("@getConsolidatedData"); | ||
agHelper.AssertURL("/signup-success?"); | ||
agHelper.AssertContains("Brand New"); | ||
agHelper.GetNClick(welcomePage.proficiencyGroupButton, 0); | ||
agHelper.GetNClick(welcomePage.useCaseGroupButton, 0); | ||
agHelper.ClickButton("Get started"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use SignupFromAPI instead of form-based signup
According to the coding guidelines, signup should be performed via API using SignupFromAPI.
- it("1. User can sign up to application with form.", () => {
- homePage.LogOutviaAPI();
- agHelper.VisitNAssert("/");
- agHelper.WaitUntilEleAppear(SignupPageLocators.forgetPasswordLink);
- agHelper.AssertElementExist(SignupPageLocators.signupLink);
- agHelper.GetNClick(SignupPageLocators.signupLink);
- agHelper.AssertURL("/user/signup");
- cy.get(loginPage.username).type(emailOne);
- cy.get(loginPage.password).type(tempPassword, { log: false });
- cy.get(loginPage.submitBtn).click();
- cy.wait("@getConsolidatedData");
- agHelper.AssertURL("/signup-success?");
- agHelper.AssertContains("Brand New");
- agHelper.GetNClick(welcomePage.proficiencyGroupButton, 0);
- agHelper.GetNClick(welcomePage.useCaseGroupButton, 0);
- agHelper.ClickButton("Get started");
+ it("1. User can sign up to application via API.", () => {
+ homePage.SignupFromAPI(emailOne, tempPassword);
+ // Verify post-signup flow
+ agHelper.AssertURL("/signup-success?");
+ agHelper.GetNClick(welcomePage.proficiencyGroupButton, 0);
+ agHelper.GetNClick(welcomePage.useCaseGroupButton, 0);
+ agHelper.ClickButton("Get started");
});
📝 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.
it("1. User can sign up to application with form.", () => { | |
homePage.LogOutviaAPI(); | |
agHelper.VisitNAssert("/"); | |
agHelper.WaitUntilEleAppear(SignupPageLocators.forgetPasswordLink); | |
agHelper.AssertElementExist(SignupPageLocators.signupLink); | |
agHelper.GetNClick(SignupPageLocators.signupLink); | |
agHelper.AssertURL("/user/signup"); | |
cy.get(loginPage.username).type(emailOne); | |
cy.get(loginPage.password).type(tempPassword, { log: false }); | |
cy.get(loginPage.submitBtn).click(); | |
cy.wait("@getConsolidatedData"); | |
agHelper.AssertURL("/signup-success?"); | |
agHelper.AssertContains("Brand New"); | |
agHelper.GetNClick(welcomePage.proficiencyGroupButton, 0); | |
agHelper.GetNClick(welcomePage.useCaseGroupButton, 0); | |
agHelper.ClickButton("Get started"); | |
}); | |
it("1. User can sign up to application via API.", () => { | |
homePage.SignupFromAPI(emailOne, tempPassword); | |
// Verify post-signup flow | |
agHelper.AssertURL("/signup-success?"); | |
agHelper.GetNClick(welcomePage.proficiencyGroupButton, 0); | |
agHelper.GetNClick(welcomePage.useCaseGroupButton, 0); | |
agHelper.ClickButton("Get started"); | |
}); |
cy.get(loginPage.username).type(emailOne); | ||
cy.get(loginPage.password).type(tempPassword, { log: false }); | ||
cy.get(loginPage.submitBtn).click(); | ||
cy.wait("@getConsolidatedData"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove cy.wait and use proper assertions
Replace cy.wait("@getConsolidatedData")
with proper assertions to validate the response.
- cy.wait("@getConsolidatedData");
+ cy.intercept("GET", "/api/v1/users/consolidated-data").as("getConsolidatedData");
+ cy.wait("@getConsolidatedData").then((interception) => {
+ expect(interception.response.statusCode).to.eq(200);
+ });
📝 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.wait("@getConsolidatedData"); | |
cy.intercept("GET", "/api/v1/users/consolidated-data").as("getConsolidatedData"); | |
cy.wait("@getConsolidatedData").then((interception) => { | |
expect(interception.response.statusCode).to.eq(200); | |
}); |
"signupLink":".t--sign-up" | ||
"username": "input[name='username'], input[name='email']", | ||
"password": "input[name='password']", | ||
"submitBtn": "button[type='submit']", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use data-testid for button selector
Replace type attribute selector with data-testid.
- "submitBtn": "button[type='submit']",
+ "submitBtn": "[data-testid='t--submit-button']",
📝 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.
"submitBtn": "button[type='submit']", | |
"submitBtn": "[data-testid='t--submit-button']", |
/ci-test-limit-count run_count=20 update_snapshot=false specs_to_run=cypress/e2e/Regression/ClientSide/Login/SignInSignUp_spec.ts |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/13153151578. |
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13153151578.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
## Description Added new cases Fixes # https://app.zenhub.com/workspaces/qa-63316faf86bb2e170ed2e46b/issues/gh/appsmithorg/appsmith/39027 ## Automation /ok-to-test tags="@tag.SignIn" ### 🔍 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/13152773763> > Commit: 6a16e3a > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13152773763&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.SignIn` > Spec: > <hr>Wed, 05 Feb 2025 09:48:35 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** - Enhanced authentication: Users can now log in using either a username or an email address. - Improved sign-up flow for a smoother and more accessible account creation experience. - **Tests** - Expanded automation ensuring robust and consistent sign-in and sign-up processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Added new cases
Fixes # https://app.zenhub.com/workspaces/qa-63316faf86bb2e170ed2e46b/issues/gh/appsmithorg/appsmith/39027
Automation
/ok-to-test tags="@tag.SignIn"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13152773763
Commit: 6a16e3a
Cypress dashboard.
Tags:
@tag.SignIn
Spec:
Wed, 05 Feb 2025 09:48:35 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests