Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactor Cypress Auth Test to POM and Improve Test Stability #9024

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
+100 −44
Open

Refactor Cypress Auth Test to POM and Improve Test Stability #9024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
80a8168
Add function to authorize user to register patient
JavidSumra Nov 5, 2024
18443a0
Merge branch 'develop' of github.com:JavidSumra/care_fe into issues/9…
JavidSumra Nov 6, 2024
8730d9a
Refactor Auth Test Suit
JavidSumra Nov 6, 2024
b01c10b
Remove Unnecessary files
JavidSumra Nov 6, 2024
052a052
Remove Unnecessary space
JavidSumra Nov 6, 2024
7315053
Merge branch 'develop' into issues/9015/refactor-cypress-auth-test
JavidSumra Nov 6, 2024
1960971
Merge branch 'develop' into issues/9015/refactor-cypress-auth-test
JavidSumra Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions cypress/e2e/auth_spec/ForgotPassword.cy.ts
View file
Open in desktop

This file was deleted.

23 changes: 0 additions & 23 deletions cypress/e2e/auth_spec/auth.cy.ts
View file
Open in desktop

This file was deleted.

45 changes: 45 additions & 0 deletions cypress/e2e/homepage_spec/UserLogin.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import LoginPage from "pageobject/Login/LoginPage";
const loginPage = new LoginPage();

describe("Authorisation/Authentication", () => {
beforeEach(() => {
cy.awaitUrl("/", true);
});

it("Try login as admin with correct password", () => {
loginPage.loginManuallyAsDistrictAdmin();
loginPage.interceptFacilityReq();
loginPage.verifyFacilityReq();
loginPage.ensureLoggedIn();
loginPage.clickSignOutBtn();
loginPage.verifyLoginPageUrl();
});

it("Try login as admin with incorrect password", () => {
loginPage.interceptLoginReq();
loginPage.loginManuallyAsDistrictAdmin(false);
loginPage.verifyLoginReq();
cy.verifyNotification("No active account found with the given credentials");
});
});
Comment on lines +4 to +24
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance test stability with retry and error handling.

While the test structure is good, consider these improvements for better stability:

  1. Add retry mechanism for flaky scenarios
  2. Implement proper error handling for API failures
 describe("Authorisation/Authentication", () => {
   beforeEach(() => {
     cy.awaitUrl("/", true);
   });

-  it("Try login as admin with correct password", () => {
+  it("Try login as admin with correct password", { retries: 2 }, () => {
     loginPage.loginManuallyAsDistrictAdmin();
-    loginPage.interceptFacilityReq();
+    // Add timeout and error handling for API calls
+    loginPage.interceptFacilityReq({ timeout: 10000 }).catch((err) => {
+      cy.log('Facility request failed:', err);
+      throw err;
+    });
     loginPage.verifyFacilityReq();
     loginPage.ensureLoggedIn();
     loginPage.clickSignOutBtn();
     loginPage.verifyLoginPageUrl();
   });

-  it("Try login as admin with incorrect password", () => {
+  it("Try login as admin with incorrect password", { retries: 2 }, () => {
     loginPage.interceptLoginReq();
     loginPage.loginManuallyAsDistrictAdmin(false);
     loginPage.verifyLoginReq();
     cy.verifyNotification("No active account found with the given credentials");
   });
 });

Committable suggestion skipped: line range outside the PR's diff.


describe("Forgot Password", () => {
const userName = "dummy_user_1";
beforeEach(() => {
cy.awaitUrl("/", true);
cy.verifyAndClickElement("#forgot-pass-btn", "Forgot password?");
});

it("Send Password Reset Link", () => {
loginPage.fillUserNameInForgotPasswordForm(userName);
loginPage.interceptResetLinkReq();
loginPage.clickSendResetLinkBtn();
loginPage.verifyResetLinkReq();
cy.verifyNotification("Password Reset Email Sent");
});

it("Go to Login page", () => {
loginPage.clickBackButton();
loginPage.verifyLoginPageUrl();
});
});
Comment on lines +26 to +45
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

🛠️ Refactor suggestion

Based on the analysis of the codebase, I can now provide the final response:

Consider using fixtures for test data and add validation coverage

The review comment is valid as the codebase shows:

  1. Most test data is currently hardcoded as constants across test files
  2. Limited input validation in test cases
  3. No negative test cases for the forgot password flow

However, the suggested implementation needs adjustment:

  • Instead of importing from testData.json, create a dedicated fixture file for auth-related test data
  • Add validation for both success and error scenarios
  • Include rate limiting test only if the API supports it (needs manual verification)
+// cypress/fixtures/auth.json
+{
+  "forgotPassword": {
+    "validUser": "dummy_user_1",
+    "invalidUser": "nonexistent@user",
+    "invalidFormat": "user@"
+  }
+}

 describe("Forgot Password", () => {
+  beforeEach(() => {
+    cy.fixture('auth.json').as('authData');
+    cy.awaitUrl("/", true);
+    cy.verifyAndClickElement("#forgot-pass-btn", "Forgot password?");
+  });

-  const userName = "dummy_user_1";
-
   it("Send Password Reset Link", () => {
-    loginPage.fillUserNameInForgotPasswordForm(userName);
+    cy.get('@authData').then((authData) => {
+      loginPage.fillUserNameInForgotPasswordForm(authData.forgotPassword.validUser);
+      cy.get('input[name="username"]')
+        .should('have.value', authData.forgotPassword.validUser)
+        .and('be.valid');
+      cy.get('button[type="submit"]').should('be.enabled');
+      loginPage.interceptResetLinkReq();
+      loginPage.clickSendResetLinkBtn();
+      loginPage.verifyResetLinkReq();
+      cy.verifyNotification("Password Reset Email Sent");
+    });
   });

+  it("Show error for invalid username format", () => {
+    cy.get('@authData').then((authData) => {
+      loginPage.fillUserNameInForgotPasswordForm(authData.forgotPassword.invalidFormat);
+      cy.get('input[name="username"]').should('have.class', 'error');
+      cy.get('button[type="submit"]').should('be.disabled');
+    });
+  });
🔗 Analysis chain

Move test data to fixtures and add input validation.

Consider these improvements:

  1. Move test data to a fixture file
  2. Add validation for the username input
  3. Add assertions for button states
+import testData from '../fixtures/testData.json';
+
 describe("Forgot Password", () => {
-  const userName = "dummy_user_1";
   beforeEach(() => {
     cy.awaitUrl("/", true);
     cy.verifyAndClickElement("#forgot-pass-btn", "Forgot password?");
   });

   it("Send Password Reset Link", () => {
-    loginPage.fillUserNameInForgotPasswordForm(userName);
+    loginPage.fillUserNameInForgotPasswordForm(testData.users.testUser.username);
+    // Validate input field
+    cy.get('input[name="username"]').should('have.value', testData.users.testUser.username);
+    // Verify button is enabled
+    cy.get('button[type="submit"]').should('be.enabled');
     loginPage.interceptResetLinkReq();
     loginPage.clickSendResetLinkBtn();
     loginPage.verifyResetLinkReq();
     cy.verifyNotification("Password Reset Email Sent");
   });

Also, consider adding negative test cases:

  1. Invalid username format
  2. Non-existent username
  3. Rate limiting scenarios

Let me check the implementation of these methods and validation patterns in other test files to make a more informed decision.

Let me check for existing test data patterns and negative test cases to make a complete assessment.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if similar validation exists in other test files
rg -l "fillUserNameInForgotPasswordForm|verifyNotification" --type ts

Length of output: 1074

Script:

#!/bin/bash
# Check implementation of fillUserNameInForgotPasswordForm and related validations
rg "fillUserNameInForgotPasswordForm" -A 5 --type ts

# Check if test data is used from fixtures in other tests
rg "fixtures.*\.json" --type ts

# Check for input validation patterns in other tests
ast-grep --pattern 'cy.get($SELECTOR).should("have.value", $_)'

Length of output: 1589

Script:

#!/bin/bash
# Check for existing test data patterns
rg "const \w+ = ['\"].*['\"]" cypress/e2e/ --type ts

# Check for negative test cases and validation patterns
rg "it\(.*invalid.*|it\(.*error.*" -A 5 cypress/e2e/ --type ts

# Check for existing fixture usage patterns
fd fixtures.ts$ -x cat {}

Length of output: 21386

0 cypress/e2e/auth_spec/redirect.cy.ts → cypress/e2e/homepage_spec/redirect.cy.ts
View file
Open in desktop
File renamed without changes.
56 changes: 52 additions & 4 deletions cypress/pageobject/Login/LoginPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,20 @@ class LoginPage {
cy.loginByApi("staffdev", "Coronasafe@123");
}

loginManuallyAsDistrictAdmin(): void {
loginManuallyAsDistrictAdmin(isCorrectCredentials: boolean = true): void {
cy.get("input[id='username']").type("devdistrictadmin");
cy.get("input[id='password']").type("Coronasafe@123");
cy.get("button").contains("Login").click();
if (isCorrectCredentials) {
cy.get("input[id='password']").type("Coronasafe@123");
} else {
cy.get("input[id='password']").type("Corona");
}
cy.submitButton("Login");
}

loginManuallyAsNurse(): void {
cy.get("input[id='username']").click().type("dummynurse1");
cy.get("input[id='password']").click().type("Coronasafe@123");
cy.get("button").contains("Login").click();
cy.submitButton("Login");
}

login(username: string, password: string): void {
Expand All @@ -34,6 +38,50 @@ class LoginPage {
cy.get("#sign-out-button").scrollIntoView();
cy.get("#sign-out-button").contains("Sign Out").should("exist");
}

clickSignOutBtn(): void {
cy.verifyAndClickElement("#sign-out-button", "Sign Out");
}

fillUserNameInForgotPasswordForm(userName: string): void {
cy.get("#forgot_username").type(userName);
}

clickSendResetLinkBtn(): void {
cy.verifyAndClickElement("#send-reset-link-btn", "Send Reset Link");
}

verifyLoginPageUrl(): void {
cy.url().should("include", "/");
}
Comment on lines +54 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance URL verification specificity.

The current URL verification is too broad. Consider checking for a more specific path.

-  verifyLoginPageUrl(): void {
-    cy.url().should("include", "/");
+  verifyLoginPageUrl(): void {
+    cy.url().should("include", "/login");
   }
📝 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.

Suggested change
verifyLoginPageUrl(): void {
cy.url().should("include", "/");
}
verifyLoginPageUrl(): void {
cy.url().should("include", "/login");
}


clickBackButton(): void {
cy.verifyAndClickElement("#back-to-login-btn", "Back to login");
}

interceptFacilityReq(): void {
cy.intercept("GET", "**/api/v1/facility/**").as("getFacilities");
}

verifyFacilityReq(): void {
cy.wait("@getFacilities").its("response.statusCode").should("eq", 200);
}

interceptLoginReq(): void {
cy.intercept("POST", "**/api/v1/auth/login").as("userLogin");
}

verifyLoginReq(): void {
cy.wait("@userLogin").its("response.statusCode").should("eq", 401);
}

interceptResetLinkReq(): void {
cy.intercept("POST", "**/api/v1/password_reset").as("resetLink");
}

verifyResetLinkReq(): void {
cy.wait("@resetLink").its("response.statusCode").should("eq", 200);
}
Comment on lines +62 to +84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance API interception methods for better flexibility and type safety.

The API interception methods could be improved in several ways:

  1. Use constants for status codes and URL patterns
  2. Add flexibility for different response scenarios
  3. Implement type safety for response handling

Example implementation:

// Constants
const API_ENDPOINTS = {
  FACILITY: '**/api/v1/facility/**',
  LOGIN: '**/api/v1/auth/login',
  PASSWORD_RESET: '**/api/v1/password_reset'
} as const;

const HTTP_STATUS = {
  OK: 200,
  UNAUTHORIZED: 401
} as const;

// Enhanced methods
interface ApiResponse {
  statusCode: number;
  body?: unknown;
}

interceptLoginReq(response?: Partial<ApiResponse>): this {
  cy.intercept('POST', API_ENDPOINTS.LOGIN, response).as('userLogin');
  return this;
}

verifyLoginReq(expectedStatus: number = HTTP_STATUS.UNAUTHORIZED): this {
  cy.wait('@userLogin')
    .its('response.statusCode')
    .should('eq', expectedStatus);
  return this;
}

}

export default LoginPage;
3 changes: 3 additions & 0 deletions src/components/Auth/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ const Login = (props: { forgot?: boolean }) => {
setForgotPassword(true);
}}
type="button"
id="forgot-pass-btn"
className="text-sm text-primary-400 hover:text-primary-500"
>
{t("forget_password")}
Expand Down Expand Up @@ -370,6 +371,7 @@ const Login = (props: { forgot?: boolean }) => {
setForgotPassword(false);
}}
type="button"
id="back-to-login-btn"
className="mb-4 text-sm text-primary-400 hover:text-primary-500"
>
<div className="flex justify-center">
Expand Down Expand Up @@ -403,6 +405,7 @@ const Login = (props: { forgot?: boolean }) => {
) : (
<button
type="submit"
id="send-reset-link-btn"
className="inline-flex w-full cursor-pointer items-center justify-center rounded bg-primary-500 px-4 py-2 text-sm font-semibold text-white"
>
{t("send_reset_link")}
Expand Down
Loading