test(frontend): migrate to Jest v30, bump @types/jest, add VM modules… #2304
Conversation
… flag; stabilize date formatting and mock @heroui/button for tests
Summary by CodeRabbit
WalkthroughUpdates test infrastructure (Jest v30 migration, test setup and mocks, polyfills), switches frontend date formatting to UTC, and adds conditional Docker cache checks and offline-safe GraphQL codegen logic; also adjusts frontend Makefile and npm config. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/jest.setup.ts (2)
82-112: Restore spies after each test to prevent cascading failures.
jest.spyOnon console methods insidebeforeEachwill throw on the second test without restore.Add:
beforeEach(() => { jest.spyOn(console, 'error').mockImplementation((...args) => { throw new Error(`Console error: ${args.join(' ')}`) }) @@ global.removeAnimationFrameCallbacks = jest.fn() }) + +afterEach(() => { + jest.restoreAllMocks() +})
87-95: Don’t silence all console.warn; forward non‑ignored messages to the original.Apply:
- jest.spyOn(global.console, 'warn').mockImplementation((message) => { - if ( - typeof message === 'string' && - message.includes('[@zag-js/dismissable] node is `null` or `undefined`') - ) { - return - } - }) + const originalWarn = console.warn + jest.spyOn(global.console, 'warn').mockImplementation((...args) => { + const [message] = args + if ( + typeof message === 'string' && + message.includes('[@zag-js/dismissable] node is `null` or `undefined`') + ) { + return + } + return originalWarn.apply(console, args as any) + })frontend/src/components/ProgramCard.tsx (1)
68-71: Guard optional onEdit to prevent runtime crash.
onEditis optional but invoked unconditionally for admins.- <ActionButton onClick={() => onEdit(program.key)}> + <ActionButton onClick={() => onEdit?.(program.key)}>frontend/src/utils/dateFormatter.ts (1)
31-36: Align comparisons with UTC to match UTC formatting.You format in UTC but compute “same day/month/year” in local time. This will misclassify ranges near midnight in non‑UTC locales.
- if ( - start.getTime() === end.getTime() || - (start.getFullYear() === end.getFullYear() && - start.getMonth() === end.getMonth() && - start.getDate() === end.getDate()) - ) { + if ( + start.getTime() === end.getTime() || + (start.getUTCFullYear() === end.getUTCFullYear() && + start.getUTCMonth() === end.getUTCMonth() && + start.getUTCDate() === end.getUTCDate()) + ) { @@ - const sameMonth = start.getMonth() === end.getMonth() && start.getFullYear() === end.getFullYear() - const sameYear = start.getFullYear() === end.getFullYear() + const sameMonth = + start.getUTCMonth() === end.getUTCMonth() && start.getUTCFullYear() === end.getUTCFullYear() + const sameYear = start.getUTCFullYear() === end.getUTCFullYear()Also applies to: 40-41
🧹 Nitpick comments (5)
frontend/jest.setup.ts (2)
64-66: Pass a timestamp to requestAnimationFrame callbacks.Some libs expect the high‑res time arg; provide one.
- jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { - return setTimeout(cb, 0) - }) + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => { + return window.setTimeout(() => cb(performance.now()), 0) as unknown as number + })
2-2: Remove duplicate React import.-import React from 'react'Also applies to: 15-15
frontend/src/components/ProgramCard.tsx (1)
16-22: Consider reusing the shared date util for consistency.Import and use
formatDatefromutils/dateFormatterinstead of an inline formatter to keep all date rendering aligned with UTC policy.frontend/src/utils/dateFormatter.ts (2)
45-61: Dash style consistency.This formatter uses an em dash (—) while ProgramCard uses an en dash (–). Pick one repo‑wide and stick to it for visual consistency.
1-9: Falsy 0 input is treated as “no date”.
if (!input)returns '' for0, which could be a valid Unix seconds timestamp. Consider explicit null/undefined check.Example:
-export const formatDate = (input: number | string) => { - if (!input) { +export const formatDate = (input: number | string | null | undefined) => { + if (input === null || input === undefined || input === '') { return '' }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
frontend/jest.setup.ts(1 hunks)frontend/package.json(3 hunks)frontend/src/components/ProgramCard.tsx(1 hunks)frontend/src/utils/dateFormatter.ts(2 hunks)
🔇 Additional comments (1)
frontend/package.json (1)
68-119: Verified — Jest configured to use @swc/jest, jsdom env, and setup filejest.config.ts sets transform '^.+\.tsx?$': '@swc/jest', testEnvironment: 'jest-environment-jsdom', and setupFilesAfterEnv: ['/jest.setup.ts']; frontend/package.json includes @swc/jest and jest-environment-jsdom.
|
Hi, I'm trying my level best to get this done, but my docker is refusing to boot up despite trying for days. I've also been really sick so I havent spent much time. Please give me a slight extension for this. @arkid15r I'm really sorry for this delay. |
|
Update @arkid15r , I fixed the make test and make check. All 242 tests are being passed, and I am uploading the code as we speak. Sorry for this delay! |
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (8)
backend/Makefile(1 hunks)frontend/.npmrc(1 hunks)frontend/Makefile(1 hunks)frontend/__tests__/e2e/pages/ProjectDetails.spec.ts(2 hunks)frontend/__tests__/e2e/pages/ProjectsHealthDashboardMetrics.spec.ts(1 hunks)frontend/__tests__/e2e/pages/UserDetails.spec.ts(1 hunks)frontend/graphql-codegen.ts(3 hunks)frontend/jest.setup.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/.npmrc
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/jest.setup.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T12:52:32.214Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2155
File: frontend/graphql-codegen.ts:52-52
Timestamp: 2025-08-30T12:52:32.214Z
Learning: When using GraphQL Codegen with async configuration that needs to fetch data (like CSRF tokens), export the async function directly instead of using top-level await: `export default async function()` rather than `export default await function()`.
Applied to files:
frontend/graphql-codegen.ts
🧬 Code graph analysis (3)
frontend/__tests__/e2e/pages/UserDetails.spec.ts (2)
frontend/__tests__/e2e/helpers/mockDashboardCookies.ts (1)
mockDashboardCookies(1-74)frontend/__tests__/unit/data/mockUserDetails.ts (1)
mockUserDetailsData(1-66)
frontend/__tests__/e2e/pages/ProjectsHealthDashboardMetrics.spec.ts (1)
frontend/__tests__/unit/data/mockProjectsHealthMetricsData.ts (1)
mockHealthMetricsData(1-25)
frontend/__tests__/e2e/pages/ProjectDetails.spec.ts (2)
frontend/__tests__/e2e/helpers/mockDashboardCookies.ts (1)
mockDashboardCookies(1-74)frontend/__tests__/unit/data/mockProjectDetailsData.ts (1)
mockProjectDetailsData(1-128)
🔇 Additional comments (3)
frontend/__tests__/e2e/pages/ProjectsHealthDashboardMetrics.spec.ts (1)
21-26: Stronger page sync looks goodThe extra
networkidlewait plus the longer visibility timeout should keep this spec from flaking when the dashboard takes a moment to hydrate. Nice catch.frontend/Makefile (1)
67-70: Thanks for front-loading codegenKicking off
graphql-codegen(with a verbose retry) before the Docker build plus guarding--cache-fromon image availability keeps CI stable without incurring noisy failures.backend/Makefile (1)
115-118: Conditional cache flag mirrors frontend nicelyMatching the frontend’s conditional
--cache-fromkeeps the build ergonomic while avoiding noisy failures when the cache image isn’t around.
| await mockDashboardCookies(page, mockProjectDetailsData, true) | ||
| await page.route('**/graphql*', async (route) => { | ||
| await route.fulfill({ status: 200, json: { data: mockProjectDetailsData } }) | ||
| }) | ||
| await page.context().addCookies([ | ||
| { | ||
| name: 'csrftoken', | ||
| value: 'abc123', | ||
| domain: 'localhost', | ||
| path: '/', | ||
| }, | ||
| ]) | ||
| await page.goto('/projects/test-project', { timeout: 60000 }) | ||
| await page.waitForLoadState('networkidle') |
There was a problem hiding this comment.
Guard the helper’s SyncDjangoSession stub
Here too, the new **/graphql* handler supersedes mockDashboardCookies’ existing **/graphql/ route, so SyncDjangoSession now receives mockProjectDetailsData (no githubAuth) instead of the staff-auth payload. That will regress auth-sensitive flows as soon as the page triggers the sync call. (playwright.dev)
Either drop this extra route—passing mockProjectDetailsData into the helper already feeds the project query—or fall back for SyncDjangoSession before fulfilling other operations, e.g.:
- await page.route('**/graphql*', async (route) => {
- await route.fulfill({ status: 200, json: { data: mockProjectDetailsData } })
- })
+ await page.route('**/graphql*', async (route, request) => {
+ const payload = request.postDataJSON?.() ?? {}
+ if (payload.operationName === 'SyncDjangoSession') {
+ await route.fallback()
+ return
+ }
+ await route.fulfill({ status: 200, json: { data: mockProjectDetailsData } })
+ })📝 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.
| await mockDashboardCookies(page, mockProjectDetailsData, true) | |
| await page.route('**/graphql*', async (route) => { | |
| await route.fulfill({ status: 200, json: { data: mockProjectDetailsData } }) | |
| }) | |
| await page.context().addCookies([ | |
| { | |
| name: 'csrftoken', | |
| value: 'abc123', | |
| domain: 'localhost', | |
| path: '/', | |
| }, | |
| ]) | |
| await page.goto('/projects/test-project', { timeout: 60000 }) | |
| await page.waitForLoadState('networkidle') | |
| await mockDashboardCookies(page, mockProjectDetailsData, true) | |
| await page.route('**/graphql*', async (route, request) => { | |
| const payload = request.postDataJSON?.() ?? {} | |
| if (payload.operationName === 'SyncDjangoSession') { | |
| await route.fallback() | |
| return | |
| } | |
| await route.fulfill({ status: 200, json: { data: mockProjectDetailsData } }) | |
| }) | |
| await page.goto('/projects/test-project', { timeout: 60000 }) | |
| await page.waitForLoadState('networkidle') |
🤖 Prompt for AI Agents
In frontend/__tests__/e2e/pages/ProjectDetails.spec.ts around lines 7 to 12, the
new global '**/graphql*' route handler is overriding the helper
mockDashboardCookies' existing '**/graphql/' stub so the SyncDjangoSession
request gets project payload (missing githubAuth) and will break auth-sensitive
flows; either remove this extra route entirely (the helper already injects
mockProjectDetailsData) or make the handler guard for the SyncDjangoSession
operation by inspecting the incoming request (e.g. operationName or request
body/url) and call route.continue() or return the staff-auth payload for
SyncDjangoSession, otherwise fulfill with mockProjectDetailsData.
| await mockDashboardCookies(page, mockUserDetailsData, true) | ||
| await page.route('**/graphql*', async (route) => { | ||
| await route.fulfill({ status: 200, json: { data: mockUserDetailsData } }) | ||
| }) | ||
| await page.context().addCookies([ | ||
| { | ||
| name: 'csrftoken', | ||
| value: 'abc123', | ||
| domain: 'localhost', | ||
| path: '/', | ||
| }, | ||
| ]) | ||
| await page.goto('members/test-user') | ||
| await page.goto('/members/test-user') | ||
| await page.waitForLoadState('networkidle') |
There was a problem hiding this comment.
Don’t override the SyncDjangoSession mock
mockDashboardCookies already intercepts **/graphql/ and special-cases SyncDjangoSession to return the githubAuth payload that grants OWASP staff access. Registering a broader **/graphql* route afterwards runs first, so every GraphQL call—including SyncDjangoSession—now gets mockUserDetailsData, which is missing githubAuth and will break the session handshake when the app checks staff status. (playwright.dev)
Please either remove this extra route (the helper already fulfills the other operations) or fall back to the helper for SyncDjangoSession before fulfilling user-detail queries yourself. For example:
- await page.route('**/graphql*', async (route) => {
- await route.fulfill({ status: 200, json: { data: mockUserDetailsData } })
- })
+ await page.route('**/graphql*', async (route, request) => {
+ const payload = request.postDataJSON?.() ?? {}
+ if (payload.operationName === 'SyncDjangoSession') {
+ await route.fallback()
+ return
+ }
+ await route.fulfill({ status: 200, json: { data: mockUserDetailsData } })
+ })📝 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.
| await mockDashboardCookies(page, mockUserDetailsData, true) | |
| await page.route('**/graphql*', async (route) => { | |
| await route.fulfill({ status: 200, json: { data: mockUserDetailsData } }) | |
| }) | |
| await page.context().addCookies([ | |
| { | |
| name: 'csrftoken', | |
| value: 'abc123', | |
| domain: 'localhost', | |
| path: '/', | |
| }, | |
| ]) | |
| await page.goto('members/test-user') | |
| await page.goto('/members/test-user') | |
| await page.waitForLoadState('networkidle') | |
| await mockDashboardCookies(page, mockUserDetailsData, true) | |
| await page.route('**/graphql*', async (route, request) => { | |
| const payload = request.postDataJSON?.() ?? {} | |
| if (payload.operationName === 'SyncDjangoSession') { | |
| await route.fallback() | |
| return | |
| } | |
| await route.fulfill({ status: 200, json: { data: mockUserDetailsData } }) | |
| }) | |
| await page.goto('/members/test-user') | |
| await page.waitForLoadState('networkidle') |
| export default (async (): Promise<CodegenConfig> => { | ||
| let response | ||
| let csrfToken: string | undefined | ||
| let backendUp = false | ||
|
|
||
| // Detect if backend is reachable | ||
| try { | ||
| response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | ||
| method: 'GET', | ||
| }) | ||
| const statusRes = await fetch(`${PUBLIC_API_URL}/status/`, { method: 'GET' }) | ||
| backendUp = statusRes.ok | ||
| } catch { | ||
| /* eslint-disable no-console */ | ||
| console.log('Failed to fetch CSRF token: make sure the backend is running.') | ||
| return | ||
| backendUp = false | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | ||
| if (backendUp) { | ||
| try { | ||
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { method: 'GET' }) | ||
| if (response.ok) { | ||
| csrfToken = (await response.json()).csrftoken | ||
| } | ||
| } catch { | ||
| // If CSRF fails but backend is up, proceed without CSRF headers | ||
| } | ||
| } |
There was a problem hiding this comment.
Export the async config function instead of invoking it once.
Wrapping the config in an async IIFE means the backend reachability check and CSRF fetch run only at module evaluation time. In graphql-codegen --watch (or any long-lived process), the CLI reuses the resolved Promise and never re-invokes this logic, so a transient backend outage or expired CSRF token leaves the process stuck on the fallback schema until the command is restarted. Please export the async function itself so Codegen can call it for every run.
-export default (async (): Promise<CodegenConfig> => {
+export default async function generateGraphqlCodegenConfig(): Promise<CodegenConfig> {
let csrfToken: string | undefined
let backendUp = false
…
- }
-})()
+ }
+}Based on learnings
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default (async (): Promise<CodegenConfig> => { | |
| let response | |
| let csrfToken: string | undefined | |
| let backendUp = false | |
| // Detect if backend is reachable | |
| try { | |
| response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | |
| method: 'GET', | |
| }) | |
| const statusRes = await fetch(`${PUBLIC_API_URL}/status/`, { method: 'GET' }) | |
| backendUp = statusRes.ok | |
| } catch { | |
| /* eslint-disable no-console */ | |
| console.log('Failed to fetch CSRF token: make sure the backend is running.') | |
| return | |
| backendUp = false | |
| } | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | |
| if (backendUp) { | |
| try { | |
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { method: 'GET' }) | |
| if (response.ok) { | |
| csrfToken = (await response.json()).csrftoken | |
| } | |
| } catch { | |
| // If CSRF fails but backend is up, proceed without CSRF headers | |
| } | |
| } | |
| export default async function generateGraphqlCodegenConfig(): Promise<CodegenConfig> { | |
| let csrfToken: string | undefined | |
| let backendUp = false | |
| // Detect if backend is reachable | |
| try { | |
| const statusRes = await fetch(`${PUBLIC_API_URL}/status/`, { method: 'GET' }) | |
| backendUp = statusRes.ok | |
| } catch { | |
| backendUp = false | |
| } | |
| if (backendUp) { | |
| try { | |
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { method: 'GET' }) | |
| if (response.ok) { | |
| csrfToken = (await response.json()).csrftoken | |
| } | |
| } catch { | |
| // If CSRF fails but backend is up, proceed without CSRF headers | |
| } | |
| } | |
| // …rest of your CodegenConfig construction and return… | |
| } |
🤖 Prompt for AI Agents
In frontend/graphql-codegen.ts around lines 5 to 26, the file currently exports
a resolved Promise (an async IIFE) so backend reachability/CSRF checks run only
once at module load; change the export to an async function so GraphQL Codegen
can call it on every run. Replace the current export with a named or default
async function signature (e.g., export default async function generateConfig():
Promise<CodegenConfig> { ... }) and ensure you do not immediately invoke it;
keep all fetch logic inside that function body so the checks run each time the
function is called.
|
Fixed in #2362 |
2 participants
Proposed change
-Fixes #2212
-Migrate frontend unit test stack from Jest v29 to v30 and ensure compatibility:
-Bump Jest to v30 and @types/jest to v30
-Keep @swc/jest as transformer; remove ts-jest
-Update test script to enable Node VM modules for dynamic imports
-Stabilize date formatting (UTC) to avoid timezone flakiness
-Add lightweight mock for @heroui/button to bypass framer-motion dynamic import in tests
-Set deterministic test env in jest.setup.ts (TZ, project-health flag)
Changes
-frontend/package.json
-jest: ^30.x
-@types/jest: ^30.x
-remove ts-jest
-test:unit → NODE_OPTIONS='--experimental-vm-modules' jest
-frontend/jest.setup.ts
-process.env.TZ='UTC', NEXT_PUBLIC_IS_PROJECT_HEALTH_ENABLED='true'
-mock @heroui/button to a plain button component for tests
-frontend/src/components/ProgramCard.tsx
-force UTC in date output for deterministic tests
-frontend/src/utils/dateFormatter.ts
-force UTC in formatDate and formatDateRange computations
-pnpm-lock.yaml updated
Testing
-Locally ran pnpm test:unit (Node 22+ required):
-All suites pass after env/mocks/date updates
-No production/runtime behavior changes
Checklist
[X ] I’ve read and followed the contributing guidelines
[ X] I’ve run make check-test locally; all checks and tests passed
Notes:
-Jest v30 requires Node 18+. The repo targets Node 22; CI should be OK.
-If any suites still fail in CI due to environment differences, we can revisit the VM modules flag or mocks accordingly.