test: raise global line coverage from 57% to 86% - #213
Conversation
Add ~330 unit tests across the layers that had little or no coverage: - pure libs and API helpers: transforms, format, period-to-date-range, analytics-client, status, peers, scoring, derivations, metric-order - the react-query layer: team-view, ic-dashboard, sales-dashboard, v2/ic-extras, v2/team-extras - widgets and hooks: use-period, feature-flags, period-selector-bar, counters-block, bullet-chart, metric-card, person-header, kpi-strip, collapsible-section, dashboard-empty-state - screens and shell: sales-dashboard, ic-dashboard router, team-view-v2 gaps, app-sidebar, sidebar-v2-settings, metric-timeseries-view/table, theme-provider, theme-switcher, error-fallback, catalog-provider, mock-banner, ic-view-toggle Also exclude vendored shadcn/ui primitives (src/components/ui/**) and thin TanStack Router wrappers (src/routes/**) from coverage: neither is project logic, and testing them adds noise rather than signal. No production code changes. Remaining uncovered lines are almost entirely the legacy v1 dashboards (kept as is on purpose) and a handful of defensive branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <roman.mitasov@constructor.tech>
|
Warning Review limit reached
Next review available in: 43 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds broad Vitest coverage for API clients, date and formatting utilities, state stores, data transformations, React Query hooks, dashboard widgets, screen flows, feature flags, and tenant-aware catalog behavior. Coverage exclusions are expanded for UI primitives and router wrappers. ChangesApplication test coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/queries/ic-dashboard.test.ts (1)
110-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffExtract the shared react-query test harness to reduce duplication. All five query suites copy an identical
wrapper()(QueryClient withretry:false,gcTime:0) plus near-identicalodata/ok/err/PAGEbuilders. Hoisting these into a single test util (e.g.src/test/react-query.ts) keeps the harness consistent as it evolves. Optional and deferrable.
src/queries/ic-dashboard.test.ts#L110-L116: movewrapper()(andodata/PAGE) into the shared util and import it here.src/queries/sales-dashboard.test.ts#L78-L84: replace the localwrapper()/odatawith the shared import.src/queries/team-view.test.ts#L83-L89: replace the localwrapper()/ok/err/odatawith the shared import.src/queries/v2/ic-extras.test.ts#L42-L48: replace the localwrapper()/ok/odatawith the shared import.src/queries/v2/team-extras.test.ts#L72-L78: replace the localwrapper()/ok/errwith the shared import.🤖 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/queries/ic-dashboard.test.ts` around lines 110 - 116, Create a shared React Query test utility containing wrapper() and the reusable odata, ok, err, and PAGE builders, preserving their existing behavior. Update src/queries/ic-dashboard.test.ts:110-116, src/queries/sales-dashboard.test.ts:78-84, src/queries/team-view.test.ts:83-89, src/queries/v2/ic-extras.test.ts:42-48, and src/queries/v2/team-extras.test.ts:72-78 to import the shared symbols and remove their duplicated local definitions.src/components/widgets/period-selector-bar.test.tsx (1)
51-51: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTrim the 30s timeout or document why it’s needed
These four calendar-popover tests all use a 30s timeout; if that headroom is genuinely needed in CI, add a short note, otherwise reduce it so slowdowns surface sooner.🤖 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/components/widgets/period-selector-bar.test.tsx` at line 51, Update the four calendar-popover tests in period-selector-bar.test.tsx to use a shorter timeout appropriate for their execution, or add a concise comment documenting why the existing 30-second timeout is required in CI.
🤖 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/components/ic-view-toggle.test.tsx`:
- Around line 9-20: Import the React type namespace, or equivalent
ReactNode/ReactElement types, in the test mocks using React type annotations.
Update src/components/ic-view-toggle.test.tsx lines 9-20 and
src/components/theme-switcher.test.tsx lines 9-40; no other behavior changes are
needed.
---
Nitpick comments:
In `@src/components/widgets/period-selector-bar.test.tsx`:
- Line 51: Update the four calendar-popover tests in
period-selector-bar.test.tsx to use a shorter timeout appropriate for their
execution, or add a concise comment documenting why the existing 30-second
timeout is required in CI.
In `@src/queries/ic-dashboard.test.ts`:
- Around line 110-116: Create a shared React Query test utility containing
wrapper() and the reusable odata, ok, err, and PAGE builders, preserving their
existing behavior. Update src/queries/ic-dashboard.test.ts:110-116,
src/queries/sales-dashboard.test.ts:78-84, src/queries/team-view.test.ts:83-89,
src/queries/v2/ic-extras.test.ts:42-48, and
src/queries/v2/team-extras.test.ts:72-78 to import the shared symbols and remove
their duplicated local definitions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6478e031-9461-408f-a2bb-20b26884f6ea
📒 Files selected for processing (42)
src/api/analytics-client.test.tssrc/api/catalog-provider.test.tsxsrc/api/period-to-date-range.test.tssrc/api/transforms.test.tssrc/components/app-sidebar.test.tsxsrc/components/error-fallback.test.tsxsrc/components/ic-view-toggle.test.tsxsrc/components/mock-banner.test.tsxsrc/components/sidebar-v2-settings.test.tsxsrc/components/theme-provider.test.tsxsrc/components/theme-switcher.test.tsxsrc/components/widgets/bullet-chart.test.tsxsrc/components/widgets/collapsible-section.test.tsxsrc/components/widgets/coming-soon.test.tsxsrc/components/widgets/deal-flow-chart.test.tsxsrc/components/widgets/kpi-strip.test.tsxsrc/components/widgets/metric-card.test.tsxsrc/components/widgets/metric-views/metric-timeseries-presentations.test.tsxsrc/components/widgets/metric-views/metric-timeseries-view.test.tsxsrc/components/widgets/period-selector-bar.test.tsxsrc/components/widgets/person-header.test.tsxsrc/components/widgets/sales-pacing-band.test.tsxsrc/components/widgets/v2/counters-block.test.tsxsrc/components/widgets/v2/dashboard-empty-state.test.tsxsrc/hooks/use-period.test.tssrc/lib/feature-flags.test.tssrc/lib/format.test.tssrc/lib/insight/get-initials.test.tssrc/lib/insight/v2/derivations.test.tssrc/lib/insight/v2/metric-order.test.tssrc/lib/peers.test.tssrc/lib/scoring.test.tssrc/lib/status.test.tssrc/queries/ic-dashboard.test.tssrc/queries/sales-dashboard.test.tssrc/queries/team-view.test.tssrc/queries/v2/ic-extras.test.tssrc/queries/v2/team-extras.test.tssrc/screens/ic-dashboard/index.test.tsxsrc/screens/ic-dashboard/sales-dashboard.test.tsxsrc/screens/team-view-v2.test.tsxvitest.config.ts
| Link: ({ | ||
| to, | ||
| children, | ||
| ...rest | ||
| }: { | ||
| to?: string; | ||
| children?: React.ReactNode; | ||
| } & Record<string, unknown>) => ( | ||
| <a data-to={to} {...(rest as object)}> | ||
| {children} | ||
| </a> | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether a global/UMD React type is available without an explicit import.
fd -e json tsconfig | xargs -I{} sh -c 'echo "== {} =="; cat {}' 2>/dev/null
rg -n "allowUmdGlobalAccess" -g "tsconfig*.json"
rg -n "declare global" -t tsRepository: constructorfabric/insight-front
Length of output: 1894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in src/components/ic-view-toggle.test.tsx src/components/theme-switcher.test.tsx; do
echo "== $f =="
cat -n "$f"
echo
done
echo "== React type references =="
rg -n "\bReact\.(ReactNode|ReactElement)\b|\bimport type React\b|\bimport .*React" src/components/*.test.tsx src/**/*.d.ts src/**/*.ts 2>/dev/null || true
echo "== tsconfig / vitest config files =="
fd -e json -e ts -e mts -e cts 'vitest|vite|tsconfig' . | sed 's#^\./##'Repository: constructorfabric/insight-front
Length of output: 7865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ambient declarations mentioning React =="
rg -n --glob '!node_modules' --glob '!dist' --glob '!build' 'declare\s+global|export\s+as\s+namespace\s+React|namespace\s+React\b|import\s+type\s+React\b' src . 2>/dev/null || true
echo "== package / tsconfig references to react typings =="
rg -n '"types"\s*:\s*\[[^]]*react|`@types/react`|allowUmdGlobalAccess|jsxImportSource' package.json tsconfig*.json src 2>/dev/null || trueRepository: constructorfabric/insight-front
Length of output: 933
Import the React type namespace in these test mocks.
React.ReactNode / React.ReactElement is used here, but neither test file imports React. Add import type React from "react"; (or switch to direct type { ReactNode, ReactElement } imports) in:
src/components/ic-view-toggle.test.tsxsrc/components/theme-switcher.test.tsx
📍 Affects 2 files
src/components/ic-view-toggle.test.tsx#L9-L20(this comment)src/components/theme-switcher.test.tsx#L9-L40
🤖 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/components/ic-view-toggle.test.tsx` around lines 9 - 20, Import the React
type namespace, or equivalent ReactNode/ReactElement types, in the test mocks
using React type annotations. Update src/components/ic-view-toggle.test.tsx
lines 9-20 and src/components/theme-switcher.test.tsx lines 9-40; no other
behavior changes are needed.
…overage Signed-off-by: Roman Mitasov <roman.mitasov@constructor.tech> # Conflicts: # src/screens/team-view-v2.test.tsx
2 participants
What
Adds ~330 unit tests (34 new test files, 8 extended) across the layers that had little or no coverage, and tunes the coverage config. No production code changes.
Coverage added
transforms,format,period-to-date-range,analytics-client,status,peers,scoring,derivations,metric-order,get-initialsteam-view,ic-dashboard,sales-dashboard,v2/ic-extras,v2/team-extrasuse-period,feature-flags,period-selector-bar,counters-block,bullet-chart,metric-card,person-header,kpi-strip,collapsible-section,dashboard-empty-state,coming-soon,deal-flow-chart,sales-pacing-bandsales-dashboard,ic-dashboardrouter,team-view-v2gaps,app-sidebar,sidebar-v2-settings,metric-timeseries-view/-table,theme-provider,theme-switcher,error-fallback,catalog-provider,mock-banner,ic-view-toggleConfig change
vitest.config.tscoverage excludes now also skip:src/components/ui/**— vendored shadcn/ui primitivessrc/routes/**— thin TanStack Router wrappers around screensNeither is project logic; excluding them keeps the global number (and the diff-coverage gate) focused on code we actually own. Worth a second opinion in review — trivially revertable.
Deliberately not covered
The legacy v1 dashboards (
engineering-dashboard.tsx,team-view.tsxand their v1-only widgets/queries, ~320 lines) are left as is — they are slated for removal once the metrics-v2 rollout completes, so investing tests there would be wasted. A handful of files are unreachable from the live import graph (section-trend,treemap-composition,settled-result,menu-item-id) and are candidates for deletion in a follow-up.Verification
pnpm vitest run --coverage(both projects, as CI runs it): 95 files / 657 tests passpnpm typecheckclean,pnpm lintclean (--max-warnings 0)🤖 Generated with Claude Code
Summary by CodeRabbit