test(twenty-ui): add unit tests for Checkbox, Toggle, Banner, Status#19066
test(twenty-ui): add unit tests for Checkbox, Toggle, Banner, Status#19066jnMetaCode wants to merge 1 commit intotwentyhq:mainfrom
Conversation
…tus components Add the first component-level unit tests for twenty-ui, covering four core components across input and display categories: - Checkbox: rendering states, prop variants (size/shape/accent), change handlers, indeterminate state, disabled state, className forwarding - Toggle: default/checked rendering, onChange callback, sizes, disabled state, custom id and className - Banner: children rendering, default/danger variants, complex children, className - Status: text rendering, color variants, unknown color fallback, onClick handler, weight options, className These tests use the existing Jest + @testing-library infrastructure that was previously only used for utility/hook tests. Closes twentyhq#19058
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-ui/src/display/status/components/__tests__/Status.test.tsx">
<violation number="1" location="packages/twenty-ui/src/display/status/components/__tests__/Status.test.tsx:31">
P2: Behavior-labeled tests for color fallback/weight do not assert prop-driven behavior, so regressions in these contracts can pass unnoticed.</violation>
</file>
<file name="packages/twenty-ui/src/display/banner/components/__tests__/Banner.test.tsx">
<violation number="1" location="packages/twenty-ui/src/display/banner/components/__tests__/Banner.test.tsx:15">
P2: Variant tests are non-discriminating: they don't assert any variant-dependent output, so regressions in `variant` handling would pass.</violation>
</file>
<file name="packages/twenty-ui/src/input/components/__tests__/Checkbox.test.tsx">
<violation number="1" location="packages/twenty-ui/src/input/components/__tests__/Checkbox.test.tsx:114">
P2: Indeterminate checkbox test is too broad: it only checks for any SVG, not the minus icon specific to indeterminate state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| <Status color={'unknown' as any} text="Unknown" />, | ||
| ); | ||
|
|
||
| expect(container.firstChild).toBeInTheDocument(); |
There was a problem hiding this comment.
P2: Behavior-labeled tests for color fallback/weight do not assert prop-driven behavior, so regressions in these contracts can pass unnoticed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-ui/src/display/status/components/__tests__/Status.test.tsx, line 31:
<comment>Behavior-labeled tests for color fallback/weight do not assert prop-driven behavior, so regressions in these contracts can pass unnoticed.</comment>
<file context>
@@ -0,0 +1,86 @@
+ <Status color={'unknown' as any} text="Unknown" />,
+ );
+
+ expect(container.firstChild).toBeInTheDocument();
+ expect(screen.getByText('Unknown')).toBeInTheDocument();
+ });
</file context>
| it('renders with default variant', () => { | ||
| const { container } = render(<Banner>Default banner</Banner>); | ||
|
|
||
| expect(container.firstChild).toBeInTheDocument(); |
There was a problem hiding this comment.
P2: Variant tests are non-discriminating: they don't assert any variant-dependent output, so regressions in variant handling would pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-ui/src/display/banner/components/__tests__/Banner.test.tsx, line 15:
<comment>Variant tests are non-discriminating: they don't assert any variant-dependent output, so regressions in `variant` handling would pass.</comment>
<file context>
@@ -0,0 +1,58 @@
+ it('renders with default variant', () => {
+ const { container } = render(<Banner>Default banner</Banner>);
+
+ expect(container.firstChild).toBeInTheDocument();
+ expect(screen.getByText('Default banner')).toBeInTheDocument();
+ });
</file context>
|
|
||
| const svgElements = container.querySelectorAll('svg'); | ||
|
|
||
| expect(svgElements.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
P2: Indeterminate checkbox test is too broad: it only checks for any SVG, not the minus icon specific to indeterminate state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-ui/src/input/components/__tests__/Checkbox.test.tsx, line 114:
<comment>Indeterminate checkbox test is too broad: it only checks for any SVG, not the minus icon specific to indeterminate state.</comment>
<file context>
@@ -0,0 +1,137 @@
+
+ const svgElements = container.querySelectorAll('svg');
+
+ expect(svgElements.length).toBeGreaterThan(0);
+ });
+
</file context>
Summary
packages/twenty-ui, covering 4 core components (Checkbox, Toggle, Banner, Status) with 30+ test cases@testing-library/reactinfrastructure that was previously only used for utility/hook testsTest plan
nx test twenty-uiCloses #19058