Feat/adding test for replying to a comment#1183

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

Merged
NiallJoeMaher merged 6 commits into from
Oct 28, 2024
Merged

Feat/adding test for replying to a comment #1183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
02aabfe
chore: moving out from user auth out into a util
JohnAllenTech Oct 26, 2024
984b47d
chore: missed some updates in the README
JohnAllenTech Oct 26, 2024
168f007
chore: split out e2e user creation/seeding to support two e2e users i…
JohnAllenTech Oct 26, 2024
f023902
feat: added a test to check a user can reply to a comment
JohnAllenTech Oct 26, 2024
ce0cb8e
chore: removing a check to prevent a flaky test
JohnAllenTech Oct 26, 2024
420d9cf
Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment
JohnAllenTech Oct 28, 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
57 changes: 57 additions & 0 deletions e2e/articles.spec.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
);
});

test("Should be able to navigate directly to an article", async ({
page,
}) => {
await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
await expect(
page.getByRole("heading", { name: "Test Article" }),
).toBeVisible();
await expect(
page.getByRole("heading", { name: "Written by E2E Test User One" }),
).toBeVisible();
await expect(page.getByLabel("like-trigger")).toBeVisible();
await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
});

test("Should show bookmark article icon", async ({ page }) => {
await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

await expect(page.getByText(commentContent)).toBeVisible();
});

test("Should be able reply to a comment", async ({ page }) => {
await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
const numberOfCommentsIntially = await page
.locator("div")
.filter({ hasText: /^Thanks for the positive feedback!$/ })
.count();
await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
await expect(
page.getByRole("heading", { name: "Test Article" }),
).toBeVisible();
await expect(
page.getByRole("heading", { name: "Written by E2E Test User One" }),
).toBeVisible();
await expect(
page.getByRole("heading", { name: "Discussion (0)" }),
).toBeVisible();
await expect(page.getByLabel("like-trigger")).toBeVisible();
await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

await page.getByRole("button", { name: "Reply" }).first().click();

await page.locator("#reply").fill("Thanks for the positive feedback!");
await page.getByRole("button", { name: "Submit" }).nth(1).click();
await page.waitForTimeout(250);

await expect(
page.getByText("AUTHOR", { exact: true }).first(),
).toBeVisible();
const numberOfCommentsAfteringCommenting = await page
.locator("div")
.filter({ hasText: /^Thanks for the positive feedback!$/ })
.count();
expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
numberOfCommentsIntially,
);
await expect(
page
.getByRole("link", { name: "E2E Test User One", exact: true })
.nth(numberOfCommentsIntially + 1),
).toBeVisible();
});
});
65 changes: 65 additions & 0 deletions e2e/setup.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
if (
!process.env.DATABASE_URL ||
!process.env.E2E_USER_ONE_ID ||
!process.env.E2E_USER_TWO_ID
) {
throw new Error("Missing env variables for DB clean up script");
}
Comment on lines +8 to +15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling with specific missing variable information.

The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.

Consider implementing more detailed error reporting:

- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}

const db = drizzle(postgres(process.env.DATABASE_URL as string));

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add database connection error handling and cleanup.

The database connection should be properly handled with error catching and cleanup mechanisms.

Consider implementing connection handling:

- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }

Committable suggestion was skipped due to low confidence.


const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
const postId = "1nFnMmN5";
const now = new Date().toISOString();
await db
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();

await db
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
Comment on lines +18 to +50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test data management and validation.

Several improvements could make the test data setup more robust:

  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test

Consider these improvements:

 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};


try {
console.log("creating articles");

await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
console.log("DB setup successful");
} catch (err) {
console.log("Error while setting up DB before E2E test run", err);
}
Comment on lines +52 to +62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling and logging in the main execution block.

The current error handling and logging could be more informative for debugging test setup issues.

Consider enhancing the error handling:

 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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
try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}

};

export default setup;
1 change: 1 addition & 0 deletions playwright.config.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
* See https://playwright.dev/docs/test-configuration.
*/
export default defineConfig({
globalSetup: "./e2e/setup.ts",
globalTeardown: "./e2e/teardown.ts",
testDir: "e2e",
/* Run tests in files in parallel */
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Feat/adding test for replying to a comment#1183

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

 Merged

NiallJoeMaher
merged 6 commits into

from

Oct 28, 2024
 Merged


Feat/adding test for replying to a comment
#1183
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
02aabfe

chore: moving out from user auth out into a util


JohnAllenTech Oct 26, 2024
984b47d

chore: missed some updates in the README


JohnAllenTech Oct 26, 2024
168f007

chore: split out e2e user creation/seeding to support two e2e users i…


JohnAllenTech Oct 26, 2024
f023902

feat: added a test to check a user can reply to a comment


JohnAllenTech Oct 26, 2024
ce0cb8e

chore: removing a check to prevent a flaky test


JohnAllenTech Oct 26, 2024
420d9cf

Merge branch 'develop' into feat/adding-test-for-replying-to-a-comment


JohnAllenTech Oct 28, 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


57 changes: 57 additions & 0 deletions
e2e/articles.spec.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -14,6 +14,21 @@ test.describe("Unauthenticated Articles Page", () => {
 );
 });

 test("Should be able to navigate directly to an article", async ({
 page,
 }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();
 });

 test("Should show bookmark article icon", async ({ page }) => {
 await page.goto("http://localhost:3000/articles");

Expand DownExpand Up@@ -274,4 +289,46 @@ test.describe("Authenticated Articles Page", () => {

 await expect(page.getByText(commentContent)).toBeVisible();
 });

 test("Should be able reply to a comment", async ({ page }) => {
 await page.goto("http://localhost:3000/articles/e2e-test-slug-eqj0ozor");
 const numberOfCommentsIntially = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 await expect(page.getByText("Lorem ipsum dolor sit amet,")).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Test Article" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Written by E2E Test User One" }),
 ).toBeVisible();
 await expect(
 page.getByRole("heading", { name: "Discussion (0)" }),
 ).toBeVisible();
 await expect(page.getByLabel("like-trigger")).toBeVisible();
 await expect(page.getByLabel("bookmark-trigger")).toBeVisible();

 await page.getByRole("button", { name: "Reply" }).first().click();

 await page.locator("#reply").fill("Thanks for the positive feedback!");
 await page.getByRole("button", { name: "Submit" }).nth(1).click();
 await page.waitForTimeout(250);

 await expect(
 page.getByText("AUTHOR", { exact: true }).first(),
 ).toBeVisible();
 const numberOfCommentsAfteringCommenting = await page
 .locator("div")
 .filter({ hasText: /^Thanks for the positive feedback!$/ })
 .count();
 expect(numberOfCommentsAfteringCommenting).toBeGreaterThan(
 numberOfCommentsIntially,
 );
 await expect(
 page
 .getByRole("link", { name: "E2E Test User One", exact: true })
 .nth(numberOfCommentsIntially + 1),
 ).toBeVisible();
 });
});

65 changes: 65 additions & 0 deletions
e2e/setup.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
import dotenv from "dotenv";
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { post, comment } from "@/server/db/schema";

dotenv.config(); // Load .env file contents into process.env

export const setup = async () => {
 if (
 !process.env.DATABASE_URL ||
 !process.env.E2E_USER_ONE_ID ||
 !process.env.E2E_USER_TWO_ID
 ) {
 throw new Error("Missing env variables for DB clean up script");
 }

Comment on lines

+8
 to 
+15

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling with specific missing variable information.
The current error message doesn't indicate which specific environment variables are missing, making debugging more difficult.
Consider implementing more detailed error reporting:
- throw new Error("Missing env variables for DB clean up script");+ const missingVars = [+ ['DATABASE_URL', process.env.DATABASE_URL],+ ['E2E_USER_ONE_ID', process.env.E2E_USER_ONE_ID],+ ['E2E_USER_TWO_ID', process.env.E2E_USER_TWO_ID],+ ].filter(([name, value]) => !value).map(([name]) => name);+ throw new Error(`Missing required environment variables: ${missingVars.join(', ')}`);
📝 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

exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
thrownewError("Missing env variables for DB clean up script");
}
exportconstsetup=async()=>{
if(
!process.env.DATABASE_URL||
!process.env.E2E_USER_ONE_ID||
!process.env.E2E_USER_TWO_ID
){
constmissingVars=[
['DATABASE_URL',process.env.DATABASE_URL],
['E2E_USER_ONE_ID',process.env.E2E_USER_ONE_ID],
['E2E_USER_TWO_ID',process.env.E2E_USER_TWO_ID],
].filter(([name,value])=>!value).map(([name])=>name);
thrownewError(`Missing required environment variables: ${missingVars.join(', ')}`);
}
 const db = drizzle(postgres(process.env.DATABASE_URL as string));
@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

⚠️ Potential issue
Add database connection error handling and cleanup.
The database connection should be properly handled with error catching and cleanup mechanisms.
Consider implementing connection handling:
- const db = drizzle(postgres(process.env.DATABASE_URL as string));+ let sql;+ try {+ sql = postgres(process.env.DATABASE_URL as string);+ const db = drizzle(sql);+ return db;+ } catch (error) {+ console.error('Failed to establish database connection:', error);+ throw error;+ } finally {+ // Ensure to add cleanup in the main try-catch block+ // await sql?.end();+ }
Committable suggestion was skipped due to low confidence.

 const addE2EArticleAndComment = async (
 authorId: string,
 commenterId: string,
 ) => {
 const postId = "1nFnMmN5";
 const now = new Date().toISOString();
 await db
 .insert(post)
 .values({
 id: postId,
 published: now,
 excerpt: "Lorem ipsum dolor sit amet",
 updatedAt: now,
 slug: "e2e-test-slug-eqj0ozor",
 likes: 10,
 readTimeMins: 3,
 title: "Test Article",
 body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
 userId: authorId,
 })
 .onConflictDoNothing()
 .returning();

 await db
 .insert(comment)
 .values({
 postId,
 body: "What a great article! Thanks for sharing",
 userId: commenterId,
 })
 .onConflictDoNothing()
 .returning();
 };

Comment on lines

+18
 to 
+50

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Improve test data management and validation.
Several improvements could make the test data setup more robust:
  1. The hardcoded postId could cause conflicts in parallel test runs
  2. No validation that the insertions were successful
  3. The article body content is unnecessarily long for a test
Consider these improvements:
 const addE2EArticleAndComment = async (
authorId: string,
commenterId: string,
) => {
- const postId = "1nFnMmN5";+ const postId = `e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
const now = new Date().toISOString();
- await db+ const [insertedPost] = await db
.insert(post)
.values({
// ... other fields ...
- body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",+ body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
+ if (!insertedPost) {+ throw new Error(`Failed to insert test article with ID ${postId}`);+ }- await db+ const [insertedComment] = await db
.insert(comment)
// ... rest of the comment insertion ...
.returning();
+ if (!insertedComment) {+ throw new Error(`Failed to insert test comment for article ${postId}`);+ }++ return { postId, commentId: insertedComment.id };
};
📝 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

constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId="1nFnMmN5";
constnow=newDate().toISOString();
awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas vitae ipsum id metus vestibulum rutrum eget a diam. Integer eget vulputate risus, ac convallis nulla. Mauris sed augue nunc. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Nam congue posuere tempor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Ut ac augue non libero ullamcorper ornare. Ut commodo ligula vitae malesuada maximus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Etiam sagittis justo non justo placerat, a dapibus sapien volutpat. Nullam ullamcorper sodales justo sed.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
};
constaddE2EArticleAndComment=async(
authorId: string,
commenterId: string,
)=>{
constpostId=`e2e-test-${Date.now()}-${Math.random().toString(36).slice(2)}`;
constnow=newDate().toISOString();
const[insertedPost]=awaitdb
.insert(post)
.values({
id: postId,
published: now,
excerpt: "Lorem ipsum dolor sit amet",
updatedAt: now,
slug: "e2e-test-slug-eqj0ozor",
likes: 10,
readTimeMins: 3,
title: "Test Article",
body: "This is a test article content for E2E testing.",
userId: authorId,
})
.onConflictDoNothing()
.returning();
if(!insertedPost){
thrownewError(`Failed to insert test article with ID ${postId}`);
}
const[insertedComment]=awaitdb
.insert(comment)
.values({
postId,
body: "What a great article! Thanks for sharing",
userId: commenterId,
})
.onConflictDoNothing()
.returning();
if(!insertedComment){
thrownewError(`Failed to insert test comment for article ${postId}`);
}
return{ postId,commentId: insertedComment.id};
};

 try {
 console.log("creating articles");

 await addE2EArticleAndComment(
 process.env.E2E_USER_ONE_ID as string,
 process.env.E2E_USER_TWO_ID as string,
 );
 console.log("DB setup successful");
 } catch (err) {
 console.log("Error while setting up DB before E2E test run", err);
 }

Comment on lines

+52
 to 
+62

@coderabbitaicoderabbitaiBotOct 26, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

🛠️ Refactor suggestion
Enhance error handling and logging in the main execution block.
The current error handling and logging could be more informative for debugging test setup issues.
Consider enhancing the error handling:
 try {
- console.log("creating articles");+ console.log("[E2E Setup] Starting database initialization...");
await addE2EArticleAndComment(
process.env.E2E_USER_ONE_ID as string,
process.env.E2E_USER_TWO_ID as string,
);
- console.log("DB setup successful");+ console.log("[E2E Setup] Successfully initialized test data");
} catch (err) {
- console.log("Error while setting up DB before E2E test run", err);+ console.error("[E2E Setup] Failed to initialize test data:", err);+ throw err; // Re-throw to ensure test setup fails properly
}
📝 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

try{
console.log("creating articles");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("DB setup successful");
}catch(err){
console.log("Error while setting up DB before E2E test run",err);
}
try{
console.log("[E2E Setup] Starting database initialization...");
awaitaddE2EArticleAndComment(
process.env.E2E_USER_ONE_IDasstring,
process.env.E2E_USER_TWO_IDasstring,
);
console.log("[E2E Setup] Successfully initialized test data");
}catch(err){
console.error("[E2E Setup] Failed to initialize test data:",err);
throwerr;// Re-throw to ensure test setup fails properly
}
};

export default setup;

1 change: 1 addition & 0 deletions
playwright.config.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@ import { defineConfig, devices } from "@playwright/test";
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
 globalSetup: "./e2e/setup.ts",
 globalTeardown: "./e2e/teardown.ts",
 testDir: "e2e",
 /* Run tests in files in parallel */
Expand Down