Skip to content

Test for location input on /settings page. #1204

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

Open
petercr wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Test for location input on /settings page. #1204

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
67c5c4f
feat: UI tests for username change on /settings
petercr Oct 31, 2024
e6883a5
Merge branch 'codu-code:develop' into develop
petercr Oct 31, 2024
8724657
Test to see if input is showing, working, and update persits.
petercr Nov 1, 2024
ed62c79
Merge branch 'develop' into test-for-location-input
CarolinaCobo Nov 12, 2024
4699c2b
fix: fixed typo and added test for max length on location input.
petercr Nov 14, 2024
1fad2e0
fix: added assertion for `#location` and test after page refresh.
petercr Nov 17, 2024
2df741c
Merge branch 'develop' into test-for-location-input
CarolinaCobo Feb 12, 2025
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
55 changes: 52 additions & 3 deletions e2e/settings.spec.ts
View file
Open in desktop
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,55 @@ 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 });
Comment thread
petercr marked this conversation as resolved.
Outdated

// 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.');
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify error message assertion

The error message assertion is duplicated. Also, consider testing additional validation rules like minimum/maximum length constraints.

-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.');
+const errorMessage = page.getByText('Username can only contain alphanumerics and dashes.');
+await expect(errorMessage).toBeVisible();

+// Add tests for length constraints
+await inputField.fill('a');  // Too short
+await page.locator('button[type="submit"]').click();
+await expect(page.getByText('Username must be at least 3 characters')).toBeVisible();

Copy link
Copy Markdown
Contributor

@CarolinaCobo CarolinaCobo Nov 12, 2024

Choose a reason for hiding this comment

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

@petercr this seems like good points, could you add this changes? :) thanks!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 12, 2024

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!

Copy link
Copy Markdown
Contributor Author

@petercr petercr Nov 17, 2024

Choose a reason for hiding this comment

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

@CarolinaCobo I think I already did this on PR #1203
That PR handled the username input and testing.
But let me know if you want me to add anything there 👌

This one #1204 is focusing on the #location input.
I have tested for max length and its error, but there is no minimum length for location currently.

// 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');
Comment thread
petercr marked this conversation as resolved.
Outdated
});
Comment thread
petercr marked this conversation as resolved.

// Tests location input, autocomplete, and saved values
test('location input is visible', async ({page}) => {
// Test to see if input is visible
await page.locator('#location').isVisible();

// Test to fill if value can be changed
await page.locator('#location').fill('New York');
await expect(page.locator('#location')).toHaveValue('New York');

// Test to see if autocomplete is working
await expect(page.locator('#location')).toHaveAttribute('autocomplete', 'country-name');

// Test to see if change in location persits
await page.locator('#location').fill('A fun place to visit.');
await page.locator('button[type="submit"]').click();
await expect(page.locator('#location')).toHaveValue('A fun place to visit.');
});
Comment thread
petercr marked this conversation as resolved.
Outdated
Comment on lines +71 to 82
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 14, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing page reload in persistence test.

The test comment mentions verifying persistence after page reload, but the actual reload step is missing.

Add the reload step:

   await page.fill('input[id="location"]', "A fun place to visit.");
   await page.locator('button[type="submit"]').click();
   await expect(page.locator('input[id="location"]')).toHaveValue(
     "A fun place to visit.",
   );
+  // Verify persistence after reload
+  await page.reload();
+  await expect(page.locator('input[id="location"]')).toHaveValue(
+    "A fun place to visit.",
+  );
📝 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
// Test to see if change in location persists after submit and page reload
await page.fill('input[id="location"]', "A fun place to visit.");
await page.locator('button[type="submit"]').click();
await expect(page.locator('input[id="location"]')).toHaveValue(
"A fun place to visit.",
);
});
// Test to see if change in location persists after submit and page reload
await page.fill('input[id="location"]', "A fun place to visit.");
await page.locator('button[type="submit"]').click();
await expect(page.locator('input[id="location"]')).toHaveValue(
"A fun place to visit.",
);
// Verify persistence after reload
await page.reload();
await expect(page.locator('input[id="location"]')).toHaveValue(
"A fun place to visit.",
);
});






});