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

Add e2e my-posts page tests for delete interactions #1209

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

Add e2e my-posts page tests for delete interactions #1209

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6cd0e80
test: add e2e tests for managing delete modal interactions (#1169)
patrickhladun Nov 2, 2024
f85a64d
feat(utils): add createArticle function for setting up test articles
patrickhladun Nov 4, 2024
66e7450
feat: enhance test setup with additional articles in setup.ts
patrickhladun Nov 4, 2024
b5817df
fix: improve selectors and function names, update delete test
patrickhladun Nov 4, 2024
30d566f
Merge branch 'develop' into deleting-published-posts-tests
NiallJoeMaher Nov 4, 2024
e33431b
fix: code style issue in types/types.ts
patrickhladun Nov 4, 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
62 changes: 62 additions & 0 deletions e2e/my-posts.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
import type { Page } from "@playwright/test";
import test, { expect } from "@playwright/test";
import { articleExcerpt, loggedInAsUserOne } from "./utils";

async function openPublishedTab(page: Page) {
await page.goto("http://localhost:3000/my-posts");
await page.getByRole("link", { name: "Published" }).click();
await expect(page).toHaveURL(/\/my-posts\?tab=published$/);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this might create flakiness if there is a delay in clicking the button and the url being updated with the published tab query.

we can add something like
await page.waitForURL('url pattern');
then assert the url

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for this, @pkspyder007; this is added, too.

}

async function openDeleteModal(page: Page) {
await page.getByRole("button", { name: "Options" }).click();
const optionsDiv = page.locator(
"div[aria-labelledby='headlessui-menu-button-:r5:']",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: can we do a little readable locator instead of this headless ui dependent one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is updated @pkspyder007 and looks much cleaner, thanks for the tip.

);
await expect(optionsDiv).toBeVisible();
const deleteButton = optionsDiv.locator("text=Delete");
await deleteButton.click();
const confirmationDiv = page.getByText(
"Are you sure you want to delete this article?",
);
await expect(confirmationDiv).toBeVisible();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve selector reliability and maintainability.

The current implementation has a few potential issues:

  1. Hardcoded aria-labelledby selector is brittle and may break with UI framework updates
  2. Hardcoded text content should be externalized for easier maintenance
  3. Missing timeout configurations for visibility checks

Consider these improvements:

+const SELECTORS = {
+  OPTIONS_MENU: '[role="menu"]',
+  DELETE_CONFIRMATION: 'div[role="dialog"]'
+};
+
+const TEXT = {
+  DELETE_CONFIRMATION: 'Are you sure you want to delete this article?'
+};
+
 async function openDeleteModal(page: Page) {
   await page.getByRole("button", { name: "Options" }).click();
-  const optionsDiv = page.locator(
-    "div[aria-labelledby='headlessui-menu-button-:r5:']",
-  );
+  const optionsDiv = page.locator(SELECTORS.OPTIONS_MENU);
   await expect(optionsDiv).toBeVisible();
   const deleteButton = optionsDiv.locator("text=Delete");
   await deleteButton.click();
-  const confirmationDiv = page.getByText(
-    "Are you sure you want to delete this article?",
-  );
-  await expect(confirmationDiv).toBeVisible();
+  const confirmationDiv = page.locator(SELECTORS.DELETE_CONFIRMATION);
+  await expect(confirmationDiv).toBeVisible({ timeout: 5000 });
+  await expect(confirmationDiv).toHaveText(TEXT.DELETE_CONFIRMATION);
 }
📝 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
async function openDeleteModal(page: Page) {
await page.getByRole("button", { name: "Options" }).click();
const optionsDiv = page.locator(
"div[aria-labelledby='headlessui-menu-button-:r5:']",
);
await expect(optionsDiv).toBeVisible();
const deleteButton = optionsDiv.locator("text=Delete");
await deleteButton.click();
const confirmationDiv = page.getByText(
"Are you sure you want to delete this article?",
);
await expect(confirmationDiv).toBeVisible();
}
const SELECTORS = {
OPTIONS_MENU: '[role="menu"]',
DELETE_CONFIRMATION: 'div[role="dialog"]'
};
const TEXT = {
DELETE_CONFIRMATION: 'Are you sure you want to delete this article?'
};
async function openDeleteModal(page: Page) {
await page.getByRole("button", { name: "Options" }).click();
const optionsDiv = page.locator(SELECTORS.OPTIONS_MENU);
await expect(optionsDiv).toBeVisible();
const deleteButton = optionsDiv.locator("text=Delete");
await deleteButton.click();
const confirmationDiv = page.locator(SELECTORS.DELETE_CONFIRMATION);
await expect(confirmationDiv).toBeVisible({ timeout: 5000 });
await expect(confirmationDiv).toHaveText(TEXT.DELETE_CONFIRMATION);
}


test.describe("Unauthenticated my-posts Page", () => {
test("Unauthenticated users should be redirected to get-started page if they access my-posts directly", async ({
page,
Expand Down Expand Up @@ -53,4 +74,45 @@ test.describe("Authenticated my-posts Page", () => {
).toBeVisible();
await expect(page.getByText(articleExcerpt, { exact: true })).toBeVisible();
});

test("User should close delete modal with Cancel button", async ({
page,
}) => {
await page.goto("http://localhost:3000/my-posts");
await openPublishedTab(page);
await openDeleteModal(page);

const closeButton = page.getByRole("button", { name: "Cancel" });
await closeButton.click();

await expect(
page.locator("text=Are you sure you want to delete this article?"),
).toBeHidden();
});

test("User should close delete modal with Close button", async ({ page }) => {
await page.goto("http://localhost:3000/my-posts");
await openPublishedTab(page);
await openDeleteModal(page);

const closeButton = page.getByRole("button", { name: "Close" });
await closeButton.click();

await expect(
page.locator("text=Are you sure you want to delete this article?"),
).toBeHidden();
});

test("User should delete published article", async ({ page }) => {
await page.goto("http://localhost:3000/my-posts");
await openPublishedTab(page);
await openDeleteModal(page);

const closeButton = page.getByRole("button", { name: "Delete" });
await closeButton.click();

await expect(
page.getByRole("link", { name: "/articles/e2e-test-slug-published" }),
).toHaveCount(0);
});
});
Loading