Skip to content

Vitest for ui_card - #2261

Merged
jaylfc merged 1 commit into
devfrom
exec/tsk-dgpyjk
Aug 3, 2026
Merged

Vitest for ui_card#2261
jaylfc merged 1 commit into
devfrom
exec/tsk-dgpyjk

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Vitest for ui_card

Autonomous build of board card tsk-dgpyjk.

Files:
desktop/src/components/ui/tests/card.test.tsx | 367 +++++++++++++++++++++
desktop/src/hooks/use-server-notifications.test.ts | 228 -------------
tests/cluster/test_worker_capacity.py | 180 +++-------
3 files changed, 419 insertions(+), 356 deletions(-)

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9afa5b86-e80a-4556-b1f8-8621679205aa

📥 Commits

Reviewing files that changed from the base of the PR and between 00f888d and 889fd2a.

📒 Files selected for processing (1)
  • desktop/src/components/ui/__tests__/card.test.tsx

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.

❤️ Share

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

@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Reviewed: clean single-file addition - focused vitest for the ui Card family (base classes, className merge, ref forwarding, onClick args, arbitrary props, empty children) across Card/CardHeader/etc. Branch base slightly predates 2259/2260 but the merge-base diff is exactly this one file, so nothing regresses. Merging on green.

@jaylfc
jaylfc enabled auto-merge (squash) August 3, 2026 03:37
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add Vitest coverage for ui Card primitives

🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Add Vitest/RTL unit tests for Card layout primitives (Card/Header/Title/etc.).
• Remove legacy tests for server notifications hook.
• Simplify/refresh pytest coverage for worker capacity parsing and error handling.
Diagram

graph TD
  A["Vitest"] --> B["React Testing Library"] --> C["UI Card components"]
  D["Pytest"] --> E["worker_capacity module"] --> F["btrfs CLI output"]
  E --> G["bees status file"]
  H["Deleted hook test"]
Loading
High-Level Assessment

The direct unit-test approach is appropriate for these presentational primitives and parsing helpers. No alternative strategy meaningfully improves maintainability beyond keeping assertions focused on public behavior (class merge/ref/prop forwarding and expected parsing outcomes).

Files changed (2) +367 / -0 · 1 not counted

Tests (2) +367 / -0
card.test.tsxAdd Vitest/RTL unit tests for Card primitives +367/-0

Add Vitest/RTL unit tests for Card primitives

• Adds a comprehensive Vitest suite for Card, CardHeader, CardTitle, CardDescription, CardContent, and CardFooter. Covers base Tailwind classes, className merging, ref forwarding, arbitrary prop forwarding (ARIA/roles), click handler wiring, and empty-children rendering.

desktop/src/components/ui/tests/card.test.tsx

test_worker_capacity.pyRefine pytest coverage for worker capacity parsing and failure modes not counted

Refine pytest coverage for worker capacity parsing and failure modes

• Updates/streamlines tests around btrfs pool size parsing, bees dedup total parsing, and capacity snapshot output. Includes coverage for error return codes, missing commands/files, timeouts, and size unit parsing via parametrization.

tests/cluster/test_worker_capacity.py

@@ -0,0 +1,367 @@
import { describe, it, expect, vi } from "vitest";
import { render, fireEvent, screen } from "@testing-library/react";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Unused import screen from @testing-library/react

The screen import is included but never referenced in the file. This triggers lint/TS warnings and can mask actual missing imports as the test suite grows.

Suggested change
import { render, fireEvent, screen } from "@testing-library/react";
import { render, fireEvent } from "@testing-library/react";

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
WARNING 1
Issue Details (click to expand)

WARNING

File Line Issue
desktop/src/components/ui/__tests__/card.test.tsx 2 Unused import screen from @testing-library/react
Files Reviewed (1 files)
  • desktop/src/components/ui/__tests__/card.test.tsx - 1 issue

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash · Input: 44K · Output: 2.1K · Cached: 263.9K

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-ultra-kilo review

VERDICT: Minor style issues, no blocking issues found.

  • desktop/src/components/ui/tests/card.test.tsx:367 - Missing newline at end of file
  • desktop/src/components/ui/tests/card.test.tsx:1-367 - Repetitive test patterns across 6 describe blocks (base classes, className, ref, onClick, arbitrary props, empty children) could be DRYed up with helper functions or parameterized tests
  • desktop/src/components/ui/tests/card.test.tsx:14,34,etc - Using container.firstChild as HTMLElement is fragile; prefer screen.getByTestId() or container.querySelector('[data-testid="..."]') for robust element selection
    VERDICT: Minor style issues, no blocking issues found.
  • desktop/src/components/ui/tests/card.test.tsx:367 - Missing newline at end of file
  • desktop/src/components/ui/tests/card.test.tsx:1-367 - Repetitive test patterns across 6 describe blocks (base classes, className, ref, onClick, arbitrary props, empty children) could be DRYed up with helper functions or parameterized tests
  • desktop/src/components/ui/tests/card.test.tsx:14,34,etc - Using container.firstChild as HTMLElement is fragile; prefer screen.getByTestId() or container.querySelector('[data-testid="..."]') for robust element selection

Automated first-pass review by the nemotron-ultra-kilo lane. The lead still reviews before merge.

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-ultra-orB review

VERDICT: Needs fixes - ref forwarding tests are incorrect, missing newline at EOF, and onClick tests on non-interactive elements

  • desktop/src/components/ui/tests/card.test.tsx:30 - Ref forwarding test uses plain object { current: null } instead of React's createRef() or useRef(), so ref.current will never be set by React
  • desktop/src/components/ui/tests/card.test.tsx:40 - Testing onClick on Card (div) tests implementation detail; Card is not inherently interactive
  • desktop/src/components/ui/tests/card.test.tsx:77 - Same ref issue for CardHeader
  • desktop/src/components/ui/tests/card.test.tsx:87 - onClick on CardHeader (div) - not semantically interactive
  • desktop/src/components/ui/tests/card.test.tsx:124 - Same ref issue for CardTitle
  • desktop/src/components/ui/tests/card.test.tsx:134 - onClick on CardTitle (heading) - headings shouldn't be clickable
  • desktop/src/components/ui/tests/card.test.tsx:171 - Same ref issue for CardDescription
  • desktop/src/components/ui/tests/card.test.tsx:181 - onClick on CardDescription (text) - not interactive
  • desktop/src/components/ui/tests/card.test.tsx:218 - Same ref issue for CardContent
  • desktop/src/components/ui/tests/card.test.tsx:228 - onClick on CardContent (div) - not inherently interactive
  • desktop/src/components/ui/tests/card.test.tsx:265 - Same ref issue for CardFooter
  • desktop/src/components/ui/tests/card.test.tsx:275 - onClick on CardFooter (div) - not inherently interactive
  • desktop/src/components/ui/tests/card.test.tsx:367 - Missing newline at end of file

Automated first-pass review by the nemotron-ultra-orB lane. The lead still reviews before merge.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 35 rules

Grey Divider


Informational

1. Unused screen import 🐞 Bug ⚙ Maintainability
Description
screen is imported from @testing-library/react but never referenced in this test file, adding
dead code and noise.
Code

desktop/src/components/ui/tests/card.test.tsx[R1-3]

+import { describe, it, expect, vi } from "vitest";
+import { render, fireEvent, screen } from "@testing-library/react";
+import { Card, CardHeader, CardTitle, CardDescription, CardContent, CardFooter } from "../card";
Relevance

●●● Strong

Trivial cleanup; team has accepted small maintainability nits/typos in tests and comments.

PR-#396

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The import list includes screen, but the file’s tests use container.firstChild/fireEvent and
do not reference screen anywhere else.

desktop/src/components/ui/tests/card.test.tsx[1-3]
desktop/src/components/ui/tests/card.test.tsx[34-44]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`screen` is imported but unused in `card.test.tsx`, which adds dead code and can trigger unused-import tooling.

### Issue Context
This file uses `container.firstChild` for element access and never calls `screen.*` APIs.

### Fix Focus Areas
- desktop/src/components/ui/__tests__/card.test.tsx[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Misleading click test names 🐞 Bug ⚙ Maintainability
Description
Multiple tests are titled "fires onClick with correct args" but only assert the handler call count,
so the name overstates what is verified and can mislead future readers about coverage.
Code

desktop/src/components/ui/tests/card.test.tsx[R34-44]

+  it("fires onClick with correct args", () => {
+    const handleClick = vi.fn();
+    const { container } = render(
+      <Card data-testid="card" onClick={handleClick}>
+        <div>child</div>
+      </Card>
+    );
+    const card = container.firstChild as HTMLElement;
+    fireEvent.click(card);
+    expect(handleClick).toHaveBeenCalledTimes(1);
+  });
Relevance

●●● Strong

Team has accepted renaming misleading/contradictory test names and strengthening vacuous tests for
clarity.

PR-#364
PR-#507

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
In the shown examples, the test name says it checks "correct args", but the only assertion is
toHaveBeenCalledTimes(1); no argument assertions are present.

desktop/src/components/ui/tests/card.test.tsx[34-44]
desktop/src/components/ui/tests/card.test.tsx[103-113]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Several click-handler tests claim to validate "correct args" but only check `toHaveBeenCalledTimes(1)`. This mismatch makes the suite misleading.

### Issue Context
If the intent is only to verify the click calls the handler, rename the test(s). If the intent is to verify arguments, add an assertion on the event argument (e.g., `expect(handleClick).toHaveBeenCalledWith(expect.anything())` or check `mock.calls[0][0]` shape).

### Fix Focus Areas
- desktop/src/components/ui/__tests__/card.test.tsx[34-44]
- desktop/src/components/ui/__tests__/card.test.tsx[103-113]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment on lines +1 to +3
import { describe, it, expect, vi } from "vitest";
import { render, fireEvent, screen } from "@testing-library/react";
import { Card, CardHeader, CardTitle, CardDescription, CardContent, CardFooter } from "../card";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

1. Unused screen import 🐞 Bug ⚙ Maintainability

screen is imported from @testing-library/react but never referenced in this test file, adding
dead code and noise.
Agent Prompt
### Issue description
`screen` is imported but unused in `card.test.tsx`, which adds dead code and can trigger unused-import tooling.

### Issue Context
This file uses `container.firstChild` for element access and never calls `screen.*` APIs.

### Fix Focus Areas
- desktop/src/components/ui/__tests__/card.test.tsx[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +34 to +44
it("fires onClick with correct args", () => {
const handleClick = vi.fn();
const { container } = render(
<Card data-testid="card" onClick={handleClick}>
<div>child</div>
</Card>
);
const card = container.firstChild as HTMLElement;
fireEvent.click(card);
expect(handleClick).toHaveBeenCalledTimes(1);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

2. Misleading click test names 🐞 Bug ⚙ Maintainability

Multiple tests are titled "fires onClick with correct args" but only assert the handler call count,
so the name overstates what is verified and can mislead future readers about coverage.
Agent Prompt
### Issue description
Several click-handler tests claim to validate "correct args" but only check `toHaveBeenCalledTimes(1)`. This mismatch makes the suite misleading.

### Issue Context
If the intent is only to verify the click calls the handler, rename the test(s). If the intent is to verify arguments, add an assertion on the event argument (e.g., `expect(handleClick).toHaveBeenCalledWith(expect.anything())` or check `mock.calls[0][0]` shape).

### Fix Focus Areas
- desktop/src/components/ui/__tests__/card.test.tsx[34-44]
- desktop/src/components/ui/__tests__/card.test.tsx[103-113]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-super review

VERDICT: Pass

  • No blocking issues found.

Automated first-pass review by the nemotron-super lane. The lead still reviews before merge.

@jaylfc
jaylfc merged commit ef8a8dc into dev Aug 3, 2026
21 checks passed
@jaylfc
jaylfc deleted the exec/tsk-dgpyjk branch August 3, 2026 04:04
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.

1 participant