chore(unit test): expand unit test coverage across HTTP layer, services, custom hooks and utilities - #212
chore(unit test): expand unit test coverage across HTTP layer, services, custom hooks and utilities#212ginaxu1 wants to merge 3 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📝 WalkthroughWalkthroughThe PR adds Vitest coverage for frontend HTTP handling, application and consignment services, authentication and debounce hooks, and date formatting. It also adds an explicit return type to ChangesFrontend test coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/hooks/useDebounce.test.ts (1)
19-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the debounce boundary and stale-timer cancellation.
The test advances directly to 400 milliseconds after one rerender. It would pass if the hook used a shorter delay. It would also pass if an old timer were not cleared, because the old and new callbacks can leave
"world"as the final value. Advance to 399 milliseconds, perform a second rerender, and assert that the intermediate value is not published. This protects the cleanup contract infrontend/src/hooks/useDebounce.ts, Lines 3-17.Suggested timing assertions
rerender({ val: 'world' }) - // Before delay, still initial value - expect(result.current).toBe('hello') - act(() => { - vi.advanceTimersByTime(400) + vi.advanceTimersByTime(399) }) + expect(result.current).toBe('hello') - expect(result.current).toBe('world') + rerender({ val: 'later' }) + act(() => { + vi.advanceTimersByTime(1) + }) + expect(result.current).toBe('hello') + + act(() => { + vi.advanceTimersByTime(399) + }) + expect(result.current).toBe('later')🤖 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 `@frontend/src/hooks/useDebounce.test.ts` around lines 19 - 35, Update the “updates debounced value after specified delay” test to cover the exact 400ms boundary and stale-timer cancellation: advance 399ms after the first rerender, rerender with a second value, assert the initial value remains, then advance the remaining delay and assert only the second value is published. Preserve the existing immediate-value assertion.
🤖 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 `@frontend/src/http.test.ts`:
- Line 1: Run Prettier across the changed frontend test files, including
http.test.ts and service.test.ts, and commit the resulting formatting changes;
remove the file-level eslint suppression only if it becomes unnecessary after
formatting.
In `@frontend/src/utils/date.test.ts`:
- Around line 10-14: Update the valid-date test around formatDateForTable to set
a deterministic locale and time zone, then assert the complete expected
formatted result for August 10, 2026 rather than only checking the year.
Preserve the existing input and ensure the assertion verifies all date
components.
---
Nitpick comments:
In `@frontend/src/hooks/useDebounce.test.ts`:
- Around line 19-35: Update the “updates debounced value after specified delay”
test to cover the exact 400ms boundary and stale-timer cancellation: advance
399ms after the first rerender, rerender with a second value, assert the initial
value remains, then advance the remaining delay and assert only the second value
is published. Preserve the existing immediate-value assertion.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92794950-81b4-45d2-93d7-264978d5e2be
📒 Files selected for processing (8)
frontend/src/features/application/service.test.tsfrontend/src/features/consignment/hooks/useConsignmentList.test.tsfrontend/src/features/consignment/service.test.tsfrontend/src/features/user/hooks/useSignOutHandler.test.tsfrontend/src/hooks/useDebounce.test.tsfrontend/src/http.test.tsfrontend/src/http.tsfrontend/src/utils/date.test.ts
| @@ -0,0 +1,84 @@ | |||
| /* eslint-disable @typescript-eslint/no-unsafe-assignment */ | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
npx prettier --check \
frontend/src/http.test.ts \
frontend/src/features/application/service.test.ts \
frontend/src/features/consignment/service.test.ts \
frontend/src/features/consignment/hooks/useConsignmentList.test.tsRepository: OpenNSW/nsw-agency
Length of output: 746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Changed frontend test files by git diff --stat:\n'
git diff --stat -- '*.test.ts' || true
printf '\nChanged frontend test files by git diff --name-only:\n'
git diff --name-only -- '*.test.ts' || true
printf '\nPrettier check for tracked frontend test files (no external deps):\n'
if [ -f frontend/package.json ]; then
if [ -f frontend/node_modules/.bin/prettier ]; then
./frontend/node_modules/.bin/prettier --check frontend/src/http.test.ts frontend/src/features/application/service.test.ts frontend/src/features/consignment/service.test.ts frontend/src/features/consignment/hooks/useConsignmentList.test.ts || true
else
printf 'frontend/node_modules/.bin/prettier not present; using npx prettier --bin\n'
npx --yes prettier --check frontend/src/http.test.ts frontend/src/features/application/service.test.ts frontend/src/features/consignment/service.test.ts frontend/src/features/consignment/hooks/useConsignmentList.test.ts || true
fi
else
printf 'frontend/package.json not present; using npx prettier --check\n'
npx --yes prettier --check frontend/src/http.test.ts frontend/src/features/application/service.test.ts frontend/src/features/consignment/service.test.ts frontend/src/features/consignment/hooks/useConsignmentList.test.ts || true
fi
printf '\nPrettier diffs for changed frontend test files:\n'
rm -rf /tmp/prettier-diffs
pnpm list -g prettier 2>/dev/null | sed -n '1,20p' || true
npx --yes prettier --no-config --print-width=120 --write /tmp/prettier-diffs --stdin-filepath frontend/src/http.test.ts < frontend/src/http.test.ts > /tmp/prettier-diffs/http.test.ts
diff -u frontend/src/http.test.ts /tmp/prettier-diffs/http.test.ts | sed -n '1,160p' || true
npx --yes prettier --no-config --print-width=120 --write /tmp/prettier-diffs --stdin-filepath frontend/src/features/application/service.test.ts < frontend/src/features/application/service.test.ts > /tmp/prettier-diffs/application.service.test.ts
diff -u frontend/src/features/application/service.test.ts /tmp/prettier-diffs/application.service.test.ts | sed -n '1,160p' || trueRepository: OpenNSW/nsw-agency
Length of output: 1026
Restore Prettier compliance for the changed frontend tests.
frontend/src/http.test.ts and frontend/src/features/application/service.test.ts fail Prettier formatting; run Prettier in the frontend workspace and commit the result so CI formatting passes.
🧰 Tools
🪛 GitHub Actions: Frontend CI / 1_Quality Check & Build.txt
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
🪛 GitHub Actions: Frontend CI / Quality Check & Build
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues.
🤖 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 `@frontend/src/http.test.ts` at line 1, Run Prettier across the changed
frontend test files, including http.test.ts and service.test.ts, and commit the
resulting formatting changes; remove the file-level eslint suppression only if
it becomes unnecessary after formatting.
Source: Pipeline failures
2b003e6 to
b213fdc
Compare
afa6c3f to
622d80d
Compare
…e,s custom hooks and utilites
…t and ensure prettier compliance
Drop the sign-out pass-through case and redundant debounce/date assertions, and tighten the HTTP client tests around auth, query params, and error handling. Co-authored-by: Cursor <cursoragent@cursor.com>
622d80d to
5b8dd01
Compare
Following #155, expanded unit test coverage to frontend HTTP layer, services, custom hooks, and utilities
Changes Made
HTTP Client
src/http.test.tshttp.requestattachesAuthorization: Bearer <token>whenattachToken: true.undefined/nullparameters.HTTP error! status: 404).Consignment Service
src/features/consignment/service.test.tsfetchConsignmentsformatsq,page, andpageSizeparameters correctly.Consignment Hook
src/features/consignment/hooks/useConsignmentList.test.tstotal,totalPages).Sign-Out Handler
src/features/user/hooks/useSignOutHandler.test.tssignoutRedirect()invocation via OIDC authentication context.Date Formatter src/utils/date.test.ts`
"-") and valid ISO date string formatting.Debounce Hook
src/hooks/useDebounce.test.tsApplication API Service
src/features/application/service.test.tsfetchApplications,fetchApplicationDetail,submitReview,submitFeedback, andgetDownloadUrl.Verification
pnpm test:run— PASS (17/17 passed)pnpm type-check— PASS (0 errors)pnpm lint— PASS (0 errors, 0 warnings)pnpm build— PASSSummary by CodeRabbit
Tests
Refactor