Skip to content

Improve frontend test coverage above 95%#3952

Merged
arkid15r merged 7 commits intoOWASP:mainfrom
anurag2787:fix/frontend-unit-test-95
Feb 15, 2026
Merged

Improve frontend test coverage above 95%#3952
arkid15r merged 7 commits intoOWASP:mainfrom
anurag2787:fix/frontend-unit-test-95

Conversation

@anurag2787
Copy link
Contributor

@anurag2787 anurag2787 commented Feb 15, 2026

Proposed change

This PR improves frontend unit test coverage for files that were previously below the 95% line coverage

Resolves #3937

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds extensive unit tests across many frontend components and pages and makes focused implementation changes to ItemCardList, MetricsCard, EntityActions, MultiSearch, ProgramForm, TruncatedText, and two pages; no public API signatures were expanded.

Changes

Cohort / File(s) Summary
Component tests (bulk additions)
frontend/__tests__/unit/components/CalendarButton.test.tsx, frontend/__tests__/unit/components/CardDetailsPage.test.tsx, frontend/__tests__/unit/components/EntityActions.test.tsx, frontend/__tests__/unit/components/InfoBlock.test.tsx, frontend/__tests__/unit/components/ItemCardList.test.tsx, frontend/__tests__/unit/components/MentorshipPullRequest.test.tsx, frontend/__tests__/unit/components/MetricsCard.test.tsx, frontend/__tests__/unit/components/ModuleCard.test.tsx, frontend/__tests__/unit/components/MultiSearch.test.tsx, frontend/__tests__/unit/components/NavDropDown.test.tsx, frontend/__tests__/unit/components/RepositoryCard.test.tsx, frontend/__tests__/unit/components/Search.test.tsx, frontend/__tests__/unit/components/SortBy.test.tsx, frontend/__tests__/unit/components/TruncatedText.test.tsx, frontend/__tests__/unit/components/UserCard.test.tsx, frontend/__tests__/unit/components/UserMenu.test.tsx
Many new/expanded tests covering accessibility, keyboard interactions, null/undefined fallbacks, link/href behavior, avatars, and edge-case rendering.
Form & skeleton tests
frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx, frontend/__tests__/unit/components/skeletons/Card.test.tsx
New suites for FormTextarea (rendering, validation, onChange) and CardSkeleton (conditional skeleton counts).
Context tests
frontend/__tests__/unit/contexts/BreadcrumbContext.test.tsx
Adds registration and sorting tests for breadcrumb ordering with cleanup between tests.
Page tests (bulk additions)
frontend/__tests__/unit/pages/About.test.tsx, frontend/__tests__/unit/pages/CreateModule.test.tsx, frontend/__tests__/unit/pages/CreateProgram.test.tsx, frontend/__tests__/unit/pages/EditModule.test.tsx, frontend/__tests__/unit/pages/EditProgram.test.tsx, frontend/__tests__/unit/pages/Header.test.tsx, frontend/__tests__/unit/pages/IssuesPage.test.tsx, frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx, frontend/__tests__/unit/pages/MyMentorship.test.tsx, frontend/__tests__/unit/pages/OrganizationDetails.test.tsx, frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx, frontend/__tests__/unit/pages/Users.test.tsx, frontend/__tests__/unit/pages/Release.test.tsx
Adds edge-case and defensive tests for pages: missing fields, mutation errors, debounce/unmount, disabled controls, and rendering fallbacks.
ItemCardList implementation
frontend/src/components/ItemCardList.tsx
Refactors list-item key generation to build an array of parts, filter falsy values, join with dashes, and fallback to item-${idx}.
MetricsCard implementation
frontend/src/components/MetricsCard.tsx
Introduces local score = metric.score ?? 0 and reuses it for styling and display.
EntityActions implementation
frontend/src/components/EntityActions.tsx
Removes early return in handleKeyDown (handles keys when closed), triggers focused item on Enter/Space without focusIndex guard, and sets menu tabIndex to 0 unconditionally.
Minor component adjustments
frontend/src/components/MultiSearch.tsx, frontend/src/components/ProgramForm.tsx, frontend/src/components/TruncatedText.tsx
Small behavior/safety tweaks: use `
Page tweaks
frontend/src/app/chapters/[chapterKey]/page.tsx, frontend/src/app/projects/dashboard/metrics/page.tsx
Removed empty-string fallback for formatted updatedAt; replaced conditional selectedKeys/labels with unconditional arrays (assumes key presence).
Repo & config
.markdownlint.yaml, package.json
Added markdownlint configuration; small manifest changes reported in tests metadata.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • kasya
  • arkid15r
🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main change: improving frontend test coverage to exceed 95%, which directly aligns with the changeset's focus on adding comprehensive unit tests across multiple components and pages.
Description check ✅ Passed The PR description is related to the changeset. It explains the purpose (improving frontend test coverage above 95%), references the linked issue (#3937), and confirms all required workflow steps were completed including running tests locally.
Linked Issues check ✅ Passed The PR successfully addresses the primary objectives from issue #3937 by adding comprehensive unit tests to 32 identified files. Tests cover edge cases, fallback behaviors, and error handling scenarios to increase line coverage for components, pages, and forms. All code changes support the stated goal of achieving 95%+ coverage per file.
Out of Scope Changes check ✅ Passed The PR contains minimal non-test code changes that fall within scope: refactoring of MetricsCard, EntityActions, ItemCardList, MultiSearch, ProgramForm, and TruncatedText improve code robustness and maintainability while supporting test coverage objectives. One markdown lint config is a configuration addition. All changes support the coverage improvement goal.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🤖 Fix all issues with AI agents
In `@frontend/__tests__/unit/components/CardDetailsPage.test.tsx`:
- Around line 2373-2397: The test shows a malformed fallback URL with a double
slash because CardDetailsPage calls getMenteeUrl with undefined/empty
programKey/entityKey; update the production code (preferably in getMenteeUrl or
wherever CardDetailsPage builds the link) to omit empty segments when keys are
falsy (e.g., only append `/programs/${programKey}` if programKey is truthy) so
URLs normalize (no `//`), and then update this test (CardDetailsPage.test.tsx)
to expect the normalized URL (e.g., '/programs/mentees/test_mentee' or
'/mentees/test_mentee' depending on chosen convention); adjust DetailsCardProps
handling if needed to ensure programKey/entityKey default to undefined/null and
not produce empty string segments.

In `@frontend/__tests__/unit/components/EntityActions.test.tsx`:
- Around line 730-742: The new "Toggle Behavior" test suite (describe('Toggle
Behavior', ...)) is declared outside the main describe('EntityActions', ...) so
it misses the existing useRouter mock setup and beforeEach that sets useRouter
to return { push: mockPush }; move the entire describe('Toggle Behavior', ...)
block so it sits inside the main describe('EntityActions', ...) block (so it
inherits the beforeEach/useRouter mock and mockPush), ensuring the EntityActions
component has the router mock during render and preventing potential TypeError
if router is accessed.

In `@frontend/__tests__/unit/components/ProgramForm.test.tsx`:
- Around line 1197-1222: The test 'handles string menteesLimit in validation'
uses userEvent.click directly, skipping the required setup; replace the direct
call with the library's async user setup: create const user = userEvent.setup()
(like other tests) and call await user.click(submitButton) to trigger the
submit; update the invocation around submitButton in the ProgramForm.test.tsx
test to use the user variable and await the click so it matches other tests and
works with `@testing-library/user-event` v14+.

In `@frontend/__tests__/unit/components/TruncatedText.test.tsx`:
- Around line 248-268: The test replaces React.useRef with a mock but doesn’t
restore it correctly; capture the spy returned by jest.spyOn(React, 'useRef')
into a variable (e.g., useRefSpy) before mocking, and ensure you call
useRefSpy.mockRestore() in a finally block (or after the assertion) so
React.useRef is properly restored even if the test fails; update the test that
creates nullRef and mocks React.useRef to restore the original via the stored
spy instead of calling jest.spyOn(...) again.

In `@frontend/__tests__/unit/contexts/BreadcrumbContext.test.tsx`:
- Around line 41-52: Rename the test title to accurately reflect that Home ('/')
is not registered: update the it(...) description in BreadcrumbContext.test.tsx
(the test that uses registerBreadcrumb and getBreadcrumbItems and pushes into
cleanupFns) from "sorts by length when home is involved" to something like
"sorts by path length when home is not present" so the name matches the actual
behavior being asserted (the fallback a.path.length - b.path.length branch).
- Around line 10-13: The forEach callback in the afterEach block implicitly
returns fn() which violates the Biome rule; change the callback to not return a
value by using a block-bodied arrow or a for...of loop to call each cleanup
function without returning. Update the afterEach that references cleanupFns and
its callback (currently written as (fn) => fn()) to use either (fn) => { fn(); }
or iterate with for (const fn of cleanupFns) { fn(); } and then reset cleanupFns
= [].

In `@frontend/__tests__/unit/pages/MyMentorship.test.tsx`:
- Around line 380-419: The test "updates debounced search query" uses
jest.useFakeTimers() but calls jest.useRealTimers() unguarded, risking fake
timers leaking if an assertion fails; modify the test (the it block named
"updates debounced search query") to wrap the test body that uses
jest.useFakeTimers()/jest.advanceTimersByTime(500) in a try/finally and move
jest.useRealTimers() into the finally block so real timers are always restored
regardless of failures.

In `@frontend/src/components/ProgramForm.tsx`:
- Around line 157-160: There is a duplicated comment above the name-uniqueness
check in ProgramForm.tsx; remove one of the identical lines so only a single
comment remains above the call to checkNameUniquenessSync and the subsequent
setNameUniquenessError call (look for the comment text "// Check name uniqueness
and capture result before validation" near the const uniquenessError = await
checkNameUniquenessSync(formData.name) and remove the duplicate).
🧹 Nitpick comments (19)
frontend/__tests__/unit/pages/Users.test.tsx (1)

150-170: Consider adding a negative assertion for the missing company.

The test verifies the user name renders, but doesn't assert that no company text is shown. Adding a check like expect(screen.queryByText('Some Company')).not.toBeInTheDocument() (or whatever the component renders for company) would make this test more meaningful by confirming the null company path is actually exercised, not just that the name renders.

That said, the test is consistent with the existing patterns in this file and is a reasonable addition.

💡 Optional: strengthen the assertion
     await waitFor(() => {
       expect(screen.getByText('User Without Company')).toBeInTheDocument()
     })
+
+    // Verify no company is rendered for this user
+    expect(screen.queryByText('company')).not.toBeInTheDocument()
   })

Adjust the query to match whatever text or element the component renders for the company field.

frontend/__tests__/unit/components/RepositoryCard.test.tsx (1)

81-86: Consider a less brittle assertion than querying by CSS class.

container.querySelector('.grid') couples the test to the component's internal CSS class. If the class name changes, this test breaks without any behavioral change. A more resilient alternative:

-    expect(container.querySelector('.grid')).toBeNull()
+    expect(container.firstChild).toBeNull()

Or if the component renders a wrapper even when empty, toBeEmptyDOMElement() on the container could work. Either way, this is a minor point — the test logic itself is sound.

frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx (1)

4-11: Consider resetting mocks between tests for safety.

defaultProps.onChange is a shared jest.fn() that persists across tests. Currently no test relies on its call count, but adding a cleanup guard prevents subtle issues as the suite grows.

♻️ Suggested addition
 describe('FormTextarea', () => {
   const defaultProps = {
     id: 'test-textarea',
     label: 'Test Label',
     placeholder: 'Enter text',
     value: '',
     onChange: jest.fn(),
   }
+
+  afterEach(() => {
+    jest.clearAllMocks()
+  })
frontend/__tests__/unit/pages/IssuesPage.test.tsx (1)

341-373: Largely overlaps with the test at Line 265; consider consolidating.

This test covers the combination of empty availableLabels + a mix of non-null and null labels across issues, but the empty-availableLabels + extraction path is already exercised by the test at Line 265. The only incremental value here is verifying that a sibling issue with labels: null doesn't break extraction.

Consider folding the null-labels issue into the existing test at Line 265 instead of adding a near-duplicate test block. This keeps the suite leaner and easier to maintain.

Also, this is the only mockUseQuery call in the file that omits error: undefined — minor inconsistency with the rest of the test suite.

Suggested consolidation into the existing test (Line 265)
 it('extracts labels from issues when availableLabels is empty', async () => {
     const dataWithoutAvailableLabels = {
       getModule: {
         ...mockModuleData.getModule,
         availableLabels: [],
         issues: [
           {
             ...mockModuleData.getModule.issues[0],
             labels: ['extracted-label-1', 'extracted-label-2'],
           },
+          {
+            ...mockModuleData.getModule.issues[0],
+            id: '2',
+            labels: null,
+          },
         ],
       },
     }
frontend/__tests__/unit/pages/ProjectHealthDashboardMetricsDetails.test.tsx (1)

263-287: Assertion is weak — consider verifying the date is actually processed.

The only assertion checks that mocked ApexCharts elements exist, which would also pass with createdAt: null (as the test on line 158 already demonstrates). This means the test doesn't actually verify the "valid createdAt" code path behaves differently from the null case.

If the component formats the date and displays it (e.g., as axis labels or a heading), assert on that rendered output. If the date only flows into the chart config (invisible due to the mock), consider at minimum asserting that useQuery was called and the component didn't error — or inspect the props passed to the mocked ApexCharts component.

Example: capture props passed to mocked ApexCharts
-jest.mock('react-apexcharts', () => {
-  return {
-    __esModule: true,
-    default: () => {
-      return <div data-testid="mock-apexcharts">Mock ApexChart</div>
-    },
-  }
-})
+const mockApexCharts = jest.fn(() => <div data-testid="mock-apexcharts">Mock ApexChart</div>)
+jest.mock('react-apexcharts', () => ({
+  __esModule: true,
+  default: mockApexCharts,
+}))

Then in the test you could assert the chart received series data derived from the valid date:

await waitFor(() => {
  expect(mockApexCharts).toHaveBeenCalled()
  // Optionally inspect: mockApexCharts.mock.calls[0][0].options / series
})
frontend/__tests__/unit/components/Release.test.tsx (2)

396-411: Duplicate test — this is identical to the test at Line 333.

Both 'does not navigate when clicking repository button with missing org' (Line 333) and this test use the same setup (organizationName: ''), the same action (fireEvent.click), and the same assertion (mockRouterPush not called). Remove one of them.

Additionally, the block of speculative comments on lines 402–408 is noisy and should be removed — tests should document intent, not uncertainty about the testing framework.


411-412: Nit: Missing blank lines between test cases.

The rest of the file consistently separates it blocks with a blank line, but lines 411–412 and 440–441 are missing that separator.

Also applies to: 440-441

frontend/src/components/ItemCardList.tsx (1)

96-106: Variable title on Line 100 shadows the component's title prop (Line 62).

Inside getItemKey, the local const title shadows the outer destructured title prop. It's not a bug here since getItemKey doesn't reference the prop, but it can trip up future maintainers. Consider renaming to itemTitle.

♻️ Suggested rename
-            const title = 'title' in i ? i.title : ''
+            const itemTitle = 'title' in i ? i.title : ''
             const name = 'name' in i ? i.name : ''
             const url = 'url' in i ? i.url : ''
-            const keyParts = [repoName, title || name, url].filter(Boolean)
+            const keyParts = [repoName, itemTitle || name, url].filter(Boolean)
frontend/__tests__/unit/contexts/BreadcrumbContext.test.tsx (1)

54-55: Remove stale coverage-tracking comments.

These comments reference specific source line numbers and don't provide lasting value — they'll go stale as the source file evolves.

frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx (2)

617-621: Remove or rewrite the leftover exploratory comments.

Lines 619-620 read like draft notes ("LabelList handles null?", "If LabelList handles it, fine."). Either replace them with a concrete assertion (e.g., verify no label elements are rendered) or remove them so the test intent stays clear.

Proposed cleanup
     // Should render main sections without crashing
     expect(screen.getByText('Test Issue Title')).toBeInTheDocument()
-    // Labels section is rendered but empty (LabelList handles null?)
-    // If LabelList handles it, fine.
+    // Labels, assignees, and interested users sections should degrade gracefully
+    expect(screen.queryByText('bug')).not.toBeInTheDocument()

623-663: Assign/unassign assertions are redundant — buttons are already disabled.

The test at line 417 already verifies that assign/unassign buttons are disabled when assigning/unassigning are true. A disabled button won't fire its onClick in React, so assignIssue/unassignIssue not being called here (lines 652, 657) is due to the disabled attribute — not the in-handler logic guard the comment on line 649 describes.

The deadline-change guard (lines 659-662) is the genuinely valuable part of this test since the input element isn't disabled.

Consider either:

  1. Removing the assign/unassign portion (it duplicates existing coverage), or
  2. Testing the logic guard directly by keeping assigning: true but ensuring the button is not disabled (e.g., by verifying the component's guard fires even if the disabled prop were absent).
frontend/__tests__/unit/pages/Header.test.tsx (2)

407-432: Nice addition — covers the desktop logo closing the mobile menu.

This test correctly validates the behavior described in the component where the desktop logo's onClick calls setMobileMenuOpen(false). Minor nit: consider using toBeInTheDocument() instead of toBeDefined() on lines 427 and 400 for more descriptive failure messages — .find() returns undefined on miss, and toBeDefined() just says "expected not to be undefined" vs a proper DOM assertion message.

Optional: slightly more descriptive assertions
-      expect(desktopLogoLink).toBeDefined()
+      expect(desktopLogoLink).toBeInTheDocument()

Same for line 400:

-      expect(mobileLogoLink).toBeDefined()
+      expect(mobileLogoLink).toBeInTheDocument()

524-531: expect(true).toBe(true) is a no-op assertion.

This test only verifies that dispatching a resize event doesn't throw, but the assertion on line 530 provides zero signal. The test at lines 773–815 already properly validates resize behavior by extracting the handler and checking menu state. Consider either adding a real assertion here (e.g., verifying the menu is still present) or removing this test to avoid false confidence.

Suggested improvement
-      // Test passes if no errors are thrown
-      expect(true).toBe(true)
+      // Verify component is still rendered correctly after resize
+      expect(screen.getByRole('banner')).toBeInTheDocument()
frontend/__tests__/unit/components/skeletons/Card.test.tsx (1)

33-47: Add dedicated test-id to icon skeletons for more resilient test selectors.

Both tests identify "icon skeletons" by matching the substring 'h-8 w-16' in className. While this pattern is currently unique to icon skeletons in the component, relying on class names makes tests fragile to Tailwind refactoring. Add data-testid="icon-skeleton" to the icon skeleton elements (line 40 of Card.tsx) and use that selector in tests instead of className filtering.

frontend/src/components/ProgramForm.tsx (1)

245-255: Type mismatch: menteesLimit is typed as number but handled as potentially undefined.

Line 250 uses optional chaining (?.) and line 152 checks !== undefined && !== null, yet the interface on line 25 declares menteesLimit: number (required, non-nullable). If undefined is a valid runtime value, update the type to menteesLimit?: number for consistency. Otherwise the defensive checks are misleading.

frontend/src/components/TruncatedText.tsx (1)

1-1: Production code restyled solely to enable test mocking.

Switching from named hook imports to React.useRef / React.useCallback / React.useEffect has no runtime benefit — it was done only so the test can jest.spyOn(React, 'useRef'). This couples production code style to a testing implementation detail.

A less invasive alternative is to use dependency injection (passing the ref as a prop or using a wrapper) or to test the observable behavior (what renders) rather than internal wiring. That said, this is a minor concern and the current approach does work.

Also applies to: 12-12, 14-14, 21-21

frontend/__tests__/unit/pages/MyMentorship.test.tsx (1)

356-378: Consider adding a minimal assertion to verify no errors after unmount.

The test currently only asserts the component renders before unmount. While this is fine for coverage, you could check that no console errors or warnings were emitted to add a small amount of value beyond "it doesn't crash."

frontend/src/app/projects/dashboard/metrics/page.tsx (1)

237-238: Non-null assertion (!) is safe now but couples to an implicit invariant.

parseOrderParam always returns a valid urlKey that exists in both ORDERING_MAP and SORT_FIELDS, so find(...) will never return undefined today. However, the ! on line 238 will silently mask a runtime crash if SORT_FIELDS and ORDERING_MAP keys ever diverge.

Consider a fallback instead of the assertion:

Suggested safer alternative
- selectedLabels={[SORT_FIELDS.find((f) => f.key === urlKey)!.label]}
+ selectedLabels={[SORT_FIELDS.find((f) => f.key === urlKey)?.label ?? 'Score (High → Low)']}
frontend/__tests__/unit/components/NavDropDown.test.tsx (1)

643-681: Good coverage of no-op keyboard paths.

These tests correctly exercise the uncovered branches in the onKeyDown handlers for both the button and submenu items.

Minor nit: test names referencing source line numbers (e.g., "(line 50)", "(line 84)") will go stale when the source file changes. Consider describing the behavior instead, e.g., "ignores Escape when dropdown is already closed".

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

7 issues found across 41 files

Confidence score: 4/5

  • Test reliability risk: several unit tests can leak or mis-handle mocks/timers (e.g., jest.useRealTimers() in frontend/__tests__/unit/pages/MyMentorship.test.tsx, and missing beforeEach setup in frontend/__tests__/unit/components/EntityActions.test.tsx), which could cause flaky CI but not product behavior.
  • A few assertions are ineffective or ambiguous (tautological expect in frontend/__tests__/unit/components/MultiSearch.test.tsx, ambiguous /Assign/i match in frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx), so coverage may be weaker than intended.
  • Pay close attention to frontend/__tests__/unit/pages/MyMentorship.test.tsx, frontend/__tests__/unit/components/EntityActions.test.tsx, frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx, frontend/__tests__/unit/components/MultiSearch.test.tsx, frontend/__tests__/unit/components/TruncatedText.test.tsx, frontend/__tests__/unit/components/Release.test.tsx, frontend/__tests__/unit/components/ProgramForm.test.tsx - test cleanup/consistency issues could cause flakiness or missed assertions.
Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/__tests__/unit/components/Release.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/Release.test.tsx:396">
P2: This test is a duplicate of the existing `'does not navigate when clicking repository button with missing org'` test — both set `organizationName: ''`, click the button, and assert `mockRouterPush` was not called. The long stream-of-consciousness comments ("We can't easily force the click...", "BUT checking for coverage...", "Let's try firing it again") read like unedited AI reasoning rather than useful test documentation. Remove this duplicate test or, if it's meant to cover a genuinely different code path, restructure it to actually exercise that path differently.

(Based on your team's feedback about ensuring AI-generated changes are reviewed and tested.) [FEEDBACK_USED]</violation>
</file>

<file name="frontend/__tests__/unit/pages/MyMentorship.test.tsx">

<violation number="1" location="frontend/__tests__/unit/pages/MyMentorship.test.tsx:418">
P2: `jest.useRealTimers()` is not in a `try/finally` block or `afterEach`. If any assertion in this test fails before that line, fake timers leak into all subsequent tests, causing hard-to-debug cascading failures. Wrap the test body in `try/finally` or add an `afterEach(() => jest.useRealTimers())` to the describe block.</violation>
</file>

<file name="frontend/__tests__/unit/components/EntityActions.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/EntityActions.test.tsx:730">
P2: `describe('Toggle Behavior')` is placed outside the main `describe('EntityActions')` block, so it misses the `beforeEach` that calls `jest.clearAllMocks()` and sets up the `useRouter` mock. This can cause mock leakage and flaky tests. Move this describe block inside `describe('EntityActions')` to share the same setup/teardown lifecycle.</violation>
</file>

<file name="frontend/__tests__/unit/components/ProgramForm.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/ProgramForm.test.tsx:1215">
P2: Inconsistent `userEvent` usage: all other tests in this file (37 instances) use `const user = userEvent.setup()` then `user.click()`, but this test calls `userEvent.click()` directly. Use the instance API for consistency and to avoid subtle state-tracking issues when tests are extended with additional interactions.</violation>
</file>

<file name="frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx">

<violation number="1" location="frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx:647">
P2: The regex `/Assign/i` also matches the "Unassign @user1" button's aria-label, and since the Assignees section is rendered before Interested Users in the DOM, `assignButtons[0]` is the Unassign button — not the Assign button. This means the assertion `expect(assignIssue).not.toHaveBeenCalled()` trivially passes because the Unassign handler never calls `assignIssue`. Use a more precise selector to target the actual Assign button.</violation>
</file>

<file name="frontend/__tests__/unit/components/MultiSearch.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/MultiSearch.test.tsx:988">
P2: `expect(true).toBe(true)` is a tautological assertion that always passes — it doesn't verify anything about the component's behavior. The comment says "events should not be in suggestions" but this is never actually checked. Replace with a meaningful assertion, e.g., verifying that no event-related items appear in the rendered suggestions list.</violation>
</file>

<file name="frontend/__tests__/unit/components/TruncatedText.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/TruncatedText.test.tsx:267">
P2: Improper mock cleanup: the restoration at the end creates/reuses a spy with `mockImplementation` instead of properly restoring the original. Save the spy reference from `jest.spyOn` and call `.mockRestore()` to correctly undo the mock.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2026
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2026
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 10 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/__tests__/unit/pages/CreateModule.test.tsx">

<violation number="1" location="frontend/__tests__/unit/pages/CreateModule.test.tsx:29">
P3: This mock of `CreateModuleDocument` is added to the wrong module. The component imports `CreateModuleDocument` from `types/__generated__/moduleMutations.generated`, not from `@apollo/client/react`. This mock entry has no effect and is misleading dead code. If you need to mock `CreateModuleDocument`, add a separate `jest.mock('types/__generated__/moduleMutations.generated', ...)` block instead.</violation>
</file>

<file name="frontend/__tests__/unit/components/CardDetailsPage.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/CardDetailsPage.test.tsx:78">
P2: Mock for `getMenteeUrl` introduces conditional logic (`programKey ? …`) that doesn't exist in the real implementation. The real `getMenteeUrl` always interpolates `programKey` into the URL template — passing an empty string produces a URL with an empty path segment (e.g., `/programs//mentees/…`), not an omitted segment. This makes the 'renders mentees with execution of fallback keys' test validate fabricated mock behavior rather than the component's real behavior, giving a false positive.

If the intention is to improve the URL for empty keys, that logic should be added to the real `getMenteeUrl` (with a corresponding unit test for it), not smuggled into a mock.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2026
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/__tests__/unit/components/CardDetailsPage.test.tsx">

<violation number="1" location="frontend/__tests__/unit/components/CardDetailsPage.test.tsx:2396">
P2: The simplified `getMenteeUrl` mock removes the conditional fallback logic that was the purpose of this test case ("renders mentees with execution of fallback keys"). The assertion now expects a URL with double slashes (`/programs//mentees/test_mentee`), meaning the test no longer validates any meaningful fallback behavior—it just confirms empty-string interpolation. The comment on the line above (`// with empty string keys: /mentees/test_mentee`) is also now stale and contradicts this expectation.

Consider keeping the mock's conditional logic, or if the intent is to match real behavior more closely, update the test name, comment, and consider whether this test case is still needed.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2026
@anurag2787
Copy link
Contributor Author

@coderabbitai please review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

@anurag2787: I'll review the changes in this pull request. Let me analyze the test coverage improvements and implementation changes across all the files.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@anurag2787 anurag2787 marked this pull request as ready for review February 15, 2026 11:14
@anurag2787
Copy link
Contributor Author

Hi @kasya the pr is ready for review let me know if any further changes are required.
Thanks!

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Please remove excessive comments from tests

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2026
@anurag2787 anurag2787 requested a review from arkid15r February 15, 2026 18:32
@anurag2787
Copy link
Contributor Author

Hi @arkid15r I have updated it Could you please review. Thanks!

@arkid15r arkid15r enabled auto-merge February 15, 2026 18:42
@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Feb 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.77%. Comparing base (2729855) to head (7cbcab9).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3952      +/-   ##
==========================================
+ Coverage   96.34%   96.77%   +0.42%     
==========================================
  Files         512      512              
  Lines       15826    15821       -5     
  Branches     2132     2167      +35     
==========================================
+ Hits        15248    15310      +62     
+ Misses        431      422       -9     
+ Partials      147       89      -58     
Flag Coverage Δ
backend 95.99% <ø> (ø)
frontend 99.13% <100.00%> (+1.70%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
frontend/src/app/chapters/[chapterKey]/page.tsx 100.00% <ø> (+3.57%) ⬆️
...ontend/src/app/projects/dashboard/metrics/page.tsx 100.00% <100.00%> (+0.88%) ⬆️
frontend/src/components/EntityActions.tsx 100.00% <100.00%> (+7.95%) ⬆️
frontend/src/components/ItemCardList.tsx 100.00% <100.00%> (+11.36%) ⬆️
frontend/src/components/MetricsCard.tsx 100.00% <100.00%> (+8.33%) ⬆️
frontend/src/components/MultiSearch.tsx 100.00% <100.00%> (+2.34%) ⬆️
frontend/src/components/ProgramForm.tsx 97.75% <100.00%> (+3.43%) ⬆️
frontend/src/components/TruncatedText.tsx 100.00% <100.00%> (+5.26%) ⬆️

... and 23 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2729855...7cbcab9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!

@arkid15r arkid15r added this pull request to the merge queue Feb 15, 2026
Merged via the queue into OWASP:main with commit 383e9f3 Feb 15, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Frontend Test Coverage (<95%)

2 participants