feat: add accessibility tests for pages#3420
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConsolidates jest-axe matcher registration into global Jest setup, removes per-file matcher imports from many component a11y tests, adds ~33 new page-level accessibility tests, expands the a11y test scope in package.json, and applies small accessibility attribute changes across several UI components and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 14
🤖 Fix all issues with AI agents
In `@frontend/__tests__/a11y/pages/ApiKeysPage.a11y.test.tsx`:
- Around line 45-55: Test lacks a mock for useQuery so the "Create New Key"
button may not render; in the 'should have no violations when Create Modal is
open' test, call the mockUseQuery helper (or jest mock for useQuery) to return
the expected data/loading state that causes ApiKeysPage to render the Create New
Key button before rendering <ApiKeysPage />, then proceed to click the button
and assert the dialog; reference mockUseQuery and the test name to locate where
to add the setup.
- Around line 57-67: The test fails to set up data and uses a fragile button
selector: initialize the mock data for the page by calling the existing
mockUseQuery (or appropriate test data helper) to return API keys before
rendering ApiKeysPage so revoke buttons exist; then replace the fragile
revokeButtons[0] selector with a specific query that targets the revoke control
(e.g., screen.getByRole('button', { name: /revoke/i }) or within the ApiKey list
item using getByText to locate the key and querySelector/getByRole scoped to
that item) and keep the waitFor asserting the dialog via getByRole('dialog') to
ensure the click opened the confirmation.
In `@frontend/__tests__/a11y/pages/CommitteeDetails.a11y.test.tsx`:
- Line 22: Tests render CommitteeDetails without the required Next.js params
prop; update each render call of CommitteeDetails (all occurrences in this test:
the calls around lines where render(<CommitteeDetails />) appears) to pass a
params prop shaped as Promise.resolve({ committeeKey: '<some-key>' }) (or an
async function returning that object) so the component receives params:
Promise<{ committeeKey: string }>; apply this change to all four render
invocations so the component receives a realistic committeeKey during tests.
In `@frontend/__tests__/a11y/pages/CreateProgram.a11y.test.tsx`:
- Around line 23-35: Replace the global afterAll teardown with afterEach to
clear mocks between tests (change afterAll(() => jest.clearAllMocks()) to
afterEach(...)) so the useApolloClient mock set in beforeEach cannot leak;
additionally, update the loading test to explicitly stub useQuery and
useMutation (mockReturnValue or jest.Mock for useQuery/useMutation) to return a
loading: true state so the loading UI is exercised rather than inheriting the
previous test's data-backed return values.
In `@frontend/__tests__/a11y/pages/EditProgram.a11y.test.tsx`:
- Around line 51-61: The loading-state test for EditProgramPage only mocks
useSession but not useQuery, which can break the component if useQuery is called
unconditionally; update the test to also mock useQuery (the hook used in the
component) to return a loading-like shape (e.g., { data: undefined, isLoading:
true } or matching the component's expected fields) before rendering
EditProgramPage, using the same jest.Mock pattern as useSession so the component
receives consistent loading state from both hooks.
In `@frontend/__tests__/a11y/pages/ModuleDetails.a11y.test.tsx`:
- Around line 66-76: The "no data exists" test currently uses mockUseQuery
returning { data: null, loading: true, error: null } which duplicates the
"loading" case; update the mock in the "should have no accessibility violations
when no data exists" test (the mockUseQuery call) to return loading: false
(e.g., { data: null, loading: false, error: null }) so it represents a completed
query with no data for the ModuleDetailsPage render and keep the rest of the
test (rendering ModuleDetailsPage, running axe, and expect toHaveNoViolations)
unchanged.
In `@frontend/__tests__/a11y/pages/ModuleIssueDetailsPage.a11y.test.tsx`:
- Around line 103-137: The tests for ModuleIssueDetailsPage call the component
without mocking useIssueMutations, which can cause failures; add a consistent
mock for the useIssueMutations hook (e.g., mockUseIssueMutations) and set its
mockReturnValue before each test (move mockUseQuery and useIssueMutations setup
into a beforeEach) so loading/error/no-data cases all have a predictable
useIssueMutations return value; locate references to useIssueMutations and
mockUseQuery in the test file to update their setup and ensure the mock provides
the expected methods/shape the component uses.
In `@frontend/__tests__/a11y/pages/ProgramDetailsMentorship.a11y.test.tsx`:
- Line 2: The test imports mockProgramDetailsData as a default but the module
exports it as a named export; update the import in
ProgramDetailsMentorship.a11y.test.tsx to use a named import for
mockProgramDetailsData from the '@mockData/mockProgramData' module (i.e.,
replace the default-style import with a named import) so the test receives the
actual mockProgramDetailsData value.
In `@frontend/__tests__/a11y/pages/ProjectDetails.a11y.test.tsx`:
- Around line 8-11: The test mocks next/navigation with useParams returning the
wrong key; update the jest.mock call so useParams (in the mock defined for
next/navigation) returns an object with projectKey instead of programKey to
match the ProjectDetailsPage route (app/projects/[projectKey]/page) and ensure
any assertions referencing useParams or ProjectDetailsPage receive the correct
param name.
In `@frontend/__tests__/a11y/pages/ProjectsHealthDashboardOverview.a11y.test.tsx`:
- Line 48: The test description in the it(...) for the
ProjectsHealthDashboardOverview a11y test has a typo ("exits" → "exists");
update the test case string used in the it(...) call in the test function (the
spec that currently reads "should have no accessibility violations when no data
exits") to "should have no accessibility violations when no data exists" so the
description is correct.
In `@frontend/__tests__/a11y/pages/RepositoryDetails.a11y.test.tsx`:
- Around line 12-17: The test's next/navigation mock only returns repositoryKey
but the route also provides organizationKey; update the jest.mock for
'next/navigation' (the useParams mock) to include organizationKey alongside
repositoryKey so components reading organizationKey won't break tests; keep the
existing useRouter mock intact (useRouter, useParams) and ensure the returned
shape matches what the page expects.
In `@frontend/__tests__/a11y/pages/Users.a11y.test.tsx`:
- Around line 14-16: The test mock for fetchAlgoliaData currently returns only {
hits: mockUserData.users } but must match the function contract by also
returning totalPages; update the mockResolvedValue call for fetchAlgoliaData (in
Users.a11y.test.tsx) to return an object shaped like { hits: mockUserData.users,
totalPages: <number> } (e.g. derive totalPages from mockUserData or set to 1) so
the test mirrors the fetchAlgoliaData contract and surfaces any consumer logic
that reads totalPages.
In `@frontend/__tests__/unit/components/IssuesTable.test.tsx`:
- Around line 216-222: Tests in IssuesTable.test.tsx still query avatar alt text
(which was changed to alt="") causing false positives; update any assertions
that look for alt="user1" (including the mobile-view test that references the
assignee) to instead use visible username text or role-based queries (e.g.,
getByText('user1'), getAllByText('user1'), or
getByRole('img')/getByRole('presentation') within the correct container). Locate
the relevant tests referencing IssuesTable, mockIssues, or avatar alt attributes
and replace those alt-based expects with assertions that verify the username is
rendered and/or the avatar element exists using role/text queries.
In `@frontend/src/components/DonutBarChart.tsx`:
- Around line 25-53: The Chart lost its accessible container when the wrapper
div was removed; restore a wrapper element around the <Chart> (inside the
SecondaryCard in DonutBarChart) and give it role="img" and an aria-label (e.g.,
describing the donut like "Donut chart depicting health statuses: Healthy, Need
Attention, Unhealthy" or use the component's title prop) so screen readers can
announce the chart; ensure the wrapper is placed where the commented div used to
be (around Chart) and do not pass ARIA props directly to the ApexCharts <Chart>
component.
♻️ Duplicate comments (16)
frontend/__tests__/a11y/components/ProjectTypeDashboardCard.a11y.test.tsx (1)
1-2: Same global matcher wiring concern as StatusBadge.
Please ensuretoHaveNoViolationsis still registered via Jest setup for this a11y suite.frontend/__tests__/a11y/components/MultiSearch.a11y.test.tsx (1)
1-2: Same global matcher wiring concern as StatusBadge.
Please ensuretoHaveNoViolationsis still registered via Jest setup for this a11y suite.frontend/__tests__/a11y/components/SponsorCard.a11y.test.tsx (1)
1-2: Same global matcher wiring concern as StatusBadge.
Please ensuretoHaveNoViolationsis still registered via Jest setup for this a11y suite.frontend/__tests__/a11y/components/MetricsScoreCircle.a11y.test.tsx (1)
1-2: Same global matcher wiring concern as StatusBadge.
Please ensuretoHaveNoViolationsis still registered via Jest setup for this a11y suite.frontend/__tests__/a11y/components/MetricsCard.a11y.test.tsx (1)
2-43: Same matcher-registration concern as above.
See the note on ensuringtoHaveNoViolationsis registered in Jest setup.frontend/__tests__/a11y/components/RecentPullRequests.a11y.test.tsx (1)
2-66: Same matcher-registration concern as above.
See the note on ensuringtoHaveNoViolationsis registered in Jest setup.frontend/__tests__/a11y/components/RepositoryCard.a11y.test.tsx (1)
2-51: Same matcher-registration concern as above.
See the note on ensuringtoHaveNoViolationsis registered in Jest setup.frontend/__tests__/a11y/components/Modal.a11y.test.tsx (1)
2-40: Same matcher-registration concern as above.
See the note on ensuringtoHaveNoViolationsis registered in Jest setup.frontend/__tests__/a11y/components/CalendarButton.a11y.test.tsx (1)
2-34: Same matcher-registration concern as above.
See the note on ensuringtoHaveNoViolationsis registered in Jest setup.frontend/__tests__/a11y/components/LogoCarousel.a11y.test.tsx (1)
1-3: Same verification note as BarChart.a11y.test.tsx (global matcher setup).frontend/__tests__/a11y/components/SnapshotCard.a11y.test.tsx (1)
1-3: Same verification note as BarChart.a11y.test.tsx (global matcher setup).frontend/__tests__/a11y/pages/Snapshots.a11y.test.tsx (1)
1-22: Same verification note as BarChart.a11y.test.tsx (global matcher setup).frontend/__tests__/a11y/components/SecondaryCard.a11y.test.tsx (1)
1-3: Same verification note as BarChart.a11y.test.tsx (global matcher setup).frontend/__tests__/a11y/components/ActionButton.a11y.test.tsx (1)
2-2: Verify globaltoHaveNoViolationsmatcher availability for this file.frontend/__tests__/a11y/components/AnchorTitle.a11y.test.tsx (1)
2-2: Verify globaltoHaveNoViolationsmatcher availability for this file.frontend/__tests__/a11y/components/BreadCrumbs.a11y.test.tsx (1)
3-3: Verify globaltoHaveNoViolationsmatcher availability for this file.
🧹 Nitpick comments (20)
frontend/__tests__/a11y/pages/MyMentorship.a11y.test.tsx (1)
34-46: Consider adding loading and error state tests for comprehensive a11y coverage.The test covers the successfully-loaded state. However,
loading: trueanderror: {...}states may render different UI elements (spinners, skeletons, error messages) that could have their own accessibility considerations.♻️ Suggested addition for loading and error states
describe('MyMentorshipPage', () => { it('should have no accessibility violations', async () => { ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: mockProgramData, loading: false, error: null, }) const { container } = render(<MyMentorshipPage />) const results = await axe(container) expect(results).toHaveNoViolations() }) + + it('should have no accessibility violations while loading', async () => { + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: undefined, + loading: true, + error: null, + }) + + const { container } = render(<MyMentorshipPage />) + const results = await axe(container) + expect(results).toHaveNoViolations() + }) + + it('should have no accessibility violations on error', async () => { + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: undefined, + loading: false, + error: new Error('Test error'), + }) + + const { container } = render(<MyMentorshipPage />) + const results = await axe(container) + expect(results).toHaveNoViolations() + }) })frontend/__tests__/a11y/pages/CommitteeDetails.a11y.test.tsx (1)
12-64: Add mock cleanup to ensure test isolation.While each test explicitly calls
mockReturnValue, it's best practice to reset mocks between tests to prevent potential state leakage and ensure complete test isolation.♻️ Suggested improvement
Add cleanup hooks to the test suite:
describe('CommitteeDetailsPage Accessibility', () => { const mockUseQuery = useQuery as unknown as jest.Mock + + beforeEach(() => { + jest.clearAllMocks() + }) it('should have no accessibility violations', async () => {frontend/__tests__/a11y/pages/MenteeProfilePage.a11y.test.tsx (3)
11-18: Consider providing a more completeuseRoutermock.
useRouter: jest.fn()returnsundefined, which may cause runtime errors if the component calls router methods likepush()orback(). If the test passes as-is, the component likely doesn't use these methods, but providing a complete mock improves robustness for future changes.💡 Suggested improvement
jest.mock('next/navigation', () => ({ useParams: jest.fn(() => ({ programKey: 'prog1', moduleKey: 'mod1', menteeKey: 'test-mentee', })), - useRouter: jest.fn(), + useRouter: jest.fn(() => ({ + push: jest.fn(), + back: jest.fn(), + replace: jest.fn(), + })), }))
96-105: Add null check before cloning children.
React.Children.mapwill includenullorundefinedchildren in iteration. TheReact.cloneElementcall on line 100 will throw ifchildis not a valid React element.🛠️ Suggested fix
{isOpen && ( <div id="select-popover" data-testid="select-popover" aria-label="Options"> - {React.Children.map(children, (child: React.ReactElement) => { + {React.Children.map(children, (child) => { + if (!React.isValidElement(child)) return null const itemKey = String(child.key ?? '') return React.cloneElement(child, { 'data-key': itemKey, onClick: () => handleItemClick(itemKey), } as Partial<unknown>) })} </div> )}
173-189: Consider usingafterEachand adding tests for loading/error states.Two suggestions for improved test reliability:
afterEachoverafterAll: Currently only one test exists, but if more are added, mocks won't reset between tests, potentially causing flaky failures.Loading and error states may have different accessibility characteristics: Spinners, skeleton loaders, and error messages should also be free of violations.
💡 Suggested improvements
describe('MenteeProfilePage Accessibility', () => { - afterAll(() => { + afterEach(() => { jest.clearAllMocks() }) it('should have no accessibility violations', async () => { ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: mockMenteeData, loading: false, error: null, }) const { container } = render(<MenteeProfilePage />) const results = await axe(container) expect(results).toHaveNoViolations() }) + + it('should have no accessibility violations while loading', async () => { + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: null, + loading: true, + error: null, + }) + + const { container } = render(<MenteeProfilePage />) + + const results = await axe(container) + expect(results).toHaveNoViolations() + }) })frontend/__tests__/a11y/pages/CreateModule.a11y.test.tsx (1)
28-30: PreferafterEachfor mock cleanup to keep tests isolated.
UsingafterAllcan let mock state leak if additional tests are added later in this suite. Consider switching toafterEachfor safer isolation.♻️ Suggested tweak
- afterAll(() => { - jest.clearAllMocks() - }) + afterEach(() => { + jest.clearAllMocks() + })frontend/__tests__/a11y/pages/UserDetails.a11y.test.tsx (1)
47-63: Consider adding test cleanup for consistency.The test works correctly. For consistency with other a11y tests in this PR (e.g.,
Home.a11y.test.tsx), consider adding cleanup:Optional: Add afterAll cleanup
describe('UserDetailsPage Accessibility', () => { + afterAll(() => { + jest.clearAllMocks() + }) + it('should have no accessibility violations', async () => {frontend/__tests__/a11y/pages/OrganizationDetails.a11y.test.tsx (1)
11-22: Consider addingwaitForbefore running axe for consistency.Other page-level a11y tests in this PR (e.g.,
Home.a11y.test.tsx,UserDetails.a11y.test.tsx) usewaitForto ensure the component has fully rendered before running axe checks. While the synchronous mock may work here, addingwaitForimproves test reliability and maintains consistency.Suggested improvement
+import { render, waitFor, screen } from '@testing-library/react' -import { render } from '@testing-library/react'const { container } = render(<OrganizationDetailsPage />) + + await waitFor(() => { + expect(screen.getByText('Test Organization')).toBeInTheDocument() + }) + const results = await axe(container)frontend/__tests__/a11y/pages/Program.a11y.test.tsx (1)
18-20: Consider includingtotalPagesin the mock return value.Based on the
fetchAlgoliaDatafunction signature infrontend/src/server/fetchAlgoliaData.ts, it returns{ hits, totalPages }. While the component may handle the missingtotalPagesgracefully, including it makes the mock more accurate.Suggested improvement
;(fetchAlgoliaData as jest.Mock).mockResolvedValue({ hits: mockPrograms, + totalPages: 1, })frontend/__tests__/a11y/pages/ProjectsHealthDashboardMetrics.a11y.test.tsx (2)
3-3: Inconsistent render import — consider usingwrappers/testUtil.Other accessibility tests in this PR import
renderfromwrappers/testUtil, which likely provides necessary context providers. Using@testing-library/reactdirectly may cause the component to render without required context, potentially affecting test reliability.Suggested fix
-import { render } from '@testing-library/react' +import { render } from 'wrappers/testUtil'
38-40: Consider waiting for content to load before running axe.Other page-level accessibility tests in this PR use
waitForto ensure content is rendered before running accessibility checks. This helps avoid false positives/negatives from testing loading states.Suggested pattern
+import { waitFor, screen } from '@testing-library/react' ... const { container } = render(<MetricsPage />) + await waitFor(() => { + expect(screen.getByText('Project One')).toBeInTheDocument() + }) const results = await axe(container)frontend/__tests__/a11y/pages/IssuesPage.a11y.test.tsx (2)
2-2: Inconsistent render import — consider usingwrappers/testUtil.For consistency with other accessibility tests and to ensure proper context providers are available, consider importing from
wrappers/testUtil.Suggested fix
-import { render } from '@testing-library/react' +import { render } from 'wrappers/testUtil'
54-57: Consider waiting for content to load before running axe.Other page-level tests wait for content to render before running accessibility checks. This ensures the test validates the fully-rendered page rather than a loading/intermediate state.
Suggested pattern
+import { waitFor, screen } from '@testing-library/react' ... const { container } = render(<IssuesPage />) + await waitFor(() => { + expect(screen.getByText('First Issue Title')).toBeInTheDocument() + }) const results = await axe(container)frontend/__tests__/a11y/pages/Chapters.a11y.test.tsx (1)
23-36: Consider adding loading and error state tests for consistency.Other page tests (e.g.,
ModuleDetailsPage.a11y.test.tsx) include tests for loading and error states. For consistency and comprehensive coverage, consider adding similar test cases here.💡 Example additional test cases
it('should have no accessibility violations when loading', async () => { ;(fetchAlgoliaData as jest.Mock).mockImplementation(() => new Promise(() => {})) const { container } = render(<ChaptersPage />) const results = await axe(container) expect(results).toHaveNoViolations() }) it('should have no accessibility violations when error occurs', async () => { ;(fetchAlgoliaData as jest.Mock).mockRejectedValue(new Error('test error')) const { container } = render(<ChaptersPage />) const results = await axe(container) expect(results).toHaveNoViolations() })frontend/__tests__/a11y/pages/EditModule.a11y.test.tsx (1)
19-33: Consider adding loading and error state tests for comprehensive coverage.The test currently only covers the success state. For consistency with other page tests like
ModuleDetailsPage.a11y.test.tsx, consider adding tests for loading and error states.frontend/__tests__/a11y/pages/ModuleDetailsPage.a11y.test.tsx (1)
3-3: Consider using therenderwrapper fromwrappers/testUtilfor consistency.Other page tests in this PR (e.g.,
Chapters.a11y.test.tsx,EditModule.a11y.test.tsx,ProgramDetails.a11y.test.tsx) importrenderfromwrappers/testUtil, while this file imports directly from@testing-library/react. Using the project's wrapper ensures consistent test context (providers, etc.).♻️ Suggested fix
-import { render } from '@testing-library/react' +import { render } from 'wrappers/testUtil'frontend/__tests__/a11y/pages/ProgramDetails.a11y.test.tsx (1)
18-34: Consider adding loading and error state tests for consistency.For comprehensive accessibility coverage, consider adding tests for loading and error states similar to
ModuleDetailsPage.a11y.test.tsx.💡 Example additional test cases
it('should have no accessibility violations when loading', async () => { ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: null, loading: true, error: null, }) const { container } = render(<ProgramDetailsPage />) const results = await axe(container) expect(results).toHaveNoViolations() }) it('should have no accessibility violations when error occurs', async () => { ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: null, loading: false, error: new Error('test error'), }) const { container } = render(<ProgramDetailsPage />) const results = await axe(container) expect(results).toHaveNoViolations() })frontend/__tests__/a11y/pages/ModuleIssueDetailsPage.a11y.test.tsx (1)
2-2: Inconsistent render import.Other page-level a11y tests in this PR (e.g., Organization, Committees, ApiKeysPage) use
renderfromwrappers/testUtil, which likely provides necessary providers. This test imports directly from@testing-library/react, which may cause issues if the component requires context providers.Suggested fix
-import { render } from '@testing-library/react' +import { render } from 'wrappers/testUtil'frontend/__tests__/a11y/pages/ChapterDetails.a11y.test.tsx (1)
22-24: Reset the mockeduseQuerybetween tests for isolation.This mock is shared across multiple tests; resetting it avoids accidental leakage when tests are extended or reordered.
♻️ Suggested update
describe('ChapterDetailsPage Accessibility', () => { const mockUseQuery = useQuery as unknown as jest.Mock + + afterEach(() => { + mockUseQuery.mockReset() + })frontend/__tests__/a11y/pages/About.a11y.test.tsx (1)
133-135: Reset the mockeduseQuerybetween tests for isolation.Keeping this mock clean avoids unintended cross-test coupling as new scenarios are added.
♻️ Suggested update
describe('About Page Accessibility', () => { const mockUseQuery = useQuery as unknown as jest.Mock + + afterEach(() => { + mockUseQuery.mockReset() + })
frontend/__tests__/a11y/pages/ProjectsHealthDashboardOverview.a11y.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/__tests__/a11y/pages/CommitteeDetails.a11y.test.tsx`:
- Around line 18-33: The mockCommitteeDetailsData object used by the
CommitteeDetailsPage test is missing numeric stats that the component renders;
update mockCommitteeDetailsData in the test mocks so the committee entry
includes contributorsCount, forksCount, starsCount, repositoriesCount, and
issuesCount with numeric values (e.g., integers) so CommitteeDetailsPage renders
the stats section fully during the a11y test; ensure the mock object shape
matches what CommitteeDetailsPage expects so useQuery in the test returns the
updated mockCommitteeDetailsData.
In `@frontend/__tests__/a11y/pages/Projects.a11y.test.tsx`:
- Line 1: The test currently uses a default import for mockProjectData which is
exported as a named export; update the import in Projects.a11y.test.tsx to use
the named export (reference symbol: mockProjectData) — i.e., replace the default
import with a named import for mockProjectData from '@mockData/mockProjectData'
(or alternatively add a default export to that module) so mockProjectData is
defined at runtime.
🧹 Nitpick comments (2)
frontend/__tests__/a11y/pages/Committees.a11y.test.tsx (1)
21-26: Consider moving mock setup intobeforeEachorbeforeAll.Configuring the mock at the describe block's top-level scope works, but placing it in a setup hook is more idiomatic and improves test isolation if additional tests are added later.
♻️ Suggested refactor
describe('CommitteesPage Accessibility', () => { - ;(fetchAlgoliaData as jest.Mock).mockResolvedValue({ - hits: mockCommitteeData.committees, - totalPages: 1, - }) + beforeEach(() => { + (fetchAlgoliaData as jest.Mock).mockResolvedValue({ + hits: mockCommitteeData.committees, + totalPages: 1, + }) + }) it('should have no accessibility violations', async () => {frontend/__tests__/a11y/pages/Program.a11y.test.tsx (1)
25-27: PreferfindByTextfor the async wait.This is a bit cleaner and improves failure output.
♻️ Suggested tweak
- await waitFor(() => { - expect(screen.getByText('Program 1')).toBeInTheDocument() - }) + await screen.findByText('Program 1')
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/__tests__/a11y/pages/EditProgram.a11y.test.tsx`:
- Around line 7-16: The test fails to simulate an authenticated user because the
next-auth useSession hook isn't mocked; update the EditProgram.a11y.test.tsx
mocks to include jest.mock('next-auth/react', ...) and provide a useSession mock
that returns an authenticated session object (e.g., { data: { user: { email:
'test@example.com', name: 'Test' }, ... }, status: 'authenticated' }) and any
admin/roles needed by EditProgramPage to treat the user as a program
admin—mirror the mock shape used in CreateProgram.a11y.test.tsx so
EditProgramPage's useSession checks pass during the a11y test.
🧹 Nitpick comments (2)
frontend/__tests__/a11y/pages/EditProgram.a11y.test.tsx (2)
21-26: Consider includinggetProgramModulesin the mock response.The
mockProgramDetailsDatafixture includes bothgetProgramandgetProgramModules, but onlygetProgramis provided here. If the component queries for modules as well, missing this field could cause rendering issues or incomplete accessibility testing.Suggested fix
;(useQuery as unknown as jest.Mock).mockReturnValue({ data: { getProgram: mockProgramDetailsData.getProgram, + getProgramModules: mockProgramDetailsData.getProgramModules, }, loading: false, })
18-32: Consider adding accessibility tests for loading and error states.The current test only covers the successfully loaded state. Testing additional UI states (loading spinner, error message) would improve accessibility coverage, as these states often have different DOM structures and ARIA attributes.
Example additional test cases
it('should have no accessibility violations when loading', async () => { ;(useMutation as unknown as jest.Mock).mockReturnValue([jest.fn(), { loading: false }]) ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: null, loading: true, }) const { container } = render(<EditProgramPage />) const results = await axe(container) expect(results).toHaveNoViolations() }) it('should have no accessibility violations on error', async () => { ;(useMutation as unknown as jest.Mock).mockReturnValue([jest.fn(), { loading: false }]) ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: null, loading: false, error: new Error('Failed to load'), }) const { container } = render(<EditProgramPage />) const results = await axe(container) expect(results).toHaveNoViolations() })
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
This is great 🔥
Thank you @Utkarsh-0304 !
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3420 +/- ##
=======================================
Coverage 85.56% 85.56%
=======================================
Files 461 461
Lines 14227 14227
Branches 1895 1895
=======================================
Hits 12174 12174
Misses 1680 1680
Partials 373 373
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
3 participants
Proposed change
Resolves #3364
This PR introduces accessibility tests for pages, building upon the work done in pr #3162. Also, all the existing accessibility conflicts thus encountered are also fixed.
Checklist
make check-testlocally: all warnings addressed, tests passed