test: add tests for Sidebar, Card, and Center widgets#716
Open
paridhijain153 wants to merge 1 commit into
Open
test: add tests for Sidebar, Card, and Center widgets#716paridhijain153 wants to merge 1 commit into
paridhijain153 wants to merge 1 commit into
Conversation
github-actions
Bot
added
area:widgets
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a test suite for the Card widget, verifying that border corners render correctly after rectangle updates, and that render operations complete successfully when titles are supplied via the constructor or updated via setTitle. ChangesCard Widget Tests
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR to TermUI, @paridhijain153.
Before your PR merges:
- ⭐ Star the repo. Required. The
star-checkjob blocks your merge otherwise. - ✅ All checks green:
build,test,typecheck. - 🏷 PR title follows
type: short description. Example:fix: handle empty list. - 🔗 Link your closing issue in the description.
GSSoC 2026 points come from labels after merge:
gssoc:approved. +50 base points.level:beginner/intermediate/advanced/critical. +20 / +35 / +55 / +80.quality:clean/exceptional. x 1.2 / x 1.5.type:*. Stackable bonus.
Your reviewer responds within 48 hours. Ping @Karanjot786 on Discord for urgent help.
🚀 Welcome to the cohort.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/widgets/src/data/Sidebar.test.ts (1)
1-86: ⚡ Quick winConsider adding test coverage for non-Unicode fallback.
All tests currently assume Unicode support. Consider adding test cases that verify ASCII fallback behavior when
caps.unicodeis false, ensuring the widget renders correctly in all terminal environments.📋 Example: Test ASCII fallback for active indicator
it("renders active indicator with ASCII fallback", () => { vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false); const sidebar = new Sidebar([ { label: "Home", active: true }, ]); sidebar.updateRect({ x: 0, y: 0, width: 20, height: 5, }); const screen = new Screen(20, 5); sidebar.render(screen); expect(screen.getLine(0)).toContain(">"); expect(screen.getLine(0)).toContain("Home"); });🤖 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 `@packages/widgets/src/data/Sidebar.test.ts` around lines 1 - 86, Add tests to verify ASCII fallback when terminal unicode is disabled by mocking caps.unicode to false and asserting ASCII characters are rendered; specifically, in Sidebar.test.ts create cases that call vi.spyOn(caps, "unicode", "get").mockReturnValue(false) before constructing Sidebar and rendering, then for the active indicator assert ">" appears instead of "▶" (reference Sidebar.render and the active item in Sidebar), and add similar assertions for any other unicode-only glyphs (e.g., badges/collapsed labels) to confirm ASCII fallbacks render correctly.packages/widgets/src/layout/Card.test.ts (2)
1-56: ⚡ Quick winConsider adding test coverage for non-Unicode border fallback.
The current tests only verify Unicode box-drawing characters. Consider adding test cases that verify ASCII fallback border characters when
caps.unicodeis false, ensuring the widget renders correctly in all terminal environments.📋 Example: Test ASCII fallback for borders
it("renders ASCII border when unicode is unavailable", () => { vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false); const card = new Card(); card.updateRect({ x: 0, y: 0, width: 20, height: 5, }); const screen = new Screen(20, 5); card.render(screen); // Verify ASCII corner characters (adjust based on actual implementation) expect(screen.back[0][0].char).toBe("+"); expect(screen.back[0][19].char).toBe("+"); expect(screen.back[4][0].char).toBe("+"); expect(screen.back[4][19].char).toBe("+"); });🤖 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 `@packages/widgets/src/layout/Card.test.ts` around lines 1 - 56, Add a test that verifies ASCII fallback border rendering when unicode is disabled: spy on caps.unicode to return false (e.g., using vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false)), construct a Card, call card.updateRect(...) with the same size as existing tests, render into a Screen, and assert the corner characters in screen.back match the ASCII fallback (e.g., "+" or whatever Card uses when caps.unicode is false). Reference the Card class and its render and updateRect methods and the caps.unicode getter to locate where to add the test.
25-55: ⚡ Quick winAssert rendered title content, not just absence of exceptions.
Tests on lines 25-38 and 40-55 only verify that rendering doesn't throw, but don't assert that the title actually appears in the rendered output. According to coding guidelines, tests must assert observable behavior or rendered output.
✅ Proposed enhancement: Assert title in output
it("renders without throwing when title is provided", () => { const card = new Card({}, { title: "Settings" }); card.updateRect({ x: 0, y: 0, width: 20, height: 5, }); const screen = new Screen(20, 5); expect(() => card.render(screen)).not.toThrow(); + expect(screen.getLine(0)).toContain("Settings"); });it("updates title via setTitle without throwing", () => { const card = new Card(); card.updateRect({ x: 0, y: 0, width: 20, height: 5, }); card.setTitle("Profile"); const screen = new Screen(20, 5); expect(() => card.render(screen)).not.toThrow(); + expect(screen.getLine(0)).toContain("Profile"); });As per coding guidelines: Tests must be real. Assert observable behavior or rendered output.
🤖 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 `@packages/widgets/src/layout/Card.test.ts` around lines 25 - 55, Replace the “does not throw” assertions with assertions that the rendered output actually contains the title: after calling card.updateRect(...) and card.setTitle("Profile") (or constructing with { title: "Settings" }), call card.render(screen) and then inspect the Screen's rendered output (e.g., a toString(), buffer, or equivalent getter) and assert it includes the expected title string ("Settings" / "Profile"); target the Card methods updateRect, setTitle and render and the Screen's output accessor when adding the assertions.
🤖 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 `@packages/widgets/src/data/Sidebar.test.ts`:
- Around line 26-43: The test directly asserts the Unicode glyph "▶" but doesn't
mock caps.unicode; change the test to spy on caps.unicode getter and return a
deterministic value (use vi.spyOn(caps, 'unicode', 'get').mockReturnValue(true)
for this Unicode assertion) so the Sidebar/Screen rendering respects capability
checks, and ensure the test file includes afterEach(() => vi.restoreAllMocks())
to restore mocks; locate the Sidebar/Screen usage in Sidebar.test.ts and add the
spy and restore logic around the existing "renders active indicator" test.
In `@packages/widgets/src/layout/Card.test.ts`:
- Around line 6-23: The test currently asserts Unicode box-drawing characters
directly but must respect the caps.unicode getter; update the test for the Card
widget to spy on caps.unicode using vi.spyOn(caps, 'unicode', 'get') and set it
to true (or false in a separate test) before creating and rendering the Card so
the widget picks the right border set; specifically, in the test that references
Card.updateRect and card.render(screen) spy and mock caps.unicode (restore
after) and assert Unicode chars when mocked true and ASCII fallback (e.g., '+',
'-', '|') when mocked false, targeting the Card class behavior and Screen output
rather than mutating caps directly.
---
Nitpick comments:
In `@packages/widgets/src/data/Sidebar.test.ts`:
- Around line 1-86: Add tests to verify ASCII fallback when terminal unicode is
disabled by mocking caps.unicode to false and asserting ASCII characters are
rendered; specifically, in Sidebar.test.ts create cases that call vi.spyOn(caps,
"unicode", "get").mockReturnValue(false) before constructing Sidebar and
rendering, then for the active indicator assert ">" appears instead of "▶"
(reference Sidebar.render and the active item in Sidebar), and add similar
assertions for any other unicode-only glyphs (e.g., badges/collapsed labels) to
confirm ASCII fallbacks render correctly.
In `@packages/widgets/src/layout/Card.test.ts`:
- Around line 1-56: Add a test that verifies ASCII fallback border rendering
when unicode is disabled: spy on caps.unicode to return false (e.g., using
vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false)), construct a Card, call
card.updateRect(...) with the same size as existing tests, render into a Screen,
and assert the corner characters in screen.back match the ASCII fallback (e.g.,
"+" or whatever Card uses when caps.unicode is false). Reference the Card class
and its render and updateRect methods and the caps.unicode getter to locate
where to add the test.
- Around line 25-55: Replace the “does not throw” assertions with assertions
that the rendered output actually contains the title: after calling
card.updateRect(...) and card.setTitle("Profile") (or constructing with { title:
"Settings" }), call card.render(screen) and then inspect the Screen's rendered
output (e.g., a toString(), buffer, or equivalent getter) and assert it includes
the expected title string ("Settings" / "Profile"); target the Card methods
updateRect, setTitle and render and the Screen's output accessor when adding the
assertions.
🪄 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 Plus
Run ID: d1f65559-85ad-470d-a25d-89b11fe716ed
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/widgets/src/data/Sidebar.test.tspackages/widgets/src/layout/Card.test.ts
Comment on lines
+26
to
+43
| it("renders active indicator", () => { | ||
| const sidebar = new Sidebar([ | ||
| { label: "Home", active: true }, | ||
| ]); | ||
|
|
||
| sidebar.updateRect({ | ||
| x: 0, | ||
| y: 0, | ||
| width: 20, | ||
| height: 5, | ||
| }); | ||
|
|
||
| const screen = new Screen(20, 5); | ||
| sidebar.render(screen); | ||
|
|
||
| expect(screen.getLine(0)).toContain("▶"); | ||
| expect(screen.getLine(0)).toContain("Home"); | ||
| }); |
There was a problem hiding this comment.
Test Unicode character with vi.spyOn(caps, 'unicode', 'get').
The test asserts the Unicode character ▶ directly, but the Sidebar widget should respect caps.unicode and fall back to an ASCII character when Unicode is unavailable. If the widget correctly implements this pattern, this test will fail in non-Unicode environments.
According to coding guidelines, tests checking Unicode behavior must mock caps using vi.spyOn, never direct mutation.
🔧 Proposed fix: Mock caps.unicode for this test
+import { caps } from "`@termuijs/core`";
+import { vi, afterEach } from "vitest";
+
describe("Sidebar", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
it("renders menu items", () => { it("renders active indicator", () => {
+ vi.spyOn(caps, 'unicode', 'get').mockReturnValue(true);
+
const sidebar = new Sidebar([
{ label: "Home", active: true },As per coding guidelines: Tests must mock caps with vi.spyOn(caps, 'unicode', 'get').mockReturnValue(false) plus afterEach(() => vi.restoreAllMocks()). Never assign caps.unicode = ....
🤖 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 `@packages/widgets/src/data/Sidebar.test.ts` around lines 26 - 43, The test
directly asserts the Unicode glyph "▶" but doesn't mock caps.unicode; change the
test to spy on caps.unicode getter and return a deterministic value (use
vi.spyOn(caps, 'unicode', 'get').mockReturnValue(true) for this Unicode
assertion) so the Sidebar/Screen rendering respects capability checks, and
ensure the test file includes afterEach(() => vi.restoreAllMocks()) to restore
mocks; locate the Sidebar/Screen usage in Sidebar.test.ts and add the spy and
restore logic around the existing "renders active indicator" test.
Comment on lines
+6
to
+23
| it("renders a border around its content", () => { | ||
| const card = new Card(); | ||
|
|
||
| card.updateRect({ | ||
| x: 0, | ||
| y: 0, | ||
| width: 20, | ||
| height: 5, | ||
| }); | ||
|
|
||
| const screen = new Screen(20, 5); | ||
| card.render(screen); | ||
|
|
||
| expect(screen.back[0][0].char).toBe("┌"); | ||
| expect(screen.back[0][19].char).toBe("┐"); | ||
| expect(screen.back[4][0].char).toBe("└"); | ||
| expect(screen.back[4][19].char).toBe("┘"); | ||
| }); |
There was a problem hiding this comment.
Test border characters with vi.spyOn(caps, 'unicode', 'get').
The test asserts Unicode box-drawing characters (┌┐└┘) directly, but the Card widget should respect caps.unicode and fall back to ASCII characters (like +-+|) when Unicode is unavailable. If the widget correctly implements this pattern, this test will fail in non-Unicode environments.
🔧 Proposed fix: Mock caps.unicode for border test
+import { caps } from "`@termuijs/core`";
+import { vi, afterEach } from "vitest";
+
describe("Card", () => {
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
it("renders a border around its content", () => {
+ vi.spyOn(caps, 'unicode', 'get').mockReturnValue(true);
+
const card = new Card();As per coding guidelines: To test caps.unicode or caps.motion behavior, use vi.spyOn, never direct mutation.
📝 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("renders a border around its content", () => { | |
| const card = new Card(); | |
| card.updateRect({ | |
| x: 0, | |
| y: 0, | |
| width: 20, | |
| height: 5, | |
| }); | |
| const screen = new Screen(20, 5); | |
| card.render(screen); | |
| expect(screen.back[0][0].char).toBe("┌"); | |
| expect(screen.back[0][19].char).toBe("┐"); | |
| expect(screen.back[4][0].char).toBe("└"); | |
| expect(screen.back[4][19].char).toBe("┘"); | |
| }); | |
| import { caps } from "`@termuijs/core`"; | |
| import { vi, afterEach } from "vitest"; | |
| describe("Card", () => { | |
| afterEach(() => { | |
| vi.restoreAllMocks(); | |
| }); | |
| it("renders a border around its content", () => { | |
| vi.spyOn(caps, 'unicode', 'get').mockReturnValue(true); | |
| const card = new Card(); | |
| card.updateRect({ | |
| x: 0, | |
| y: 0, | |
| width: 20, | |
| height: 5, | |
| }); | |
| const screen = new Screen(20, 5); | |
| card.render(screen); | |
| expect(screen.back[0][0].char).toBe("┌"); | |
| expect(screen.back[0][19].char).toBe("┐"); | |
| expect(screen.back[4][0].char).toBe("└"); | |
| expect(screen.back[4][19].char).toBe("┘"); | |
| }); |
🤖 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 `@packages/widgets/src/layout/Card.test.ts` around lines 6 - 23, The test
currently asserts Unicode box-drawing characters directly but must respect the
caps.unicode getter; update the test for the Card widget to spy on caps.unicode
using vi.spyOn(caps, 'unicode', 'get') and set it to true (or false in a
separate test) before creating and rendering the Card so the widget picks the
right border set; specifically, in the test that references Card.updateRect and
card.render(screen) spy and mock caps.unicode (restore after) and assert Unicode
chars when mocked true and ASCII fallback (e.g., '+', '-', '|') when mocked
false, targeting the Card class behavior and Screen output rather than mutating
caps directly.
Karanjot786
added
gssoc:approved
Contributor
|
i'd like to work on this under gssoc2026, Please assign this to me |
Karanjot786
force-pushed
the
testing/card-center-sidebar-tests
branch
from
bdc6044 to
fc100c4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added tests for the Card, Center, and Sidebar widgets in
@termuijs/widgets.The tests render widgets to a
Screenand verify expected behavior such as centering, border rendering, active indicators, badges, and collapsed sidebar mode.Related Issue
Closes #147
Which package(s)?
@termuijs/widgets
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/09522da4-acd1-4514-85f6-37ecc5849225Screenshots / Recordings (UI changes)
N/A – Added automated widget tests only.
Notes for the Reviewer
Added test coverage for:
All tests pass locally.
Summary by CodeRabbit