Add e2e my-posts page tests for delete interactions #1209
Conversation
…#1169) - Implement tests to ensure the delete modal can be closed with both 'Cancel' and 'Close' buttons. - Add test to verify deletion of a published article through the delete modal. - Introduce utility functions `openPublishedTab` and `openDeleteModal` to streamline modal interactions in tests.
|
@patrickhladun is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request enhance the end-to-end testing for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
patrickhladun
had a problem deploying
to
production
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
e2e/my-posts.spec.ts (2)
5-9: Consider making the URL assertion more robust.The URL check is good, but consider using a constant for the base URL and a more specific assertion.
+const BASE_URL = 'http://localhost:3000'; + async function openPublishedTab(page: Page) { - await page.goto("http://localhost:3000/my-posts"); + await page.goto(`${BASE_URL}/my-posts`); await page.getByRole("link", { name: "Published" }).click(); - await expect(page).toHaveURL(/\/my-posts\?tab=published$/); + await expect(page).toHaveURL(`${BASE_URL}/my-posts?tab=published`); }
1-4: Add JSDoc comments for better documentation.Consider adding JSDoc comments to describe the utility functions and test cases, including:
- Purpose of each utility function
- Expected behavior
- Required test setup
Example:
/** * Opens the Published tab in the my-posts page and verifies the URL. * @param page - Playwright Page object * @throws Will throw an error if the tab is not visible or clickable */ async function openPublishedTab(page: Page) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
e2e/my-posts.spec.ts(2 hunks)
🔇 Additional comments (1)
e2e/my-posts.spec.ts (1)
78-117: Enhance test coverage with additional scenarios.
The current test cases cover the happy path, but consider adding:
- Error handling scenarios (e.g., network errors during delete)
- Multiple article deletion scenarios
- Verification of success/error notifications
- State persistence after page reload
Also, the delete confirmation test could be flaky without proper wait conditions.
For the delete test, consider adding a wait for the network request:
const closeButton = page.getByRole("button", { name: "Delete" });
- await closeButton.click();
+ await Promise.all([
+ page.waitForResponse(response =>
+ response.url().includes('/api/articles') &&
+ response.request().method() === 'DELETE'
+ ),
+ closeButton.click()
+ ]);Would you like me to help create additional test cases for these scenarios?
e2e/my-posts.spec.ts
Outdated
| 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(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve selector reliability and maintainability.
The current implementation has a few potential issues:
- Hardcoded aria-labelledby selector is brittle and may break with UI framework updates
- Hardcoded text content should be externalized for easier maintenance
- 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.
| 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); | |
| } |
e2e/my-posts.spec.ts
Outdated
| 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$/); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Thank you for this, @pkspyder007; this is added, too.
e2e/my-posts.spec.ts
Outdated
| async function openDeleteModal(page: Page) { | ||
| await page.getByRole("button", { name: "Options" }).click(); | ||
| const optionsDiv = page.locator( | ||
| "div[aria-labelledby='headlessui-menu-button-:r5:']", |
There was a problem hiding this comment.
nitpick: can we do a little readable locator instead of this headless ui dependent one
There was a problem hiding this comment.
That is updated @pkspyder007 and looks much cleaner, thanks for the tip.
patrickhladun
temporarily deployed
to
production
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
types/types.ts (2)
15-26: LGTM! Consider adding JSDoc documentation.The
Articleinterface structure is well-defined and includes all necessary fields for article management. Consider adding JSDoc comments to document the purpose of the interface and its fields for better maintainability.+/** + * Represents an article in the system. + */ export interface Article { + /** Unique identifier for the article */ id: string; title: string; slug: string; excerpt: string; body: string; likes: number; readTimeMins: number; + /** ISO date string when the article was published, null if unpublished */ published: string | null; updatedAt: string; userId: string; }
23-24: Consider using more specific types for date fields.The
publishedandupdatedAtfields could benefit from more specific typing to ensure they contain valid date strings.- published: string | null; - updatedAt: string; + published: `${string}T${string}Z` | null; // ISO 8601 date format + updatedAt: `${string}T${string}Z`; // ISO 8601 date formatAlternatively, if you're using Zod for runtime validation:
import { z } from 'zod'; export const ArticleSchema = z.object({ // ... other fields published: z.string().datetime().nullable(), updatedAt: z.string().datetime(), }); export type Article = z.infer<typeof ArticleSchema>;e2e/constants/constants.ts (1)
4-4: LGTM! Consider making the test data more unique.The change from Lorem ipsum to a more descriptive excerpt improves test readability and better reflects its purpose in testing published articles.
Consider making the test data more unique to prevent potential conflicts in tests:
-export const articleExcerpt = "This is an excerpt for a published article."; +export const articleExcerpt = "E2E Test: This is an excerpt for a published article [test-id: delete-interaction]";e2e/utils/utils.ts (1)
71-73: Consider implementing connection pooling for better performance.Creating a new database connection for each article creation is inefficient, especially when running multiple e2e tests that create test data.
Consider implementing a connection pool or a shared connection manager:
// utils/db.ts import postgres from 'postgres'; import { drizzle } from 'drizzle-orm/postgres-js'; let client: postgres.Sql | null = null; let db: ReturnType<typeof drizzle> | null = null; export const getTestDb = () => { if (!db) { client = postgres(getDatabaseConfig()); db = drizzle(client); } return db; }; export const closeTestDb = async () => { if (client) { await client.end(); client = null; db = null; } };Then update the tests to use this shared connection:
// In your test setup file beforeAll(async () => { // Setup database connection getTestDb(); }); afterAll(async () => { // Clean up database connection await closeTestDb(); });e2e/my-posts.spec.ts (1)
85-99: Enhance test maintainability and reliability.The delete modal test cases have similar setup code and assertions. Consider these improvements:
- Extract common setup into a
beforeEachhook- Use more specific assertions for modal visibility
- Add error messages to assertions
+const setupDeleteModalTest = async (page: Page, title = "Published Article") => { + await page.goto("http://localhost:3000/my-posts"); + await openTab(page, "Published"); + await openDeleteModal(page, title); + return title; +}; test("User should close delete modal with Cancel button", async ({ page, }) => { - const title = "Published Article"; - await page.goto("http://localhost:3000/my-posts"); - await openTab(page, "Published"); - await openDeleteModal(page, title); + await setupDeleteModalTest(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(); + ).toBeHidden({ message: "Delete confirmation should be hidden after clicking Cancel" }); });Also applies to: 101-113
e2e/setup.ts (3)
33-36: Consider extracting the one-year offset as a constant.The date handling is clear and correct. However, for better maintainability, consider extracting the one-year offset to a named constant.
+const ONE_YEAR_OFFSET = 1; const now = new Date().toISOString(); const scheduled = new Date( - new Date().setFullYear(new Date().getFullYear() + 1), + new Date().setFullYear(new Date().getFullYear() + ONE_YEAR_OFFSET), ).toISOString();
38-148: Consider organizing test data for better maintainability.While the test data is comprehensive and well-structured, consider these improvements for better maintainability:
- Extract the test articles to a separate constants file
- Group articles by their state (published, scheduled, draft)
- Add JSDoc comments to document the purpose of each test article
Example structure:
// e2e/test-data/articles.ts interface TestArticle extends Article { /** Documents the testing scenario this article is used for */ purpose: string; } export const PUBLISHED_ARTICLES: TestArticle[] = [ { id: nanoid(8), title: "Published Article", // ... other fields purpose: "Basic published article for delete interaction testing" } ]; export const SCHEDULED_ARTICLES: TestArticle[] = [ ... ]; export const DRAFT_ARTICLES: TestArticle[] = [ ... ];
150-181: Consider adding error logging for failed insertions.The parallel insertion logic using Promise.all is efficient and well-structured. However, consider adding error logging to help debug test setup issues.
await Promise.all( articles.map( ({ id, title, // ... other fields }) => db .insert(post) .values({ id, title, // ... other fields }) .onConflictDoNothing() - .returning(), + .returning() + .catch(error => { + console.error(`Failed to insert article ${title}:`, error); + throw error; + }), ), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
e2e/constants/constants.ts(1 hunks)e2e/my-posts.spec.ts(2 hunks)e2e/setup.ts(3 hunks)e2e/utils/utils.ts(2 hunks)types/types.ts(1 hunks)
🔇 Additional comments (4)
e2e/my-posts.spec.ts (2)
1-6: LGTM! Clean imports and type definition.
The imports are well-organized and the TabName type provides good type safety for tab navigation.
16-24: 🛠️ Refactor suggestion
Improve selector reliability and maintainability.
The current implementation has potential issues:
- String concatenation in selectors is brittle
- Hardcoded text should be externalized
- Missing timeout configurations
+const SELECTORS = {
+ ARTICLE: (title: string) => `article:has-text("${title}")`,
+ DELETE_BUTTON: 'text="Delete"',
+ OPTIONS_BUTTON: 'button.dropdown-button'
+};
+const TEXT = {
+ DELETE_CONFIRMATION: 'Are you sure you want to delete this article?'
+};
async function openDeleteModal(page: Page, title: string) {
- const article = page.locator(`article:has-text("${title}")`);
+ const article = page.locator(SELECTORS.ARTICLE(title));
await expect(article).toBeVisible();
- await article.locator("button.dropdown-button").click();
- await article.locator('text="Delete"').click();
+ await article.locator(SELECTORS.OPTIONS_BUTTON).click();
+ await article.locator(SELECTORS.DELETE_BUTTON).click();
await expect(
- page.getByText("Are you sure you want to delete this article?"),
+ page.getByText(TEXT.DELETE_CONFIRMATION)
- ).toBeVisible();
+ ).toBeVisible({ timeout: 5000 });
}e2e/setup.ts (2)
15-15: LGTM! Good use of TypeScript type imports.
The addition of the Article type import enhances type safety for the articles array.
217-217: LGTM! Fixed grammatical error in bio text.
The correction from "an robot" to "a robot" fixes the grammar.
| import { post } from "@/server/db/schema"; | ||
| import { expect } from "@playwright/test"; | ||
| import type { Page } from "@playwright/test"; | ||
| import { drizzle } from "drizzle-orm/postgres-js"; | ||
| import postgres from "postgres"; | ||
| import { | ||
| E2E_USER_ONE_SESSION_ID, | ||
| E2E_USER_TWO_SESSION_ID, | ||
| E2E_USER_ONE_ID, | ||
| } from "../constants"; | ||
| import type { Article } from "@/types/types"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use environment variables for database configuration.
The database configuration should be moved to environment variables to follow security best practices and allow for different configurations across environments.
Consider creating a shared database configuration that uses environment variables:
// config/database.ts
export const getDatabaseConfig = () => ({
host: process.env.DB_HOST || '127.0.0.1',
port: parseInt(process.env.DB_PORT || '5432'),
database: process.env.DB_NAME || 'postgres',
username: process.env.DB_USER || 'postgres',
password: process.env.DB_PASSWORD || 'secret'
});| export async function createArticle({ | ||
| id, | ||
| title, | ||
| slug, | ||
| excerpt, | ||
| body, | ||
| likes = 10, | ||
| readTimeMins = 3, | ||
| published = new Date().toISOString(), | ||
| updatedAt = new Date().toISOString(), | ||
| userId = E2E_USER_ONE_ID, | ||
| }: Partial<Article>) { | ||
| const db = drizzle( | ||
| postgres("postgresql://postgres:secret@127.0.0.1:5432/postgres"), | ||
| ); | ||
|
|
||
| try { | ||
| await db | ||
| .insert(post) | ||
| .values({ | ||
| id, | ||
| title, | ||
| slug, | ||
| excerpt, | ||
| body, | ||
| likes, | ||
| readTimeMins, | ||
| published, | ||
| updatedAt, | ||
| userId, | ||
| } as Article) | ||
| .onConflictDoNothing() | ||
| .returning(); | ||
| } catch (err) { | ||
| throw Error("Error while creating E2E test article"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Several improvements needed for the createArticle function.
The function has multiple areas that could be improved for better type safety, resource management, and error handling:
Here's the suggested implementation:
-export async function createArticle({
+export async function createArticle(article: {
+ id: string;
+ title: string;
+ slug: string;
+ excerpt: string;
+ body: string;
likes?: number;
readTimeMins?: number;
published?: string;
updatedAt?: string;
userId?: string;
-}: Partial<Article>) {
- const db = drizzle(
+}): Promise<void> {
+ const client = postgres(getDatabaseConfig());
+ const db = drizzle(client);
+
+ try {
+ // Validate required fields
+ if (!article.id || !article.title || !article.slug || !article.excerpt || !article.body) {
+ throw new Error('Missing required fields');
+ }
+
+ const result = await db
.insert(post)
.values({
- id,
- title,
- slug,
- excerpt,
- body,
- likes,
- readTimeMins,
- published,
- updatedAt,
- userId,
- } as Article)
+ ...article,
+ likes: article.likes ?? 10,
+ readTimeMins: article.readTimeMins ?? 3,
+ published: article.published ?? new Date().toISOString(),
+ updatedAt: article.updatedAt ?? new Date().toISOString(),
+ userId: article.userId ?? E2E_USER_ONE_ID,
+ })
.onConflictDoNothing()
.returning();
+
+ if (!result.length) {
+ throw new Error('Article was not created');
+ }
} catch (err) {
- throw Error("Error while creating E2E test article");
+ throw new Error(`Failed to create test article: ${err.message}`);
+ } finally {
+ await client.end();
}
}Changes made:
- Replaced unsafe type assertion with explicit type definition
- Added validation for required fields
- Properly close database connection using
finallyblock - Enhanced error handling with specific error messages
- Added verification that the article was created
- Used object spread with default values instead of individual assignments
📝 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.
| export async function createArticle({ | |
| id, | |
| title, | |
| slug, | |
| excerpt, | |
| body, | |
| likes = 10, | |
| readTimeMins = 3, | |
| published = new Date().toISOString(), | |
| updatedAt = new Date().toISOString(), | |
| userId = E2E_USER_ONE_ID, | |
| }: Partial<Article>) { | |
| const db = drizzle( | |
| postgres("postgresql://postgres:secret@127.0.0.1:5432/postgres"), | |
| ); | |
| try { | |
| await db | |
| .insert(post) | |
| .values({ | |
| id, | |
| title, | |
| slug, | |
| excerpt, | |
| body, | |
| likes, | |
| readTimeMins, | |
| published, | |
| updatedAt, | |
| userId, | |
| } as Article) | |
| .onConflictDoNothing() | |
| .returning(); | |
| } catch (err) { | |
| throw Error("Error while creating E2E test article"); | |
| } | |
| } | |
| export async function createArticle(article: { | |
| id: string; | |
| title: string; | |
| slug: string; | |
| excerpt: string; | |
| body: string; | |
| likes?: number; | |
| readTimeMins?: number; | |
| published?: string; | |
| updatedAt?: string; | |
| userId?: string; | |
| }): Promise<void> { | |
| const client = postgres(getDatabaseConfig()); | |
| const db = drizzle(client); | |
| try { | |
| // Validate required fields | |
| if (!article.id || !article.title || !article.slug || !article.excerpt || !article.body) { | |
| throw new Error('Missing required fields'); | |
| } | |
| const result = await db | |
| .insert(post) | |
| .values({ | |
| ...article, | |
| likes: article.likes ?? 10, | |
| readTimeMins: article.readTimeMins ?? 3, | |
| published: article.published ?? new Date().toISOString(), | |
| updatedAt: article.updatedAt ?? new Date().toISOString(), | |
| userId: article.userId ?? E2E_USER_ONE_ID, | |
| }) | |
| .onConflictDoNothing() | |
| .returning(); | |
| if (!result.length) { | |
| throw new Error('Article was not created'); | |
| } | |
| } catch (err) { | |
| throw new Error(`Failed to create test article: ${err.message}`); | |
| } finally { | |
| await client.end(); | |
| } | |
| } |
| async function openTab(page: Page, tabName: TabName) { | ||
| await page.goto("http://localhost:3000/my-posts"); | ||
| await page.getByRole("link", { name: tabName }).click(); | ||
| const slug = tabName.toLowerCase(); | ||
| await page.waitForURL(`http://localhost:3000/my-posts?tab=${slug}`); | ||
| await expect(page).toHaveURL(new RegExp(`\\/my-posts\\?tab=${slug}`)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance URL verification reliability.
The URL verification could be flaky. Consider these improvements:
- Add explicit wait for navigation completion
- Simplify the RegExp pattern
async function openTab(page: Page, tabName: TabName) {
await page.goto("http://localhost:3000/my-posts");
await page.getByRole("link", { name: tabName }).click();
const slug = tabName.toLowerCase();
- await page.waitForURL(`http://localhost:3000/my-posts?tab=${slug}`);
- await expect(page).toHaveURL(new RegExp(`\\/my-posts\\?tab=${slug}`));
+ const expectedUrl = `http://localhost:3000/my-posts?tab=${slug}`;
+ await page.waitForURL(expectedUrl, { waitUntil: 'networkidle' });
+ await expect(page).toHaveURL(expectedUrl);
}📝 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.
| async function openTab(page: Page, tabName: TabName) { | |
| await page.goto("http://localhost:3000/my-posts"); | |
| await page.getByRole("link", { name: tabName }).click(); | |
| const slug = tabName.toLowerCase(); | |
| await page.waitForURL(`http://localhost:3000/my-posts?tab=${slug}`); | |
| await expect(page).toHaveURL(new RegExp(`\\/my-posts\\?tab=${slug}`)); | |
| } | |
| async function openTab(page: Page, tabName: TabName) { | |
| await page.goto("http://localhost:3000/my-posts"); | |
| await page.getByRole("link", { name: tabName }).click(); | |
| const slug = tabName.toLowerCase(); | |
| const expectedUrl = `http://localhost:3000/my-posts?tab=${slug}`; | |
| await page.waitForURL(expectedUrl, { waitUntil: 'networkidle' }); | |
| await expect(page).toHaveURL(expectedUrl); | |
| } |
| test("User should delete published article", async ({ page }) => { | ||
| const article = { | ||
| id: "test-id-for-deletion", | ||
| title: "Article to be deleted", | ||
| slug: "article-to-be-deleted", | ||
| excerpt: "This is an excerpt for the article to be deleted.", | ||
| body: "This is the body for the article to be deleted.", | ||
| }; | ||
| await createArticle(article); | ||
| await page.goto("http://localhost:3000/my-posts"); | ||
| await openTab(page, "Published"); | ||
| await expect(page.getByRole("link", { name: article.title })).toBeVisible(); | ||
| await openDeleteModal(page, article.title); | ||
|
|
||
| await page.getByRole("button", { name: "Delete" }).click(); | ||
| await expect(page.getByRole("link", { name: article.slug })).toHaveCount(0); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test data cleanup and improve assertions.
The deletion test should:
- Clean up test data after the test
- Use more specific assertions
- Add appropriate timeouts
test("User should delete published article", async ({ page }) => {
const article = {
id: "test-id-for-deletion",
title: "Article to be deleted",
slug: "article-to-be-deleted",
excerpt: "This is an excerpt for the article to be deleted.",
body: "This is the body for the article to be deleted.",
};
await createArticle(article);
+ try {
await page.goto("http://localhost:3000/my-posts");
await openTab(page, "Published");
- await expect(page.getByRole("link", { name: article.title })).toBeVisible();
+ await expect(
+ page.getByRole("link", { name: article.title })
+ ).toBeVisible({ timeout: 5000 });
await openDeleteModal(page, article.title);
await page.getByRole("button", { name: "Delete" }).click();
- await expect(page.getByRole("link", { name: article.slug })).toHaveCount(0);
+ await expect(
+ page.getByRole("link", { name: article.title })
+ ).toBeHidden({ timeout: 5000 });
+ } finally {
+ // Clean up test data
+ // TODO: Add cleanup logic here
+ }
});Would you like me to help implement the test data cleanup logic?
Committable suggestion skipped: line range outside the PR's diff.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
NiallJoeMaher
temporarily deployed
to
production
GitHub Actions
Inactive
patrickhladun
temporarily deployed
to
production
GitHub Actions
Inactive
NiallJoeMaher
temporarily deployed
to
production
GitHub Actions
Inactive
| }) | ||
| .onConflictDoNothing() | ||
| .returning(), | ||
| const articles: Article[] = [ |
There was a problem hiding this comment.
Check out the seed file in how we can generate data consistently. Will be a good follow up pr
✨ Codu Pull Request 💻
add more tests to #1169
Pull Request details
openTabandopenDeleteModalto streamline modal interactions in tests.Any Breaking changes
Associated Screenshots