Skip to content

test(components): expand unit tests for 22 components from smoke to variant-complete#123

Open
oyi77 wants to merge 4 commits intoakii09:mainfrom
oyi77:claude/pdfx-contribution-b9G87
Open

test(components): expand unit tests for 22 components from smoke to variant-complete#123
oyi77 wants to merge 4 commits intoakii09:mainfrom
oyi77:claude/pdfx-contribution-b9G87

Conversation

@oyi77
Copy link
Copy Markdown

@oyi77 oyi77 commented Apr 11, 2026

Description

Raises unit test coverage for 22 of the 24 registry components from ~11-line
smoke tests to 30–85 line variant-complete tests. Before this PR only
list.test.tsx and table.test.tsx asserted on every variant / edge case /
style invariant (they caught real Yoga flexbox regressions). The other 22
components only checked .not.toThrow() against one or two prop combinations,
so variant renames, theme schema changes, and flex: shorthand mistakes could
land without CI signal. This PR closes that gap.

Purely additive — no component source, style, type, or registry file is
touched.

Related Issue

N/A — no existing tracking issue. Happy to open a follow-up issue if preferred.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • 🚀 New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 📖 Documentation update
  • 🔧 Refactor / chore (no functional changes)

Changes Made

  • Group A — layout primitives (text, heading, link, divider,
    stack, page-break, keep-together): enumerate every variant / weight /
    tracking / decoration / transform / alignment, exercise style overrides, and
    assert on return values where the component is a pure marker (page-break).
  • Group B — content components (alert, badge, card, section,
    key-value, signature): enumerate variants × sizes; assert the documented
    PdfAlert null-return when both title and children are missing
    (alert.tsx:193); exercise Badge label-takes-precedence-over-children
    rule (badge.tsx:131); cover Section border / accentColor variant gating
    (section.tsx:116-120); cover PdfSignatureBlock default-signers fallback
    on the double variant (signature.tsx:139).
  • Group C — page chrome (page-header, page-footer, page-number,
    watermark): render every variant with a full prop set, including
    PageHeader two-column contact info and logo-left/logo-right ReactNode
    logo slot; cover PageFooter sticky-implies-fixed / marginTop-reset-to-0
    branch (page-footer.tsx:236-237); cover PdfPageNumber custom format
    templates (including templates with no {page}/{total} tokens); cover
    all five PdfWatermark positions + arbitrary rotation angles + full 0..1
    opacity range.
  • Group D — media and data (pdf-image, qrcode, form, graph,
    data-table): exercise both string and object src shapes in PdfImage
    (including HTTP method + headers + body); cover the aspectRatio → height
    resolver (pdf-image.tsx:125); assert the unsupported-format warning path
    via vi.spyOn(console, 'warn') for .webp and data:image/avif; enumerate
    all 4 QR error correction levels and the backgroundColor='transparent'
    skip branch (qrcode.tsx:84); cover every PdfForm layout with uneven
    column chunks that exercise the chunk-padding branch (form.tsx:71-73);
    cover all 6 PdfGraph variants with both single-series and multi-series
    data shapes plus donut centerLabel, smooth line, and fullWidth sizing;
    call DataTable<Row> with a concrete generic parameter and both custom
    render and renderFooter callbacks, forwarding all 4 Table variants.
  • Split into 4 commits, one per group, for easier review.

Test count: 115 → 187 (+72 tests) across 26 test files.

How Has This Been Tested?

  • Unit tests pass (pnpm test) — 187 passed / 26 files
  • Type checks pass (pnpm typecheck) — clean across all 4 workspace packages
  • Lint passes (pnpm lint) — Biome clean (pnpm format was run to match
    the repo's print width; no biome-ignore directives added)
  • Manually tested locally — also ran pnpm build:registry; the generated
    JSON under apps/www/public/r/ is unchanged since test files aren't
    shipped in the registry

Screenshots (if applicable)

N/A — tests only, no UI or PDF output change.

Checklist

  • My code follows the project's style guidelines (Biome)
  • I have performed a self-review of my code
  • I have added/updated tests that prove my fix or feature works
  • New and existing tests pass locally
  • I have updated relevant documentation (README, CONTRIBUTING.md, etc.) —
    not applicable; no user-facing API changes
  • My commits follow the Conventional Commits format

Notes for reviewers

  • Vitest environment is 'node' (apps/www/vitest.config.ts). Tests call
    components as plain functions — no JSX render, no testing-library, no
    jest-dom. This matches the pre-existing pattern in list.test.tsx and
    table.test.tsx.
  • usePdfxTheme() and useSafeMemo() gracefully fall back to the default
    theme outside a React render tree via the hasActiveDispatcher() guard in
    apps/www/src/registry/lib/pdfx-theme-context.tsx:22-65, so tests don't
    need to wrap calls in a PdfxThemeProvider. Same mechanism list/table
    tests already depend on.
  • list.test.tsx and table.test.tsx are intentionally unchanged — they
    already serve as the reference pattern this PR matches.
  • The style?: Style prop is typed as a single object (not Style | Style[]),
    so tests exercise single-object overrides only — even though the runtime in
    several components calls [style].flat(). Widening the type is out of scope
    for this PR.

Summary by CodeRabbit

Tests

  • Expanded test coverage across multiple components with comprehensive property validation and edge-case scenarios
  • Added iterative coverage for supported variant values, prop combinations, style overrides, and boundary conditions
  • Tests now verify component rendering stability across a broader range of input scenarios without throwing errors

claude added 4 commits April 11, 2026 08:50
Raises 7 smoke-only test files to variant-complete tests covering all
variants, sizes, alignments, weights, decorations, transforms, edge
cases, and style overrides. Affected: text, heading, link, divider,
stack, page-break, keep-together.

All tests call components as plain functions and rely on the existing
usePdfxTheme dispatcher fallback in
apps/www/src/registry/lib/pdfx-theme-context.tsx, matching the pattern
already used by list.test.tsx and table.test.tsx.

https://claude.ai/code/session_016WbuSZRHFTyoJzj5cb7Zkq
Raises 6 smoke-only test files to variant-complete tests covering all
variants, sizes, children handling, and style overrides. Affected:
alert, badge, card, section, key-value, signature.

Notable additions:
- PdfAlert now asserts the documented null-return when title and
  children are both missing (alert.tsx:193).
- Badge exercises the label-takes-precedence-over-children rule from
  badge.tsx:131 and tests background/color overrides via both theme
  tokens and raw hex.
- Section tests the border + accentColor variant gating from
  section.tsx:116-120.
- PdfSignatureBlock covers the default-signers fallback on the double
  variant (signature.tsx:139).

https://claude.ai/code/session_016WbuSZRHFTyoJzj5cb7Zkq
Raises 4 smoke-only test files to variant-complete tests. Affected:
page-header, page-footer, page-number, watermark.

Notable additions:
- PageFooter exercises the sticky-implies-fixed / marginTop-reset-to-0
  branch from page-footer.tsx:236-237 and renders every layout variant
  with a full prop set (including the three-column/detailed branches).
- PageHeader renders every variant including two-column with contact
  info and logo-left/logo-right with a ReactNode logo slot.
- PdfPageNumber covers custom format templates (including templates
  with no {page}/{total} tokens) and both muted modes.
- PdfWatermark exercises the 0..1 opacity range, arbitrary rotation
  angles, and all 5 positions.

https://claude.ai/code/session_016WbuSZRHFTyoJzj5cb7Zkq
Raises 5 smoke-only test files to variant-complete tests. Affected:
pdf-image, qrcode, form, graph, data-table.

Notable additions:
- PdfImage covers both string and object src shapes (including HTTP
  method + headers + body), the aspectRatio-to-height resolver from
  pdf-image.tsx:125, and asserts the unsupported-format warning path
  via a vi.spyOn console.warn check for .webp and data:image/avif.
- PdfQRCode exercises all 4 error correction levels, the
  backgroundColor='transparent' skip branch from qrcode.tsx:84, and
  payload sizes from 1 char to 500 chars.
- PdfForm covers all 4 variants, all 3 column layouts (single,
  two-column, three-column) with uneven field counts that trigger the
  chunk-padding branch at form.tsx:71-73, and the
  title/subtitle-triggered formDivider.
- PdfGraph covers all 6 variants, both single-series and multi-series
  data shapes, all legend positions, donut centerLabel, smooth line
  with no dots, and fullWidth sizing.
- DataTable is tested with a concrete Row generic parameter, empty
  columns/data, custom render/renderFooter callbacks, and all 4 Table
  variants forwarded through data-table.tsx:24.

https://claude.ai/code/session_016WbuSZRHFTyoJzj5cb7Zkq
@oyi77 oyi77 requested a review from akii09 as a code owner April 11, 2026 09:02
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 11, 2026

@claude is attempting to deploy a commit to the akii09's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

Comprehensive test suite expansion across 20+ registry components, replacing minimal smoke tests with broad prop-coverage matrices including enumerated variant/size/layout iterations, prop combinations, edge cases, color handling (tokens and raw values), and style overrides without modifying component implementations.

Changes

Cohort / File(s) Summary
Alert/Badge/Card
apps/www/src/registry/components/alert/alert.test.tsx, badge/badge.test.tsx, card/card.test.tsx
Expanded from basic no-throw checks to comprehensive prop matrices including enumerated variants, size/layout/padding iterations, edge cases (empty props, null children), and style/color overrides.
Data-heavy components
apps/www/src/registry/components/data-table/data-table.test.tsx, graph/graph.test.tsx, key-value/key-value.test.tsx
Added typed fixtures, multi-scenario rendering with various prop combinations (columns/rows, series, layout configs), support verification across all variants/sizes, and optional props (stripe, noWrap, custom rendering).
Divider/Form
apps/www/src/registry/components/divider/divider.test.tsx, form/form.test.tsx
Parameterized coverage for all variant/thickness/spacing/layout values, color handling (token and hex), combined prop testing, and edge cases (missing titles, uneven column chunks).
Headings/Typography
apps/www/src/registry/components/heading/heading.test.tsx, text/text.test.tsx
Enumerated loops across all supported text styling options (weight, tracking, transform, decoration, align), combined prop assertions, and color variants (token and hex).
Layout/Structure components
apps/www/src/registry/components/keep-together/keep-together.test.tsx, section/section.test.tsx, stack/stack.test.tsx
Added enumerated prop coverage (gap, direction, alignment, spacing, padding), edge cases (null/empty children), behavior-gating (border/accentColor conditional on variant), and style overrides.
Page components
apps/www/src/registry/components/page-break/page-break.test.tsx, page-footer/page-footer.test.tsx, page-header/page-header.test.tsx, page-number/page-number.test.tsx
Expanded from single smoke tests to iterative variant/layout coverage, prop combinations (sticky/fixed, contact fields, text blocks), and styling/color/format handling.
Link/Image/QRCode/Signature
apps/www/src/registry/components/link/link.test.tsx, pdf-image/pdf-image.test.tsx, qrcode/qrcode.test.tsx, signature/signature.test.tsx
Added href scheme variants, fit/errorLevel/position enumeration, image format validation (with console.warn spying), multi-signer scenarios, and comprehensive prop matrices.
Watermark
apps/www/src/registry/components/watermark/watermark.test.tsx
Parameterized position/opacity/angle/fontSize coverage, rotation edge cases (negatives, multiples of 90), color variants (token and hex), and fixed/style overrides.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #112: Expanded DataTable tests (column width/alignment scenarios) directly align with table fixed-width column fixes and test improvements in this PR's DataTable test expansion.

Poem

🐰 With whiskers twitching, tests expand wide,
Props tested left, tested right, tested far and tested side,
From variants to colors, from styles to shapes so fine,
Coverage blooms like clover—each component shines!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly summarizes the main change: expanding unit tests for 22 components from smoke tests to variant-complete tests with comprehensive prop coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

@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: 3

🧹 Nitpick comments (5)
apps/www/src/registry/components/qrcode/qrcode.test.tsx (1)

30-37: Avoid hardcoded implementation line references in test comments.

Line-number comments (qrcode.tsx:42-63, qrcode.tsx:84) will go stale quickly and create maintenance noise. Prefer behavior-focused comments without source line anchors.

♻️ Suggested comment cleanup
-    // qrcode.tsx:42-63 — zero margin should still produce a valid matrix.
+    // Zero margin should still produce a valid matrix.
@@
-    // qrcode.tsx:84 — 'transparent' backgroundColor is a special case that
-    // skips the background rect.
+    // 'transparent' backgroundColor is a special case that skips the background rect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/qrcode/qrcode.test.tsx` around lines 30 -
37, Remove hardcoded source line anchors from comments in qrcode.test.tsx and
replace them with behavior-focused notes; update the comments near the PdfQRCode
calls (the zero-margin and color/transparent-background tests) to describe the
expected behavior (e.g., "zero margin should still produce a valid matrix" and
"'transparent' background skips background rect") without referencing
qrcode.tsx:42-63 or qrcode.tsx:84 so they won't go stale.
apps/www/src/registry/components/badge/badge.test.tsx (1)

24-30: Test names overstate behavior currently being asserted.

Line 24 and Line 29 claim precedence/fallback behavior, but the assertions only verify no-throw. Either assert the returned content explicitly or rename these tests to safety-focused wording.

Suggested minimal rename (keeps current assertion style)
-  it('label takes precedence over children (both branches are safe)', () => {
+  it('does not throw when both label and children are provided', () => {
     // badge.tsx:131 — `label ?? children ?? ''`
     expect(() => Badge({ label: 'A', children: 'B' })).not.toThrow();
   });

-  it('renders with neither label nor children (empty text fallback)', () => {
+  it('does not throw with neither label nor children', () => {
     expect(() => Badge({})).not.toThrow();
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/badge/badge.test.tsx` around lines 24 - 30,
Rename the two misleading tests to reflect they only assert safety (no throw)
rather than precedence/fallback: change the first test title to "does not throw
when both label and children are provided" (still calling Badge({ label: 'A',
children: 'B' })) and change the second to "does not throw when neither label
nor children are provided" (still calling Badge({})), or alternatively replace
the no-throw assertions with explicit content checks against Badge's output
(e.g., render Badge({ label: 'A', children: 'B' }) and assert the text equals
the expected precedence value), but at minimum update the test names referencing
Badge to be safety-focused.
apps/www/src/registry/components/pdf-image/pdf-image.test.tsx (2)

54-62: Make unsupported-format warning assertions stricter.

Line 60-Line 61 currently allow false positives from unrelated warnings. Assert that warning messages contain both webp and avif explicitly.

🔧 Suggested assertion tightening
-      expect(warnSpy).toHaveBeenCalled();
-      expect(warnSpy.mock.calls.length).toBeGreaterThanOrEqual(2);
+      const messages = warnSpy.mock.calls.map(([msg]) => String(msg));
+      expect(messages.some((m) => m.includes('"webp"'))).toBe(true);
+      expect(messages.some((m) => m.includes('"avif"'))).toBe(true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/pdf-image/pdf-image.test.tsx` around lines
54 - 62, The test allows unrelated console warnings to satisfy the assertion;
tighten it by asserting that the console.warn calls produced by PdfImage include
explicit mentions of the unsupported formats (check for 'webp' and 'avif')
instead of only counting calls. Locate the test using PdfImage and the
detectFormat/console.warn behavior, then replace the loose
expect(warnSpy).toHaveBeenCalled()/call count check with assertions that at
least one warnSpy.mock.calls entry includes 'webp' and at least one includes
'avif' (or assert specific warnSpy calls/arguments contain those substrings).

20-20: Avoid source line-number references in test comments.

Line 20, Line 50, and Line 55 reference exact implementation line numbers; those comments will go stale quickly after refactors.

🧹 Suggested comment cleanup
-    // pdf-image.tsx:11 — object src supports HTTP method/headers/body.
+    // Object src supports HTTP request options (method/headers/body).

-    // pdf-image.tsx:125 — aspectRatio divides numeric width to compute height.
+    // Height is derived from numeric width and aspectRatio when height is omitted.

-    // pdf-image.tsx:78-85 — detectFormat + console.warn for webp/avif/etc.
+    // Unsupported formats (for example, webp/avif) should trigger a warning.

Also applies to: 50-50, 55-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/pdf-image/pdf-image.test.tsx` at line 20,
Remove any test comments that reference implementation line numbers (e.g.,
comments like "pdf-image.tsx:11 — ...") in pdf-image.test.tsx and replace them
with descriptive, stable notes about behavior or the component under test
(reference PdfImage or the behavior "object src supports HTTP
method/headers/body" instead of a file:line pointer); search for the exact
comment text used at lines 20, 50, and 55 and update those comments to describe
the expectation or behavior being asserted rather than pointing to a specific
source line so they won’t go stale after refactors.
apps/www/src/registry/components/data-table/data-table.test.tsx (1)

69-70: Avoid hardcoded source line references in test comments.

Line 69 (data-table.tsx:24) will drift as the implementation evolves. Prefer a behavior-only note without fixed line numbers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/data-table/data-table.test.tsx` around lines
69 - 70, The test comment currently references a hardcoded source line
("data-table.tsx:24") which will drift; update the comment above the variant
loop (the for (const variant of ['grid','striped','minimal','bordered'] as
const) {...} block in data-table.test.tsx) to remove the file:line reference and
instead describe the intended behaviour (e.g., "variant prop is forwarded to
Table component" or "ensure variant prop is passed through to <Table/>"), so the
note describes behavior only and won't break as implementation lines change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/www/src/registry/components/data-table/data-table.test.tsx`:
- Around line 53-66: The test currently only asserts DataTable<Row>
instantiation does not throw, but doesn't verify that the column render
callbacks are actually invoked; replace the inline renderScore with a jest.fn()
spy (or wrap renderScore) and render the DataTable component (using the same
DataTable component factory or React render from `@testing-library/react`) so that
the table mounts and triggers cell/footer rendering, then assert the spy was
called for cell rendering and for footer rendering (or assert rendered output
contains the formatted values like "264%"); update the test that references
DataTable, renderScore, and the footer prop to assert calls or rendered DOM
instead of only not.toThrow().

In `@apps/www/src/registry/components/heading/heading.test.tsx`:
- Around line 21-27: Update the test title to reflect what it actually asserts
(that Heading does not throw for out-of-range runtime inputs) or else change the
assertions to truly verify clamping behavior; specifically either rename the
test description from "clamps out-of-range levels to the 1-6 window" to
something like "does not throw for out-of-range levels" to match the existing
expect(...).not.toThrow() checks, or modify the expectations to inspect
Heading's output (e.g., check rendered tag/level or a computed class/aria-level
produced by Heading) to assert that values like 0 and 9 are clamped to the 1–6
range; reference the Heading component used in this test to locate where to
update.

In `@apps/www/src/registry/components/page-number/page-number.test.tsx`:
- Around line 24-31: The test currently only checks that PdfPageNumber
construction doesn't throw but doesn't validate token replacement; extract the
internal formatPageNumber logic from the PdfPageNumber component into a new
exported helper (e.g., page-number.utils.ts -> export function
formatPageNumber(template: string, page: number, total: number)), update
PdfPageNumber to import and use that helper, then add unit tests that call
formatPageNumber directly to assert "{page}" and "{total}" are replaced
correctly (and that strings without tokens are returned unchanged);
alternatively, if you prefer not to export the util, mock the PDFText component
and assert its render callback is invoked with a function that produces the
expected formatted strings when given a fake PDF render context.

---

Nitpick comments:
In `@apps/www/src/registry/components/badge/badge.test.tsx`:
- Around line 24-30: Rename the two misleading tests to reflect they only assert
safety (no throw) rather than precedence/fallback: change the first test title
to "does not throw when both label and children are provided" (still calling
Badge({ label: 'A', children: 'B' })) and change the second to "does not throw
when neither label nor children are provided" (still calling Badge({})), or
alternatively replace the no-throw assertions with explicit content checks
against Badge's output (e.g., render Badge({ label: 'A', children: 'B' }) and
assert the text equals the expected precedence value), but at minimum update the
test names referencing Badge to be safety-focused.

In `@apps/www/src/registry/components/data-table/data-table.test.tsx`:
- Around line 69-70: The test comment currently references a hardcoded source
line ("data-table.tsx:24") which will drift; update the comment above the
variant loop (the for (const variant of ['grid','striped','minimal','bordered']
as const) {...} block in data-table.test.tsx) to remove the file:line reference
and instead describe the intended behaviour (e.g., "variant prop is forwarded to
Table component" or "ensure variant prop is passed through to <Table/>"), so the
note describes behavior only and won't break as implementation lines change.

In `@apps/www/src/registry/components/pdf-image/pdf-image.test.tsx`:
- Around line 54-62: The test allows unrelated console warnings to satisfy the
assertion; tighten it by asserting that the console.warn calls produced by
PdfImage include explicit mentions of the unsupported formats (check for 'webp'
and 'avif') instead of only counting calls. Locate the test using PdfImage and
the detectFormat/console.warn behavior, then replace the loose
expect(warnSpy).toHaveBeenCalled()/call count check with assertions that at
least one warnSpy.mock.calls entry includes 'webp' and at least one includes
'avif' (or assert specific warnSpy calls/arguments contain those substrings).
- Line 20: Remove any test comments that reference implementation line numbers
(e.g., comments like "pdf-image.tsx:11 — ...") in pdf-image.test.tsx and replace
them with descriptive, stable notes about behavior or the component under test
(reference PdfImage or the behavior "object src supports HTTP
method/headers/body" instead of a file:line pointer); search for the exact
comment text used at lines 20, 50, and 55 and update those comments to describe
the expectation or behavior being asserted rather than pointing to a specific
source line so they won’t go stale after refactors.

In `@apps/www/src/registry/components/qrcode/qrcode.test.tsx`:
- Around line 30-37: Remove hardcoded source line anchors from comments in
qrcode.test.tsx and replace them with behavior-focused notes; update the
comments near the PdfQRCode calls (the zero-margin and
color/transparent-background tests) to describe the expected behavior (e.g.,
"zero margin should still produce a valid matrix" and "'transparent' background
skips background rect") without referencing qrcode.tsx:42-63 or qrcode.tsx:84 so
they won't go stale.
🪄 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: 7a63cc97-5752-4604-9faf-3eeaae96a291

📥 Commits

Reviewing files that changed from the base of the PR and between 46807ae and eaa83d0.

📒 Files selected for processing (22)
  • apps/www/src/registry/components/alert/alert.test.tsx
  • apps/www/src/registry/components/badge/badge.test.tsx
  • apps/www/src/registry/components/card/card.test.tsx
  • apps/www/src/registry/components/data-table/data-table.test.tsx
  • apps/www/src/registry/components/divider/divider.test.tsx
  • apps/www/src/registry/components/form/form.test.tsx
  • apps/www/src/registry/components/graph/graph.test.tsx
  • apps/www/src/registry/components/heading/heading.test.tsx
  • apps/www/src/registry/components/keep-together/keep-together.test.tsx
  • apps/www/src/registry/components/key-value/key-value.test.tsx
  • apps/www/src/registry/components/link/link.test.tsx
  • apps/www/src/registry/components/page-break/page-break.test.tsx
  • apps/www/src/registry/components/page-footer/page-footer.test.tsx
  • apps/www/src/registry/components/page-header/page-header.test.tsx
  • apps/www/src/registry/components/page-number/page-number.test.tsx
  • apps/www/src/registry/components/pdf-image/pdf-image.test.tsx
  • apps/www/src/registry/components/qrcode/qrcode.test.tsx
  • apps/www/src/registry/components/section/section.test.tsx
  • apps/www/src/registry/components/signature/signature.test.tsx
  • apps/www/src/registry/components/stack/stack.test.tsx
  • apps/www/src/registry/components/text/text.test.tsx
  • apps/www/src/registry/components/watermark/watermark.test.tsx

Comment on lines +53 to +66
it('invokes custom cell render and footer render', () => {
const renderScore = (value: unknown) =>
typeof value === 'number' ? `${value}%` : String(value ?? '');
expect(() =>
DataTable<Row>({
columns: [
{ key: 'name', header: 'Name' },
{ key: 'score', header: 'Score', render: renderScore, renderFooter: renderScore },
],
data,
footer: { score: 264 },
})
).not.toThrow();
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Test intent and assertion are mismatched for render callbacks.

Line 53 says callback invocation is tested, but the test only checks not.toThrow. This can pass even if render/renderFooter are never called.

Suggested fix
 it('invokes custom cell render and footer render', () => {
-  const renderScore = (value: unknown) =>
-    typeof value === 'number' ? `${value}%` : String(value ?? '');
+  let cellRenderCalled = false;
+  let footerRenderCalled = false;
+  const formatScore = (value: unknown) =>
+    typeof value === 'number' ? `${value}%` : String(value ?? '');
   expect(() =>
     DataTable<Row>({
       columns: [
         { key: 'name', header: 'Name' },
-        { key: 'score', header: 'Score', render: renderScore, renderFooter: renderScore },
+        {
+          key: 'score',
+          header: 'Score',
+          render: (value) => {
+            cellRenderCalled = true;
+            return formatScore(value);
+          },
+          renderFooter: (value) => {
+            footerRenderCalled = true;
+            return formatScore(value);
+          },
+        },
       ],
       data,
       footer: { score: 264 },
     })
   ).not.toThrow();
+  expect(cellRenderCalled).toBe(true);
+  expect(footerRenderCalled).toBe(true);
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/data-table/data-table.test.tsx` around lines
53 - 66, The test currently only asserts DataTable<Row> instantiation does not
throw, but doesn't verify that the column render callbacks are actually invoked;
replace the inline renderScore with a jest.fn() spy (or wrap renderScore) and
render the DataTable component (using the same DataTable component factory or
React render from `@testing-library/react`) so that the table mounts and triggers
cell/footer rendering, then assert the spy was called for cell rendering and for
footer rendering (or assert rendered output contains the formatted values like
"264%"); update the test that references DataTable, renderScore, and the footer
prop to assert calls or rendered DOM instead of only not.toThrow().

Comment on lines +21 to 27
it('clamps out-of-range levels to the 1-6 window', () => {
// level is typed as 1-6 but runtime clamps Math.round(level) to [1,6].
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 0 as any, children: 'under' })).not.toThrow();
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 9 as any, children: 'over' })).not.toThrow();
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Test title over-claims clamping behavior.

Line 21 says clamping is verified, but Lines 24 and 26 only assert non-throw. This can pass even if clamping regresses.

Suggested minimal fix (rename to match actual assertion)
-  it('clamps out-of-range levels to the 1-6 window', () => {
+  it('does not throw for out-of-range levels', () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('clamps out-of-range levels to the 1-6 window', () => {
// level is typed as 1-6 but runtime clamps Math.round(level) to [1,6].
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 0 as any, children: 'under' })).not.toThrow();
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 9 as any, children: 'over' })).not.toThrow();
});
it('does not throw for out-of-range levels', () => {
// level is typed as 1-6 but runtime clamps Math.round(level) to [1,6].
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 0 as any, children: 'under' })).not.toThrow();
// biome-ignore lint/suspicious/noExplicitAny: intentional out-of-range runtime input
expect(() => Heading({ level: 9 as any, children: 'over' })).not.toThrow();
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/heading/heading.test.tsx` around lines 21 -
27, Update the test title to reflect what it actually asserts (that Heading does
not throw for out-of-range runtime inputs) or else change the assertions to
truly verify clamping behavior; specifically either rename the test description
from "clamps out-of-range levels to the 1-6 window" to something like "does not
throw for out-of-range levels" to match the existing expect(...).not.toThrow()
checks, or modify the expectations to inspect Heading's output (e.g., check
rendered tag/level or a computed class/aria-level produced by Heading) to assert
that values like 0 and 9 are clamped to the 1–6 range; reference the Heading
component used in this test to locate where to update.

Comment on lines +24 to +31
it('accepts custom format strings', () => {
// page-number.tsx:55 — formatPageNumber replaces {page} and {total} in the template.
expect(() => PdfPageNumber({ format: '{page} / {total}' })).not.toThrow();
expect(() => PdfPageNumber({ format: 'Page {page}' })).not.toThrow();
expect(() => PdfPageNumber({ format: 'Sheet {page} of {total}' })).not.toThrow();
// No tokens — render function still runs and returns the literal string.
expect(() => PdfPageNumber({ format: 'Footer' })).not.toThrow();
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether the format test currently invokes/asserts the PDFText render callback output.
rg -nP --type=tsx -C2 'it\(\x27accepts custom format strings\x27' apps/www/src/registry/components/page-number/page-number.test.tsx
rg -nP --type=tsx -C2 '\.props\.render|render\(\{\s*pageNumber|\btoBe\(' apps/www/src/registry/components/page-number/page-number.test.tsx

Repository: akii09/pdfx

Length of output: 110


🏁 Script executed:

#!/bin/bash
# Read the test file to see exact implementation
echo "=== Test file content ==="
cat -n apps/www/src/registry/components/page-number/page-number.test.tsx

echo -e "\n=== Component implementation ==="
cat -n apps/www/src/registry/components/page-number/page-number.tsx

Repository: akii09/pdfx

Length of output: 5647


🏁 Script executed:

#!/bin/bash
# Check if formatPageNumber is exported or used elsewhere
rg -n "formatPageNumber" apps/www/src/registry/components/page-number/
rg -n "export.*formatPageNumber" --type ts apps/www/src/

Repository: akii09/pdfx

Length of output: 531


Test does not assert format token replacement behavior.

The test at lines 24–31 only confirms component construction doesn't throw; it does not verify that {page} and {total} tokens are correctly replaced in the format string. The formatPageNumber function (line 55, component) is internal and not exported, making direct unit testing difficult. The PDFText render callback is PDF context-dependent and cannot be tested by accessing props directly.

To strengthen this test, consider one of these approaches:

  1. Export formatPageNumber to a page-number.utils.ts file and test it directly
  2. Mock the PDFText component and verify the render callback is called with the expected format
  3. Add snapshot tests to capture rendered output
  4. Move assertion to integration tests with actual PDF rendering if available

The suggested diff in the original comment would fail because PDFText's render callback requires PDF rendering context, not a plain object.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/registry/components/page-number/page-number.test.tsx` around
lines 24 - 31, The test currently only checks that PdfPageNumber construction
doesn't throw but doesn't validate token replacement; extract the internal
formatPageNumber logic from the PdfPageNumber component into a new exported
helper (e.g., page-number.utils.ts -> export function formatPageNumber(template:
string, page: number, total: number)), update PdfPageNumber to import and use
that helper, then add unit tests that call formatPageNumber directly to assert
"{page}" and "{total}" are replaced correctly (and that strings without tokens
are returned unchanged); alternatively, if you prefer not to export the util,
mock the PDFText component and assert its render callback is invoked with a
function that produces the expected formatted strings when given a fake PDF
render context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants