Skip to content
6 changes: 4 additions & 2 deletions app/(app)/settings/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,12 @@ const Settings = ({ profile }: { profile: User }) => {

useEffect(() => {
if (isSuccess) {
toast.success("Saved");
toast.success("Saved", { className: "toast-success" });
}
if (isError) {
toast.error("Something went wrong saving settings.");
toast.error("Something went wrong saving settings.", {
className: "toast-error",
});
}
}, [isError, isSuccess]);

Expand Down
38 changes: 35 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", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this to authenticated settings page. Because the user is authenticated for these tests

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'm not sure exactly what you're saying here?

Did you want a separate file for auth vs. un-auth tests, because doesn't a user have to be logged in to edit their profile?

I guess I could use more detail on this πŸ‘ŒπŸ»

Copy link
Contributor

Choose a reason for hiding this comment

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

Your new test is ran inside a test suite called 'Unauthenticated setttings Page'

But this is an authenticated test because users need to login to change their username.

If you just change the name of the test it will be fixed

Expand All @@ -10,6 +10,38 @@ 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();
const toastError = page.locator('.toast-success');
await expect(toastError).toBeVisible();
await expect(toastError).toBeHidden();

// Reload the page and check that the input field has the correct value
await page.reload();
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

});
});
2 changes: 1 addition & 1 deletion e2e/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const setup = async () => {
image: `https://robohash.org/${encodeURIComponent(name)}?bgset=bg1`,
location: "Ireland",
bio: "Hi I am an robot",
websiteUrl: "codu.co",
websiteUrl: "https://codu.co",
};
const [createdUser] = await db.insert(user).values(userData).returning();
return createdUser;
Expand Down
Loading