Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e 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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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 \u003e 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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
, '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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,7 +46,7 @@ jobs:
NEXTAUTH_SECRET: ${{ secrets.NEXTAUTH_SECRET }}
E2E_USER_EMAIL: e2e@codu.co
E2E_USER_ID: 8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID: df8a11f2-f20a-43d6-80a0-a213f1efedc1

steps:
- name: Checkout repository
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,9 +141,9 @@ You shouldn't need to change the default value here. This is a variable used by
NEXTAUTH_URL=http://localhost:3000/api/auth
```

### E2E_USER_SESSION_ID
### E2E_USER_ONE_SESSION_ID

This is the sessionToken uuid that .
This is the sessionToken uuid that is used to identify a users current active session.
This is currently hardcoded and there is no reason to change this until we require multiple E2E test users within the same test suite

### E2E_USER_ID
Expand DownExpand Up@@ -173,7 +173,7 @@ Please ensure you have the following variables set in your `.env` file:

- `E2E_USER_ID`: The id of the E2E user for testing.
- `E2E_USER_EMAIL`: The email of the E2E user for testing.
- `E2E_USER_SESSION_ID`: The session id that the user will use to authenticate.
- `E2E_USER_ONE_SESSION_ID`: The session id that the user will use to authenticate.


Note the sample .env [here](./sample.env) is fine to use.
Expand Down
2 changes: 1 addition & 1 deletion drizzle/seed.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,7 @@ import postgres from "postgres";
const DATABASE_URL = process.env.DATABASE_URL || "";
// These can be removed in a follow on PR. Until this hits main we cant add E2E_USER_* stuff to the env.
const E2E_SESSION_ID =
process.env.E2E_USER_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
process.env.E2E_USER_ONE_SESSION_ID || "df8a11f2-f20a-43d6-80a0-a213f1efedc1";
const E2E_USER_ID =
process.env.E2E_USER_ID || "8e3179ce-f32b-4d0a-ba3b-234d66b836ad";
const E2E_USER_EMAIL = process.env.E2E_USER_EMAIL || "e2e@codu.co";
Expand Down
8 changes: 4 additions & 4 deletions e2e/articles.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
import { test, expect } from "playwright/test";
import { randomUUID } from "crypto";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});

test("Should show popular tags", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand DownExpand Up@@ -133,6 +130,9 @@ test.describe("Unauthenticated Articles Page", () => {
});

test.describe("Authenticated Articles Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Should show recent bookmarks", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/articles");
await expect(
Expand Down
54 changes: 0 additions & 54 deletions e2e/auth.setup.ts

This file was deleted.

7 changes: 4 additions & 3 deletions e2e/home.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
import { test, expect } from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Authenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Homepage view", async ({ page, isMobile }) => {
await page.goto("http://localhost:3000/");

Expand All@@ -24,9 +28,6 @@ test.describe("Authenticated homepage", () => {
});

test.describe("Unauthenticated homepage", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
test("Homepage view", async ({ page }) => {
await page.goto("http://localhost:3000/");

Expand Down
4 changes: 4 additions & 0 deletions e2e/login.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { test, expect } from "playwright/test";
import "dotenv/config";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
Expand DownExpand Up@@ -31,6 +32,9 @@ test.describe("Unauthenticated Login Page", () => {
});

test.describe("Authenticated Login Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
test("Sign up page contains sign up links", async ({ page, isMobile }) => {
// authenticated users are kicked back to the homepage if they try to go to /get-started
await page.goto("http://localhost:3000/get-started");
Expand Down
7 changes: 4 additions & 3 deletions e2e/my-posts.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated my-posts Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
7 changes: 4 additions & 3 deletions e2e/settings.spec.ts
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";

test.describe("Unauthenticated setttings Page", () => {

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

Fix typo in test description

There's a typo in "setttings" (three t's) in the test suite description.

-test.describe("Unauthenticated setttings Page", () => {+test.describe("Unauthenticated settings Page", () => {
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test.describe("Unauthenticated setttings Page",()=>{
test.describe("Unauthenticated settings Page",()=>{

test.beforeEach(async ({ page }) => {
await page.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});

test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
Comment on lines 1 to 15

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

Consider adding test organization comments

To improve maintainability and clarity, consider adding JSDoc comments for each test suite to document the test organization and coverage goals.

 import test from "@playwright/test";
import { loggedInAsUserOne } from "./utils";
+/**+ * Test suite for unauthenticated user interactions with the settings page.+ * Verifies proper handling of unauthorized access and redirects.+ */
test.describe("Unauthenticated settings Page", () => {
// TODO: Implement tests for unauthenticated scenarios
});
+/**+ * Test suite for authenticated user interactions with the settings page.+ * Verifies settings management functionality for logged-in users.+ */
test.describe("Authenticated settings Page", () => {
test.beforeEach(async ({ page }) => {
await loggedInAsUserOne(page);
});
// TODO: Implement tests for authenticated scenarios
});
📝 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
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
test.describe("Unauthenticated setttings Page",()=>{
test.beforeEach(async({ page })=>{
awaitpage.context().clearCookies();
});
//
// Replace with tests for unauthenticated users
});
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});
importtestfrom"@playwright/test";
import{loggedInAsUserOne}from"./utils";
/**
*Testsuiteforunauthenticateduserinteractionswiththesettingspage.
*Verifiesproperhandlingofunauthorizedaccessandredirects.
*/
test.describe("Unauthenticated settings Page",()=>{
//
// Replace with tests for unauthenticated users
});
/**
*Testsuiteforauthenticateduserinteractionswiththesettingspage.
*Verifiessettingsmanagementfunctionalityforlogged-inusers.
*/
test.describe("Authenticated settings Page",()=>{
test.beforeEach(async({ page })=>{
awaitloggedInAsUserOne(page);
});
//
// Replace with tests for authenticated users
});

1 change: 1 addition & 0 deletions e2e/utils/index.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
export * from "./utils";
25 changes: 25 additions & 0 deletions e2e/utils/utils.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
import { expect, Page } from "@playwright/test";

export const loggedInAsUserOne = async (page: Page) => {
try {
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();

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

Add type safety for environment variable.

Consider adding runtime type checking for the environment variable value, not just its existence.

- expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();+ const sessionId = process.env.E2E_USER_ONE_SESSION_ID;+ expect(sessionId).toBeDefined();+ expect(typeof sessionId === 'string' && sessionId.length > 0).toBeTruthy();
📝 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
expect(process.env.E2E_USER_ONE_SESSION_ID).toBeDefined();
constsessionId=process.env.E2E_USER_ONE_SESSION_ID;
expect(sessionId).toBeDefined();
expect(typeofsessionId==='string'&&sessionId.length>0).toBeTruthy();


await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
Comment on lines +7 to +15

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

Enhance cookie security configuration.

The cookie configuration is missing important security flags:

  1. httpOnly to prevent XSS attacks
  2. secure flag for HTTPS-only transmission

Apply this diff to improve security:

 await page.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_ID as string,
domain: "localhost",
path: "/",
sameSite: "Lax",
+ httpOnly: true,+ secure: process.env.NODE_ENV === "production"
},
]);
📝 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
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
},
]);
awaitpage.context().addCookies([
{
name: "next-auth.session-token",
value: process.env.E2E_USER_ONE_SESSION_IDasstring,
domain: "localhost",
path: "/",
sameSite: "Lax",
httpOnly: true,
secure: process.env.NODE_ENV==="production"
},
]);


expect(
(await page.context().cookies()).find(
(cookie) => cookie.name === "next-auth.session-token",
),
).toBeTruthy();
Comment on lines +17 to +21

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

Strengthen cookie verification.

The current verification only checks for cookie existence. Consider validating the cookie value matches what was set.

 expect(
- (await page.context().cookies()).find(- (cookie) => cookie.name === "next-auth.session-token",- ),- ).toBeTruthy();+ (await page.context().cookies()).find(+ (cookie) => cookie.name === "next-auth.session-token" && + cookie.value === sessionId+ ),+ ).toBeTruthy("Session cookie was not set correctly");

Committable suggestion was skipped due to low confidence.

} catch (err) {
throw Error("Error while authenticating E2E test user one");
}
};
10 changes: 0 additions & 10 deletions playwright.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,36 +39,26 @@ export default defineConfig({
{ name: "setup", testMatch: /auth.setup\.ts/ },
{
name: "Desktop Chrome",
use: {
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},

// Example other browsers
{
name: "Desktop Firefox",
use: {
...devices["Desktop Firefox"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Chrome",
use: {
...devices["Pixel 9"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
{
name: "Mobile Safari",
use: {
...devices["iPhone 16"],
storageState: "playwright/.auth/browser.json",
},
dependencies: ["setup"],
},
],

Expand Down
15 changes: 0 additions & 15 deletions playwright/.auth/browser.json

This file was deleted.

2 changes: 1 addition & 1 deletion sample.env
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,4 +7,4 @@ DATABASE_URL=postgresql://postgres:secret@127.0.0.1:5432/postgres

E2E_USER_EMAIL=e2e@codu.co
E2E_USER_ID=8e3179ce-f32b-4d0a-ba3b-234d66b836ad
E2E_USER_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1
E2E_USER_ONE_SESSION_ID=df8a11f2-f20a-43d6-80a0-a213f1efedc1