-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Feat/improving existing e2e tests #1125
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { test, expect } from "@playwright/test"; | ||
|
||
test.describe("Accessibility Tests", () => { | ||
test.describe("Confirm all images on homepage have alt text", () => { | ||
test("Shared content", async ({ page }) => { | ||
const imagesWithoutAltText = await page.$$eval( | ||
"img:not([alt])", | ||
(images) => images.length, | ||
); | ||
expect(imagesWithoutAltText).toBe(0); // All images should have alt text | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,99 @@ | ||
import { test, expect } from "playwright/test"; | ||
|
||
test.describe("Articles", () => { | ||
test.describe("Unauthenticated Articles Page", () => { | ||
test.beforeEach(async ({ page }) => { | ||
await page.context().clearCookies(); | ||
}); | ||
|
||
test("Should show popular tags", async ({ page, isMobile }) => { | ||
await page.goto("http://localhost:3000/articles"); | ||
await expect( | ||
page.getByRole("heading", { name: "Popular topics" }), | ||
).toBeVisible({ visible: !isMobile }); | ||
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct improper usage of The Apply this diff to fix the assertions: // For lines 11-12
- ).toBeVisible({ visible: !isMobile });
+ if (isMobile) {
+ await expect(
+ page.getByRole("heading", { name: "Popular topics" }),
+ ).not.toBeVisible();
+ } else {
+ await expect(
+ page.getByRole("heading", { name: "Popular topics" }),
+ ).toBeVisible();
+ }
// For lines 15-16
- ).toBeVisible({ visible: !isMobile });
+ if (isMobile) {
+ await expect(
+ page.getByRole("link", { name: '"Codú Writing Challenge" text' }),
+ ).not.toBeVisible();
+ } else {
+ await expect(
+ page.getByRole("link", { name: '"Codú Writing Challenge" text' }),
+ ).toBeVisible();
+ }
// For lines 67-68
- ).toBeVisible({ visible: !isMobile });
+ if (isMobile) {
+ await expect(
+ page.getByRole("heading", { name: "Popular topics" }),
+ ).not.toBeVisible();
+ } else {
+ await expect(
+ page.getByRole("heading", { name: "Popular topics" }),
+ ).toBeVisible();
+ }
// For lines 71-72
- ).toBeVisible({ visible: !isMobile });
+ if (isMobile) {
+ await expect(
+ page.getByRole("link", { name: '"Codú Writing Challenge" text' }),
+ ).not.toBeVisible();
+ } else {
+ await expect(
+ page.getByRole("link", { name: '"Codú Writing Challenge" text' }),
+ ).toBeVisible();
+ } Also applies to: 15-16, 67-68, 71-72 |
||
|
||
await expect( | ||
page.getByRole("link", { name: '"Codú Writing Challenge" text' }), | ||
).toBeVisible({ visible: !isMobile }); | ||
}); | ||
|
||
test("Should not show bookmark article icon", async ({ page }) => { | ||
await page.goto("http://localhost:3000/articles"); | ||
|
||
await expect( | ||
page.getByRole("heading", { name: "Recent bookmarks" }), | ||
).toBeHidden(); | ||
|
||
await expect( | ||
page.locator("article").first().getByLabel("Bookmark this post"), | ||
).toBeHidden(); | ||
}); | ||
test("Should load more articles when scrolling to the end of the page", async ({ | ||
page, | ||
isMobile, | ||
}) => { | ||
await page.goto("http://localhost:3000/articles"); | ||
// Waits for articles to be loaded | ||
await page.waitForSelector("article"); | ||
|
||
const initialArticleCount = await page.$$eval( | ||
"article", | ||
(articles) => articles.length, | ||
); | ||
|
||
if (!isMobile) { | ||
await page.getByText("Code Of Conduct").scrollIntoViewIfNeeded(); | ||
await page.waitForTimeout(5000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid using fixed delays with Using Apply this diff to improve the test: - await page.waitForTimeout(5000);
+ await page.waitForLoadState('networkidle'); Alternatively, wait for a specific element that appears after loading more articles: - await page.waitForTimeout(5000);
+ await page.waitForSelector("article:nth-child(${initialArticleCount + 1})");
|
||
const finalArticleCount = await page.$$eval( | ||
"article", | ||
(articles) => articles.length, | ||
); | ||
expect(finalArticleCount).toBeGreaterThan(initialArticleCount); | ||
} | ||
|
||
await expect(page.getByText("Home")).toBeVisible(); | ||
await expect( | ||
page.getByLabel("Footer").getByRole("link", { name: "Events" }), | ||
).toBeVisible(); | ||
await expect(page.getByText("Sponsorship")).toBeVisible(); | ||
await expect(page.getByText("Code Of Conduct")).toBeVisible(); | ||
}); | ||
}); | ||
|
||
test.describe("Authenticated Articles Page", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set up authentication context in authenticated tests The authenticated tests need to ensure that the user is authenticated before running the tests. Currently, there is no Consider adding a test.beforeEach(async ({ page }) => {
// Replace with your authentication logic
await page.goto("http://localhost:3000/login");
await page.fill('input[name="email"]', 'user@example.com');
await page.fill('input[name="password"]', 'password');
await page.click('button[type="submit"]');
await page.waitForNavigation();
}); If you have a helper function or fixture for authentication, you can use that instead to keep your tests DRY. |
||
test("Should show recent bookmarks", async ({ page, isMobile }) => { | ||
await page.goto("http://localhost:3000/articles"); | ||
await expect( | ||
page.getByRole("heading", { name: "Popular topics" }), | ||
).toBeVisible({ visible: !isMobile }); | ||
|
||
await expect( | ||
page.getByRole("link", { name: '"Codú Writing Challenge" text' }), | ||
).toBeVisible({ visible: !isMobile }); | ||
|
||
await expect( | ||
page.getByRole("heading", { name: "Recent bookmarks" }), | ||
).toBeVisible({ visible: !isMobile }); | ||
}); | ||
|
||
test("Should show bookmark article icon", async ({ page, isMobile }) => { | ||
await page.goto("http://localhost:3000/articles"); | ||
await expect( | ||
page.getByRole("heading", { name: "Popular topics" }), | ||
).toBeVisible({ visible: !isMobile }); | ||
|
||
await expect( | ||
page.getByRole("link", { name: '"Codú Writing Challenge" text' }), | ||
).toBeVisible({ visible: !isMobile }); | ||
|
||
await expect( | ||
page.getByRole("heading", { name: "Recent bookmarks" }), | ||
).toBeVisible({ visible: !isMobile }); | ||
|
||
await expect( | ||
page.locator("article").first().getByLabel("Bookmark this post"), | ||
).toBeVisible(); | ||
}); | ||
|
||
test("Should load more articles when scrolling to the end of the page", async ({ | ||
page, | ||
isMobile, | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,23 +1,59 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { test, expect } from "playwright/test"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import "dotenv/config"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test.describe("Login Page", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("should display the welcome message", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto("http://localhost:3000/get-started"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const welcomeMessage = page.getByText("Sign in or create your accounttton"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(welcomeMessage).toBeTruthy(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("should display the Github login button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test.describe("Unauthenticated Login Page", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test.beforeEach(async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.context().clearCookies(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto("http://localhost:3000/get-started"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.waitForTimeout(3000); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Sign up page contains sign up links", async ({ page, isMobile }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByText("CodúBetaSign in or create")).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("heading", { name: "Sign in or create your account" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+10
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the expected text in the assertion The text Consider updating the assertion: -await expect(page.getByText("CodúBetaSign in or create")).toBeVisible();
+await expect(page.getByText("CodúBeta Sign in or create")).toBeVisible(); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByRole("link", { name: "return home" })).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!isMobile) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("button", { name: "Sign up for free" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("button", { name: "Sign in", exact: true }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Login page contains GitHub button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByTestId("github-login-button")).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("should display the Gitlab login button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.context().clearCookies(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto("http://localhost:3000/get-started"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.waitForLoadState(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Login page contains GitLab button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByTestId("gitlab-login-button")).toBeVisible(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test.describe("Authenticated Login Page", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Sign up page contains sign up links", async ({ page, isMobile }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// authenticated users are kicked back to the homepage if they try to go to /get-started | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto("http://localhost:3000/get-started"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(page.url()).toEqual("http://localhost:3000/"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Await the URL assertion to ensure navigation completion The assertion Update the code to use -expect(page.url()).toEqual("http://localhost:3000/");
+await expect(page).toHaveURL("http://localhost:3000/"); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByText("CodúBetaSign in or create")).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("heading", { name: "Sign in or create your account" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+38
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the expected text in the assertion Similarly, in the authenticated tests, the text Update the assertion: -await expect(page.getByText("CodúBetaSign in or create")).toBeHidden();
+await expect(page.getByText("CodúBeta Sign in or create")).toBeHidden(); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByRole("link", { name: "return home" })).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!isMobile) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("button", { name: "Sign up for free" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
page.getByRole("button", { name: "Sign in", exact: true }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+34
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename the test to reflect its purpose The test Consider renaming the test for clarity: -test("Sign up page contains sign up links", async ({ page, isMobile }) => {
+test("Authenticated users are redirected from get-started page", async ({ page, isMobile }) => { 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Login page contains GitHub button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByTestId("github-login-button")).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
test("Login page contains GitLab button", async ({ page }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await expect(page.getByTestId("gitlab-login-button")).toBeHidden(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); |
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.
🛠️ Refactor suggestion
Consider enhancing the test case implementation.
The test case effectively checks for alt text on images, which is crucial for accessibility. However, there are a couple of points to consider:
The test doesn't navigate to the homepage before performing the check. Consider adding a
page.goto()
call at the beginning of the test.The test title "Shared content" might not be the most descriptive for this specific check. A more explicit title like "All images should have alt text" would better describe the test's purpose.
Here's a suggested improvement:
This change ensures the test is performed on the homepage and provides a more descriptive test title.