From 54a14047be26a520befc8ea0f31f3f128722160f Mon Sep 17 00:00:00 2001 From: Felipe Cardoso Date: Mon, 3 Nov 2025 00:02:27 +0100 Subject: [PATCH] Enhance auth flows and improve e2e test reliability - Remove redundant `'use client'` directives in auth pages to streamline code. - Refine Playwright config: adjust worker limits and add video recording for failed tests. - Improve session management in e2e tests with isolated state clearing, console log collection, and detailed failure attachments. - Update API client: better handle auth routes, ensure safe token refresh, and prevent unnecessary redirects. --- frontend/e2e/auth-login.spec.ts | 72 ++++++++++++++++++- frontend/e2e/auth-register.spec.ts | 72 ++++++++++++++++++- frontend/playwright.config.ts | 6 +- frontend/src/app/(auth)/login/page.tsx | 2 - .../src/app/(auth)/password-reset/page.tsx | 2 - frontend/src/app/(auth)/register/page.tsx | 2 - frontend/src/lib/api/client.ts | 37 +++++++--- 7 files changed, 174 insertions(+), 19 deletions(-) diff --git a/frontend/e2e/auth-login.spec.ts b/frontend/e2e/auth-login.spec.ts index ea68c24..1894357 100644 --- a/frontend/e2e/auth-login.spec.ts +++ b/frontend/e2e/auth-login.spec.ts @@ -1,11 +1,81 @@ import { test, expect } from '@playwright/test'; test.describe('Login Flow', () => { - test.beforeEach(async ({ page }) => { + test.describe.configure({ mode: 'serial' }); + + // Collect browser console logs per test for debugging + let consoleLogs: string[] = []; + + test.beforeEach(async ({ page, context }) => { + consoleLogs = []; + + // Capture console logs + page.on('console', (msg) => { + try { + consoleLogs.push(`[${msg.type()}] ${msg.text()}`); + } catch { + // ignore + } + }); + + // Ensure clean state across parallel workers + await context.clearCookies(); + await page.addInitScript(() => { + try { + localStorage.clear(); + sessionStorage.clear(); + } catch {} + }); + // Navigate to login page before each test await page.goto('/login'); }); + test.afterEach(async ({ page }, testInfo) => { + if (testInfo.status !== 'passed') { + // Attach current URL + await testInfo.attach('page-url.txt', { + body: page.url(), + contentType: 'text/plain', + }); + + // Attach skeleton count to see if page stuck on loading state + try { + const skeletonCount = await page.locator('.animate-pulse').count(); + await testInfo.attach('skeleton-count.txt', { + body: String(skeletonCount), + contentType: 'text/plain', + }); + } catch {} + + // Attach full DOM snapshot + try { + const html = await page.content(); + await testInfo.attach('dom.html', { + body: html, + contentType: 'text/html', + }); + } catch {} + + // Attach full page screenshot + try { + const img = await page.screenshot({ fullPage: true }); + await testInfo.attach('screenshot.png', { + body: img, + contentType: 'image/png', + }); + } catch {} + + // Attach console logs + try { + await testInfo.attach('console.log', { + body: consoleLogs.join('\n'), + contentType: 'text/plain', + }); + } catch {} + } + }); + test('should display login form', async ({ page }) => { // Check page title await expect(page.locator('h2')).toContainText('Sign in to your account'); diff --git a/frontend/e2e/auth-register.spec.ts b/frontend/e2e/auth-register.spec.ts index 2695aaa..47e07b1 100644 --- a/frontend/e2e/auth-register.spec.ts +++ b/frontend/e2e/auth-register.spec.ts @@ -1,11 +1,81 @@ import { test, expect } from '@playwright/test'; test.describe('Registration Flow', () => { - test.beforeEach(async ({ page }) => { + test.describe.configure({ mode: 'serial' }); + + // Collect browser console logs per test for debugging + let consoleLogs: string[] = []; + + test.beforeEach(async ({ page, context }) => { + consoleLogs = []; + + // Capture console logs + page.on('console', (msg) => { + try { + consoleLogs.push(`[${msg.type()}] ${msg.text()}`); + } catch { + // ignore + } + }); + + // Ensure clean state across parallel workers + await context.clearCookies(); + await page.addInitScript(() => { + try { + localStorage.clear(); + sessionStorage.clear(); + } catch {} + }); + // Navigate to register page before each test await page.goto('/register'); }); + test.afterEach(async ({ page }, testInfo) => { + if (testInfo.status !== 'passed') { + // Attach current URL + await testInfo.attach('page-url.txt', { + body: page.url(), + contentType: 'text/plain', + }); + + // Attach skeleton count to see if page stuck on loading state + try { + const skeletonCount = await page.locator('.animate-pulse').count(); + await testInfo.attach('skeleton-count.txt', { + body: String(skeletonCount), + contentType: 'text/plain', + }); + } catch {} + + // Attach full DOM snapshot + try { + const html = await page.content(); + await testInfo.attach('dom.html', { + body: html, + contentType: 'text/html', + }); + } catch {} + + // Attach full page screenshot + try { + const img = await page.screenshot({ fullPage: true }); + await testInfo.attach('screenshot.png', { + body: img, + contentType: 'image/png', + }); + } catch {} + + // Attach console logs + try { + await testInfo.attach('console.log', { + body: consoleLogs.join('\n'), + contentType: 'text/plain', + }); + } catch {} + } + }); + test('should display registration form', async ({ page }) => { // Check page title await expect(page.locator('h2')).toContainText('Create your account'); diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index 7c44fa0..a29c0e4 100644 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -17,8 +17,8 @@ export default defineConfig({ forbidOnly: !!process.env.CI, /* Retry on CI and locally to handle flaky tests */ retries: process.env.CI ? 2 : 1, - /* Limit workers to prevent test interference */ - workers: process.env.CI ? 1 : 12, + /* Limit workers to prevent test interference and Next dev server overload */ + workers: process.env.CI ? 1 : 8, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: process.env.CI ? 'github' : 'list', /* Suppress console output unless VERBOSE=true */ @@ -31,6 +31,8 @@ export default defineConfig({ trace: 'on-first-retry', /* Screenshot on failure */ screenshot: 'only-on-failure', + /* Record video for failed tests to diagnose flakiness */ + video: 'retain-on-failure', }, /* Configure projects for major browsers */ diff --git a/frontend/src/app/(auth)/login/page.tsx b/frontend/src/app/(auth)/login/page.tsx index 70a8214..36d20c9 100644 --- a/frontend/src/app/(auth)/login/page.tsx +++ b/frontend/src/app/(auth)/login/page.tsx @@ -1,5 +1,3 @@ -'use client'; - import dynamic from 'next/dynamic'; // Code-split LoginForm - heavy with react-hook-form + validation diff --git a/frontend/src/app/(auth)/password-reset/page.tsx b/frontend/src/app/(auth)/password-reset/page.tsx index c06abfe..235a34f 100644 --- a/frontend/src/app/(auth)/password-reset/page.tsx +++ b/frontend/src/app/(auth)/password-reset/page.tsx @@ -3,8 +3,6 @@ * Users enter their email to receive reset instructions */ -'use client'; - import dynamic from 'next/dynamic'; // Code-split PasswordResetRequestForm diff --git a/frontend/src/app/(auth)/register/page.tsx b/frontend/src/app/(auth)/register/page.tsx index a481b96..b33998f 100644 --- a/frontend/src/app/(auth)/register/page.tsx +++ b/frontend/src/app/(auth)/register/page.tsx @@ -1,5 +1,3 @@ -'use client'; - import dynamic from 'next/dynamic'; // Code-split RegisterForm (313 lines) diff --git a/frontend/src/lib/api/client.ts b/frontend/src/lib/api/client.ts index 5054607..ea596da 100644 --- a/frontend/src/lib/api/client.ts +++ b/frontend/src/lib/api/client.ts @@ -95,17 +95,18 @@ async function refreshAccessToken(): Promise { console.error('[API Client] Token refresh failed:', error); } - // Clear auth and redirect to login + // Clear auth state const authStore = await getAuthStore(); await authStore.clearAuth(); - // Redirect to login if we're in browser + // Only redirect to login when not already on an auth route if (typeof window !== 'undefined') { const currentPath = window.location.pathname; - const returnUrl = currentPath !== '/login' && currentPath !== '/register' - ? `?returnUrl=${encodeURIComponent(currentPath)}` - : ''; - window.location.href = `/login${returnUrl}`; + const onAuthRoute = currentPath === '/login' || currentPath === '/register' || currentPath.startsWith('/password-reset'); + if (!onAuthRoute) { + const returnUrl = currentPath ? `?returnUrl=${encodeURIComponent(currentPath)}` : ''; + window.location.href = `/login${returnUrl}`; + } } throw error; @@ -129,8 +130,12 @@ client.instance.interceptors.request.use( const authStore = await getAuthStore(); const { accessToken } = authStore; - // Add Authorization header if token exists - if (accessToken && requestConfig.headers) { + // Do not attach Authorization header for auth endpoints + const url = requestConfig.url || ''; + const isAuthEndpoint = url.includes('/auth/login') || url.includes('/auth/register') || url.includes('/auth/refresh') || url.includes('/auth/password') || url.includes('/password'); + + // Add Authorization header if token exists and not hitting auth endpoints + if (accessToken && requestConfig.headers && !isAuthEndpoint) { requestConfig.headers.Authorization = `Bearer ${accessToken}`; } @@ -170,13 +175,27 @@ client.instance.interceptors.response.use( // Handle 401 Unauthorized - Token expired if (error.response?.status === 401 && originalRequest && !originalRequest._retry) { + const url = originalRequest.url || ''; + const isAuthEndpoint = url.includes('/auth/login') || url.includes('/auth/register') || url.includes('/auth/password') || url.includes('/password'); + + // If the 401 is from auth endpoints, do not attempt refresh + if (isAuthEndpoint) { + return Promise.reject(error); + } + // If refresh endpoint itself fails with 401, clear auth and reject - if (originalRequest.url?.includes('/auth/refresh')) { + if (url.includes('/auth/refresh')) { const authStore = await getAuthStore(); await authStore.clearAuth(); return Promise.reject(error); } + // Ensure we have a refresh token before attempting refresh + const authStore = await getAuthStore(); + if (!authStore.refreshToken) { + return Promise.reject(error); + } + originalRequest._retry = true; try {