Skip to content

test(ui): characterize DataTable behavior before shadcn reskin - #32208

Merged
ryan-crabbe-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_shadcn_datatable_tests
Jul 7, 2026
Merged

test(ui): characterize DataTable behavior before shadcn reskin#32208
ryan-crabbe-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_shadcn_datatable_tests

Conversation

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Tests only; no runtime surface. cd ui/litellm-dashboard && npx vitest run src/components/view_logs/table.test.tsx shows 11 passing (2 pre-existing sizing tests plus 9 new)

Type

✅ Test

Changes

The shared view_logs DataTable (TanStack under a Tremor skin, ~10 consumers) is about to be re-skinned onto shadcn Table primitives as part of the ShadCN migration's tables track. Per the migration plan, coverage gets front-loaded before the swap: the test is the contract, written before the refactor

This extends the existing sizing-only test file to pin the behaviors consumers actually rely on, using Testing-Library role/text queries that are component-library agnostic so the exact same tests must pass after the tremor-to-shadcn reskin: loading and empty state messages, plain TanStack ColumnDefs with custom cell renderers, the onRowClick payload, both expansion render paths (colspan sub-component and sibling child rows), the getRowCanExpand gate, and client-side sorting both disabled (default) and enabled

No production code changes

Pins the shared view_logs DataTable contract with library-agnostic
queries ahead of the tremor-to-shadcn table migration: loading and
empty states, TanStack column defs with custom cell renderers,
onRowClick payload, both expansion render paths (colspan sub-component
and sibling child rows), the getRowCanExpand gate, and client-side
sorting on and off. These must pass unchanged after the reskin.
@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR front-loads test coverage for the shared view_logs DataTable before a planned Tremor-to-shadcn reskin, using Testing-Library role/text queries that are component-library agnostic so the same contract must hold post-swap.

  • Extends the existing sizing test suite with 9 new tests covering loading state, empty state, custom cell renderers, onRowClick payload, both expansion render paths (colspan sub-component and sibling child-rows), the getRowCanExpand gate, and client-side sorting (disabled default vs. enabled).
  • No production code changes; all additions are pure test logic in table.test.tsx.

Confidence Score: 5/5

Pure test additions with no production code changes; safe to merge.

Only change is new test coverage in a single test file. The tests accurately reflect the component's actual API and use library-agnostic queries. The one gap — the child-rows test never performs a collapse + assert-gone step — is a minor contract incompleteness, not a wrong assertion or a regression.

No files require special attention; the child-rows expansion test would benefit from a collapse assertion to fully mirror the colspan test.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/view_logs/table.test.tsx Adds 9 new behavioral tests covering loading/empty states, custom cell renderers, onRowClick payload, both expansion paths (colspan and child-rows), the getRowCanExpand gate, and client-side sorting. The child-rows expansion test is missing a collapse/hide assertion, leaving that path's toggle contract slightly weaker than the colspan path.

Reviews (2): Last reviewed commit: "test(ui): assert child rows hidden befor..." | Re-trigger Greptile

Comment thread ui/litellm-dashboard/src/components/view_logs/table.test.tsx
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

@greptileai re review

@ryan-crabbe-berri
ryan-crabbe-berri enabled auto-merge (squash) July 7, 2026 19:05
@ryan-crabbe-berri
ryan-crabbe-berri merged commit 8c0e3c0 into litellm_internal_staging Jul 7, 2026
124 checks passed
@ryan-crabbe-berri
ryan-crabbe-berri deleted the litellm_shadcn_datatable_tests branch July 7, 2026 19:06
@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 30 untouched benchmarks


Comparing litellm_shadcn_datatable_tests (545cd3b) with litellm_internal_staging (7d15f2f)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (1280126) during the generation of this report, so 7d15f2f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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