test: Phase 13.1 — extend coverage to 70% and raise CI thresholds#245
Conversation
Adds dedicated test suites for the lowest-coverage pages (per the wiki Roadmap's Phase 13.1 "extend coverage" item), prioritized by uncovered statement count rather than raw percentage so each file moves the global number the most. - Five previously 0%-coverage pages: DocsPage, EssayBuilderPage, EssayListPage, ModerationQueuePage, StudentLearningPathPage - Six of the largest-gap pages: RubricBuilder (13%→64%, the deepest investment — form view CRUD, designer/WYSIWYG grid, standards/CEFR linking, version history, sync dialog, export menu), ExportPage, StatisticsPage, ComparativeGrading, GradeStudent, SettingsPage Global statement coverage moves from 58.65% to ~67%. All tests use stable module-level mock references for useApp() — fresh array/object literals per call were found to defeat memo/effect dependency arrays in components that sync derived state (StatisticsPage), causing infinite render loops in jsdom. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (29)
📝 WalkthroughWalkthroughThis PR adds new Vitest and React Testing Library test suites for eleven existing pages (ComparativeGrading, DocsPage, EssayBuilderPage, EssayListPage, ExportPage, GradeStudent, ModerationQueuePage, RubricBuilder, SettingsPage, StatisticsPage, StudentLearningPathPage). Each suite mocks AppContext, routing, and i18n; no production code is modified. ChangesPage Test Suites
🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
The initial commit used a mismatched/uncached local Prettier build (worktree node_modules was incomplete — npm ls prettier showed empty). Running npm ci installs the project-pinned version; format:check now passes for all files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/__tests__/ComparativeGrading.test.tsx (1)
1-145: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix Prettier formatting before merge.
CI reports Prettier
--checkfailures for this file. Runprettier --writeto resolve.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/__tests__/ComparativeGrading.test.tsx` around lines 1 - 145, This test file has Prettier formatting violations, so update the formatting in ComparativeGrading.test.tsx to match the project style. Run the formatter on the file and ensure the updated imports, object literals, and long test cases in ComparativeGradingComp/describe blocks are normalized without changing test behavior.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/__tests__/ComparativeGrading.test.tsx`:
- Around line 99-105: The test setup in ComparativeGrading.test.tsx leaves the
Math.random spy active after each run, so restore it in the suite teardown.
Update the beforeEach/afterEach flow around the existing vi.spyOn(Math,
'random') call in ComparativeGradingComp tests to call mockRestore on that spy
after each test, alongside the existing mock cleanup, so the global Math.random
implementation is returned to normal.
- Around line 99-105: Remove the unnecessary per-test dynamic import in
ComparativeGrading.test.tsx by importing ComparativeGradingComp statically at
the top of the test file instead of reloading it in beforeEach; keep the
existing mock setup in beforeEach, but drop the await
import('../ComparativeGrading') and assignment so the tests use the hoisted
vi.mock behavior with no async overhead.
In `@src/pages/__tests__/DocsPage.test.tsx`:
- Line 35: The test file is using a redundant per-test dynamic import for
DocsPage. Since vi.mock is hoisted, switch the repeated await
import('../DocsPage') usage in DocsPage.test.tsx to a single top-level import of
DocsPage, and update the affected tests to use that shared import instead. Keep
the mock setup intact and ensure the tests still reference DocsPage
consistently.
- Around line 34-54: The current DocsPage tests only assert tab labels that are
always rendered in the left nav, so they do not validate active-tab switching.
Update the tests in DocsPage.test.tsx to assert unique active-state content from
DocsPage, such as the breadcrumb active label or text rendered by the selected
TAB_CONTENT components like RouteMapTab, RubricsTab, GradingTab, CEFRTab,
EssaysTab, AnalyticsTab, and DataTab. Keep the default-render test and the
it.each cases, but make each one verify the content that changes when
setActiveTab/onClick updates activeTab rather than checking the nav label alone.
In `@src/pages/__tests__/EssayListPage.test.tsx`:
- Around line 117-127: The confirm-button lookup in EssayListPage.test.tsx is
too fragile because it depends on className matching and can silently skip the
click when the selector changes. Update the test to select the confirmation
action by a stable semantic identifier instead, using the rendered button’s role
and accessible name (or a dedicated test id/translation key), and remove the
optional guard so the test fails clearly if the confirm control is not found.
Keep the changes localized around the delete flow in EssayListPage.test.tsx and
preserve the final assertion on mockDeleteEssayGroup.
- Around line 118-120: The test in EssayListPage.test.tsx has redundant explicit
act() wrapping around fireEvent.click, since fireEvent from
`@testing-library/react` already handles act internally. Remove the unnecessary
act(async () => { ... }) wrapper around the deleteBtn click, and apply the same
cleanup to the other identical occurrence mentioned in the test so the
assertions still run after the event.
- Around line 100-105: The test named in EssayListPage.test.tsx suggests
navigation behavior, but it only clicks the monitor action and never verifies
routing. Update the “lists an essay group and navigates to edit it” test to
assert the navigation spy/mock (mockNavigate) is called after clicking the
`essays.action_monitor` element, using the existing `renderWithRouter` setup and
`EssayListPage` import so the test checks the intended edit navigation instead
of just rendering.
In `@src/pages/__tests__/ExportPage.test.tsx`:
- Around line 1-282: Prettier check is failing on this test file due to
formatting drift. Reformat the entire `ExportPage.test.tsx` file with `prettier
--write`, keeping the existing test logic intact and ensuring the imports,
mocks, and `describe('ExportPage')` block match Prettier’s style.
- Line 198: The inline comment near the rubric-students test setup is redundant
because it only restates what the code does, so remove it or replace it with a
brief note explaining any non-obvious reason for expanding that section. Update
the test around the relevant setup in ExportPage.test.tsx so the remaining
comments only capture intent that is not already clear from the identifiers and
assertions.
- Line 200: The tests in ExportPage are using non-null assertions on the result
of .find() before fireEvent.click, which can produce an unhelpful
click-on-undefined failure if no matching TD or BUTTON is found. Update the
affected assertions in ExportPage.test.tsx to first store the .find() result
from screen.getAllByText('Alice') (and the other matching text cases), assert it
is present with a clear expectation such as toBeTruthy() or toBeInTheDocument(),
and only then pass it to fireEvent.click so failures surface with a readable
test message.
In `@src/pages/__tests__/GradeStudent.test.tsx`:
- Around line 1-202: The test file has Prettier formatting mismatches that are
causing CI failures. Reformat the entire GradeStudent test module with the
project’s Prettier settings, keeping the existing test logic intact; focus on
the imports, object/array literals, and long mock/setup lines in
GradeStudent.test.tsx so the file passes prettier --check.
- Around line 164-172: The test in GradeStudent.test.tsx only exercises one
checkbox, so it does not actually verify the anchor toggle it claims to cover.
Update the test to deterministically locate both checkbox controls used by
GradeStudent (using the same labels/translation keys as in GradeStudent.tsx),
remove the fragile fallback to checkboxes[0], and click/assert each checkbox
separately. Ensure the assertions confirm both feedback-only and anchor states
change as expected.
- Around line 68-90: Type the mockAppValue object in GradeStudent.test.tsx
against the वास्तविक AppContext value type exported from the AppContext module
so it stays aligned with useApp()’s shape under strict mode. Add the explicit
context type annotation to mockAppValue and update the import to use the
exported AppContext value type name, ensuring any future field renames/removals
are caught by TypeScript.
In `@src/pages/__tests__/RubricBuilder.test.tsx`:
- Around line 552-556: The “copies a criterion” test is only checking that the
UI stays on the edit page, not that clipboard writing actually happens. Expose
the inline `saveCriterionClipboard` stub as a top-level
`mockSaveCriterionClipboard` alongside `mockLoadCriterionClipboard`, then wire
`renderEdit` to use it. Update the `copies a criterion` test in
`RubricBuilder.test.tsx` to assert the mock was called with the selected
criterion after clicking `rubricBuilder.action_copy_criterion`.
- Around line 113-139: The mocked context override object is too loosely typed,
weakening strict TypeScript checks in the RubricBuilder test. Update
appOverrides in RubricBuilder.test.tsx to use the actual useApp return type (for
example, Partial<ReturnType<typeof useApp>> or the app context type) so
overrides like rubrics, settings, and other app fields are validated at compile
time. Keep the existing vi.mock for AppContext, but ensure the spread of
appOverrides remains type-safe and aligned with the mocked useApp shape.
- Around line 469-478: The restore-version test is missing an assertion for the
page reload and leaves a broken window.location stub behind. In
RubricBuilder.test.tsx, update the restore flow test around
renderEditWithVersions, confirmSpy, and mockRestoreRubricVersion to assert
reloadSpy is called after the confirm/restore action, and add cleanup to restore
the original window.location after the test so later tests can still access
href/pathname normally.
In `@src/pages/__tests__/SettingsPage.test.tsx`:
- Around line 77-84: The TemplateUploadModal mock is defined but never used, so
the SettingsPage test suite is missing coverage for the import-submission modal
flow. Add or update a test in SettingsPage.test.tsx that opens the modal through
the SettingsPage UI, asserts the template-upload-modal renders, and verifies the
onClose path by clicking the mocked Close Upload button and checking the modal
disappears. Use the SettingsPage render flow and the TemplateUploadModal mock to
exercise both open and close behavior.
- Around line 129-135: The SettingsPage test only verifies that
mockAddGradeScale is called, but not that it receives the correct new-scale
payload. Strengthen the test in SettingsPage.test.tsx by asserting the argument
passed to mockAddGradeScale matches the expected Omit<GradeScale, 'id'> shape
when clicking settings.action_new_scale from the Teaching tab, so regressions in
the addGradeScale payload are caught.
- Around line 100-105: Replace the custom local render helper in
SettingsPage.test.tsx with the shared renderWithProviders test utility. The
current renderPage setup using createMemoryRouter and RouterProvider is
unnecessary for the /settings route because it has no params; update the test to
render SettingsPageComp through renderWithProviders instead, and remove the
bespoke router-based helper.
- Around line 146-151: The SettingsPage section controls are using button
elements with tab-like state, which is invalid ARIA. Update SettingsPage to use
proper tab semantics by placing the section controls in a tablist and marking
each control as role="tab" with the correct selected state, or, if this is meant
to stay as plain buttons, remove aria-selected and use button-appropriate state
instead. Adjust the related SettingsPage test to assert the intended semantics
for the control identified by “Administration.”
In `@src/pages/__tests__/StatisticsPage.test.tsx`:
- Around line 127-132: Replace the local router-based render helper in
StatisticsPage.test.tsx with the shared renderWithProviders test utility. The
current renderPage uses createMemoryRouter and RouterProvider unnecessarily for
a static /statistics route; update the test to render StatisticsPageComp through
renderWithProviders so it follows the test-utils pattern used for
context-enabled components, and remove the one-off router setup.
- Around line 181-195: The compare-mode test in StatisticsPage is using a
conditional check around the insights toggle, so it can pass without actually
verifying the expand behavior. Update the test in StatisticsPage.test.tsx so the
presence of the insights control is asserted explicitly before clicking it, and
then assert the expected expanded insights content/state after the click. Use
the existing helpers and selectors around renderPage, waitForCharts, and the
statistics.insights.title text to keep the assertion mandatory.
---
Outside diff comments:
In `@src/pages/__tests__/ComparativeGrading.test.tsx`:
- Around line 1-145: This test file has Prettier formatting violations, so
update the formatting in ComparativeGrading.test.tsx to match the project style.
Run the formatter on the file and ensure the updated imports, object literals,
and long test cases in ComparativeGradingComp/describe blocks are normalized
without changing test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da11872f-175f-4954-8161-2a3157074591
📒 Files selected for processing (11)
src/pages/__tests__/ComparativeGrading.test.tsxsrc/pages/__tests__/DocsPage.test.tsxsrc/pages/__tests__/EssayBuilderPage.test.tsxsrc/pages/__tests__/EssayListPage.test.tsxsrc/pages/__tests__/ExportPage.test.tsxsrc/pages/__tests__/GradeStudent.test.tsxsrc/pages/__tests__/ModerationQueuePage.test.tsxsrc/pages/__tests__/RubricBuilder.test.tsxsrc/pages/__tests__/SettingsPage.test.tsxsrc/pages/__tests__/StatisticsPage.test.tsxsrc/pages/__tests__/StudentLearningPathPage.test.tsx
Coverage Report
File CoverageNo changed files found. |
- ComparativeGrading: switch to static import; restore Math.random spy in afterEach - DocsPage: static import; stronger tab-switch assertions (count before vs after) - EssayListPage: assert navigate() after clicking the edit button (Link vs navigate); use translation-key-based confirm button selector; proper async act() wrapping - ExportPage: add pre-assertion before .find()!-click patterns; remove what-not-why comment - GradeStudent: assert both feedback-only AND anchor checkboxes in toggle test - RubricBuilder: expose saveCriterionClipboard mock + assert it; restore window.location after version-restore test and assert reloadSpy was called - SettingsPage: add TemplateUploadModal open/close test; objectContaining on addGradeScale; switch to renderWithRouter helper - StatisticsPage: switch to renderWithRouter; replace optional insights guard with explicit assertion (compare.prompt absent when 2 classes selected) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 22 test files / file extensions covering previously zero- or low-coverage pages, components, and utilities. Raises global statement coverage from ~58% (start of phase) to 70%, then tightens the CI thresholds in vite.config.ts from 52/51/42/43 → 65/65/60/58 (lines/statements/functions/branches). New test files: - QuestionEditor (all 11 question types, CEFR/standards linking) - GlobalSearch, RouteSkeleton, ResponsesGrid, ui components - components.lowcoverage (PresenceBadge, LiveDraftPanel, MultiClassTrendChart) - ActivityDashboard, AttachmentsPage, MarketplacePage, PeerReviewView - RubricList, SelfAssessPage, SpeakingSession, StudentPortalPage - StudentProfilePage (speaking-session section), StudentsPage - clozeParse, seededShuffle utils Extended tests: - TestListPage: results panel + empty state - ModerationQueuePage: accept-second-marker + view-baseline actions - SettingsPage: additional branch coverage Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1 tests - Run Prettier on 7 test files flagged by CI format check - Remove unused `fireEvent`, `act` imports from StudentPortalPage.test.tsx - Remove unused `mockRubricsArr` variable from StudentPortalPage.test.tsx - Remove unused `parseClozeGaps`, `parseHotTextFragments` imports from clozeParse.test.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed the 3 CodeQL findings:
Fixed in commit c9b3109. 🤖 Addressed by Claude Code |
The mock already uses mockRubricWithCriteria (which has criteria defined). mockRubric (no criteria) was a leftover from an earlier draft. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Removed unused 🤖 Addressed by Claude Code |
Summary
New test files
Extended tests
Test plan
🤖 Generated with Claude Code