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.

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

UI Tests for username change on /settings page. #1203

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
32 changes: 29 additions & 3 deletions e2e/settings.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import test from "@playwright/test";
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {
Expand All @@ -10,6 +10,32 @@ test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users

// Test for changing username
test('Username input field', async ({ page }) => {
await page.goto('http://localhost:3000/settings', { timeout: 30000 });

Copy link
Contributor

@coderabbitai coderabbitai bot Oct 31, 2024

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Improve test configuration and navigation

Consider these improvements:

  1. The 30-second timeout seems excessive for local development
  2. The hardcoded URL should be replaced with a configurable base URL
- await page.goto('http://localhost:3000/settings', { timeout: 30000 });
+ await page.goto('/settings');

Add this to your playwright.config.ts:

use: {
  baseURL: process.env.BASE_URL || 'http://localhost:3000',
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

// Wait for the username input field to be visible
await page.locator('input[id="username"]').waitFor();

// Test that the input field is visible and has the correct attributes
const inputField = page.locator('input[id="username"]');
await expect(inputField).toBeVisible();
await expect(inputField).toHaveAttribute('type', 'text');
await expect(inputField).toHaveAttribute('autocomplete', 'username');

// Test that the error message appears when the input field is invalid
await inputField.fill('45&p^x#@!96%*()');
await page.locator('button[type="submit"]').click();
const errorMessage = page.locator('p:text-is("Username can only contain alphanumerics and dashes.")')
await expect(errorMessage).toBeVisible();
await expect(errorMessage).toHaveText('Username can only contain alphanumerics and dashes.');
// Reset the form
await page.locator('button:has-text("Reset")').click();

// Test that the input field can be filled with a valid value and saves it
await inputField.fill('codu-rules');
await page.locator('button[type="submit"]').click();
await expect(inputField).toHaveValue('codu-rules');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this wait for the username change to take effect?

If you fill in the new username and then click submit.

Will this test see the new username you have entered as the value of the input and complete straight away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean the tests pass for it. But it could be just catching the still filled in form from before the reset.

Would you like me to change it to a page refresh instead?

Or would you like some kind of timeOut in-between the state change and checking for the change to persist?

I would be happy to add either one πŸ‘πŸ»

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I am not sure is there a loading indicator for these types of change? It would be nice to see

  1. Click submit
  2. Loading indicator appears
  3. Loading indicator hidden
  4. Check new username has taken effect

});
Loading