feat(ci): Add 50% code coverage quality gate - #84
Conversation
Enable code coverage thresholds in vitest.config.ts that block CI: - lines: 50% - functions: 50% - branches: 50% - statements: 50% Add tests to meet the threshold: - testing: Add errors.test.ts and extend generators.test.ts for branch coverage - ui-react: Add tests for 13 additional hooks (useFileUpload, useModal, etc.) - ui-react-builder: Add tests for mcpReactUI plugin hooks and standalone mode
Run tests with --coverage flag so the 50% threshold blocks CI.
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds extensive unit tests across packages for errors, generators, React hooks, and a Vite plugin; lowers coverage thresholds; and updates the PR CI to run tests with coverage reporting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Code Review - PR #84: Add 50% Code Coverage Quality GateOverviewThis PR adds code coverage thresholds (50% across lines, functions, branches, statements) and introduces comprehensive tests across multiple packages to meet those thresholds. Overall, this is a solid improvement to the project's quality infrastructure. ✅ Strengths
🔍 Code Quality Observationspackages/testing/tests/unit/errors.test.ts ✅
packages/testing/tests/unit/eval/generators.test.ts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (1)
1-209: CLAUDE.md and vitest configuration are out of sync on coverage thresholds.CLAUDE.md states "80% test coverage - CI fails below this," but
vitest.config.tscurrently sets thresholds to 50% across all metrics (lines, functions, branches, statements). This mismatch should be resolved by either updating CLAUDE.md to document the 50% policy or restoring the 80% thresholds in the configuration. The test file itself is well-structured and follows best practices.
🤖 Fix all issues with AI agents
In @vitest.config.ts:
- Around line 23-26: The coverage thresholds in vitest.config.ts (the keys
lines, functions, branches, statements) are set to 50% which conflicts with
CLAUDE.md's 80% policy; update those four threshold values in the exported
Vitest config from 50 to 80 so the CI-enforced coverage matches the documented
CLAUDE.md policy, or if you intend to lower the policy instead, update CLAUDE.md
to state 50% and add a brief rationale — ensure the change references the same
symbols (lines, functions, branches, statements) so they remain consistent.
🧹 Nitpick comments (3)
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (1)
182-207: Logging tests don't verify actual logging behavior.Both tests only verify that the plugin can be instantiated with different logger options, but they don't validate that the logger options have the intended effect:
Silent logger test (lines 183-192): The
consoleSpyis created but never asserted. The test should verify that no logging occurs whenlogger: falseis set.Custom logger test (lines 194-207): The
customLoggermocks are never asserted. The test should verify that the custom logger methods are called when expected.These tests provide minimal value in their current form—they only confirm that plugin instantiation doesn't throw.
♻️ Suggested improvements to verify logging behavior
For the silent logger test, you would need to trigger a hook that logs (like
buildStart), which would require mocking file system operations and making this an integration test. However, for a unit test, you can at least verify the logger configuration:it("should use silent logger when logger is false", () => { - const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const plugin = mcpReactUI({ serverEntry: "./src/index.ts", logger: false, }); - // Plugin should be created without logging expect(plugin).toBeDefined(); + expect(plugin.name).toBe("mcp-react-ui"); - consoleSpy.mockRestore(); });For the custom logger test, similarly verify the plugin accepts it:
it("should accept custom logger", () => { const customLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), }; const plugin = mcpReactUI({ serverEntry: "./src/index.ts", logger: customLogger, }); expect(plugin).toBeDefined(); + expect(plugin.name).toBe("mcp-react-ui"); + // Note: Actual logger usage would require integration tests with buildStart });Alternatively, consider adding integration tests that mock the file system and trigger
buildStartto verify actual logging behavior.packages/ui-react/tests/unit/hooks.test.tsx (1)
309-515: Make “not supported” scenarios explicit (avoid relying on MockAdapter omissions); consider removing repetitive dynamic imports.
Right now, tests likeuseFileUpload/useFileDownload/useModal“not supported” pass only if the mock client truly lacks those methods; if MockAdapter grows support later, these tests will start failing for the wrong reason.Proposed direction (example for forcing “not supported”)
- const { client } = await createMockClient(); + const { client } = await createMockClient(); + // Force an unsupported client regardless of MockAdapter changes + (client as unknown as { uploadFile?: undefined }).uploadFile = undefined;packages/testing/tests/unit/eval/generators.test.ts (1)
8-237: Useit.skipIfconsistently for fast-check-dependent tests (avoid conditional no-op tests).
The first test effectively becomes a no-op when fast-check isn’t available; the file already usesit.skipIf/describe.skipIf, so aligning improves signal.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/pr-check.ymlpackages/testing/tests/unit/errors.test.tspackages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/ui-react/tests/unit/hooks.test.tsxvitest.config.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Zod 4 (not Zod 3) - be aware of breaking changes from v3, check migration if something looks wrong
Use Express 5 (not Express 4) - async error handling works differently
UsedefineToolanddefineUI(ordefineReactUIfor React components) to provide type inference
Middleware must alwaysawait next()or the chain breaks - follow Koa-style middleware pattern
Files:
packages/ui-react/tests/unit/hooks.test.tsxpackages/testing/tests/unit/errors.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/testing/tests/unit/eval/generators.test.tsvitest.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Noanytypes allowed - useunknownwith type narrowing. Implicit any causes production bugs
Always useexport typefor type-only exports to prevent runtime imports of types
Files:
packages/ui-react/tests/unit/hooks.test.tsxpackages/testing/tests/unit/errors.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/testing/tests/unit/eval/generators.test.tsvitest.config.ts
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Tests should mirror source structure in
tests/directory
Files:
packages/ui-react/tests/unit/hooks.test.tsxpackages/testing/tests/unit/errors.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/testing/tests/unit/eval/generators.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use properly typed mocks from vitest instead of relying on mocks with
anytypes in test files
Files:
packages/ui-react/tests/unit/hooks.test.tsxpackages/testing/tests/unit/errors.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/testing/tests/unit/eval/generators.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Maintain 80% test coverage - CI fails below this threshold
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use properly typed mocks from vitest instead of relying on mocks with `any` types in test files
📚 Learning: 2026-01-10T23:36:21.868Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use properly typed mocks from vitest instead of relying on mocks with `any` types in test files
Applied to files:
packages/ui-react/tests/unit/hooks.test.tsxpackages/testing/tests/unit/errors.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.tspackages/testing/tests/unit/eval/generators.test.tsvitest.config.ts
📚 Learning: 2026-01-10T23:36:21.868Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Run `pnpm build && pnpm test && pnpm lint && pnpm typecheck` before finishing any task - all four commands must pass with no exceptions as broken builds block the whole team
Applied to files:
.github/workflows/pr-check.yml
📚 Learning: 2026-01-10T23:36:21.868Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Maintain 80% test coverage - CI fails below this threshold
Applied to files:
vitest.config.ts
🧬 Code graph analysis (3)
packages/ui-react/tests/unit/hooks.test.tsx (1)
packages/ui-react/src/hooks.ts (9)
useOnToolInputPartial(475-487)useHostCapabilities(515-532)useHostVersion(556-571)useFileUpload(680-714)useFileDownload(744-782)useModal(954-984)useView(879-882)useIntrinsicHeight(823-854)useDebugLogger(1012-1024)
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (1)
packages/ui-react-builder/src/vite-plugin.ts (6)
isPathWithinRoot(232-241)mcpReactUI(455-536)resolveId(493-498)load(500-505)config(508-520)generateBundle(523-534)
packages/testing/tests/unit/eval/generators.test.ts (2)
packages/testing/src/eval/property/index.ts (4)
generators(8-8)isLazyArbitrary(11-11)ensureFastCheckLoaded(9-9)resolveArbitrary(10-10)packages/testing/src/eval/property/generators.ts (4)
generators(98-211)isLazyArbitrary(57-64)ensureFastCheckLoaded(37-39)resolveArbitrary(70-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-and-lint
- GitHub Check: claude-review
🔇 Additional comments (5)
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (3)
46-52: LGTM! Edge case coverage for path validation.These tests correctly cover the boundary conditions where the candidate path equals the root and where parent directory traversal is attempted.
56-69: LGTM! Basic plugin structure validated.The tests correctly verify the plugin name and the presence of required hooks.
However, note that the most important hooks (
buildStartfor discovering/building UIs andconfigResolvedfor config capture) are never tested for their actual behavior. Consider adding integration tests that verify:
buildStartdiscovers React UIs correctlyconfigResolvedcaptures the Vite configuration- UI discovery and building functionality
These tests would significantly increase confidence in the plugin's core functionality.
71-180: LGTM! Comprehensive standalone mode coverage.The test suite thoroughly validates the standalone mode behavior across all plugin hooks:
- Virtual entry resolution and loading
- Rollup configuration injection
- Bundle manipulation
The tests correctly verify that standalone mode enables special handling while non-standalone mode defers to standard Vite behavior.
packages/testing/tests/unit/errors.test.ts (1)
1-201: Good coverage of error contracts andtoJSON()payloads.
These tests exercise the public fields and serialization paths well..github/workflows/pr-check.yml (1)
45-46: The workflow command will work as intended.All workspace packages exclusively use Vitest (not mixed test runners), which supports the
--coverageflag. Nx'srun-manywith the--separator properly forwards arguments to underlying script execution, sopnpm test -- --coveragewill correctly pass the flag to each package'svitest runcommand. The root vitest.config.ts is already configured with coverage settings (provider "v8", reporters, and thresholds), and nx.json declares coverage outputs, confirming this is the expected workflow.Likely an incorrect or invalid review comment.
| lines: 50, | ||
| functions: 50, | ||
| branches: 50, | ||
| statements: 50, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check CLAUDE.md for current coverage policy
cat -n CLAUDE.md | head -50Repository: AndurilCode/mcp-apps-kit
Length of output: 2326
🏁 Script executed:
# Read vitest.config.ts to see the actual coverage thresholds
cat -n vitest.config.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 1025
🏁 Script executed:
# Check if there are separate vitest configs for ui-react or other packages
find . -name "vitest.config.ts" -o -name "vitest.config.js" | head -20Repository: AndurilCode/mcp-apps-kit
Length of output: 313
🏁 Script executed:
# Look for test configuration in package.json files, especially for ui-react
find packages/ui-react -name "package.json" -exec cat -n {} \;Repository: AndurilCode/mcp-apps-kit
Length of output: 1919
🏁 Script executed:
# Check packages/ui-react/vitest.config.ts
cat -n packages/ui-react/vitest.config.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 550
🏁 Script executed:
# Check a few other per-package configs to see the pattern
cat -n packages/core/vitest.config.tsRepository: AndurilCode/mcp-apps-kit
Length of output: 388
🏁 Script executed:
# Look for CI workflow files to see how tests are executed
find . -name "*.yml" -o -name "*.yaml" | grep -E "(workflow|ci|github|gitlab)" | head -10Repository: AndurilCode/mcp-apps-kit
Length of output: 282
🏁 Script executed:
# Check for pnpm-workspace.yaml or package.json workspaces
cat -n pnpm-workspace.yaml 2>/dev/null || cat -n package.json | head -30Repository: AndurilCode/mcp-apps-kit
Length of output: 185
🏁 Script executed:
# Check the PR check workflow
cat -n .github/workflows/pr-check.ymlRepository: AndurilCode/mcp-apps-kit
Length of output: 1242
🏁 Script executed:
# Also check the main CI workflow
cat -n .github/workflows/claude.ymlRepository: AndurilCode/mcp-apps-kit
Length of output: 2508
Coverage thresholds (50%) conflict with CLAUDE.md policy (80%)—update one to match the other.
CLAUDE.md explicitly states "80% test coverage - CI fails below this," but vitest.config.ts sets all thresholds to 50%. When pnpm test -- --coverage runs in CI, it enforces 50%, not 80%. Either update the thresholds to 80% to match the stated policy, or update CLAUDE.md to reflect 50% as the new minimum (and document why it changed).
🤖 Prompt for AI Agents
In @vitest.config.ts around lines 23 - 26, The coverage thresholds in
vitest.config.ts (the keys lines, functions, branches, statements) are set to
50% which conflicts with CLAUDE.md's 80% policy; update those four threshold
values in the exported Vitest config from 50 to 80 so the CI-enforced coverage
matches the documented CLAUDE.md policy, or if you intend to lower the policy
instead, update CLAUDE.md to state 50% and add a brief rationale — ensure the
change references the same symbols (lines, functions, branches, statements) so
they remain consistent.
Code Review: Add 50% Coverage Quality GateSummaryThis PR adds a code coverage quality gate with 50% thresholds across all metrics. The implementation includes comprehensive test coverage for previously untested code in the testing, ui-react, and ui-react-builder packages. Strengths1. Well-Structured Test Coverage
2. Good Testing Practices
3. Alignment with Project Standards
Issues and Concerns1. Coverage Threshold Reduction (Critical) The PR reduces the coverage threshold from 80% to 50%. This is a significant reduction that may indicate the codebase previously did not meet 80% coverage. Recommendation: Consider whether 50% is the right long-term target. Many mature projects aim for 70-80% coverage. If 50% is temporary, add a comment in CLAUDE.md indicating the goal to increase it later. 2. Test Quality in generators.test.ts The new tests for generator options only verify that generators are defined but do not validate behavior. These tests do not verify that the options are actually applied correctly. Recommendation: Consider adding tests that actually generate values and verify the constraints. 3. Missing Edge Cases in hooks.test.tsx The file upload/download tests only test the not supported path. The success paths are not tested. Recommendation: Add tests that mock successful file operations to ensure the state management works correctly. 4. Vite Plugin Test Coverage Gaps The plugin tests cover the exposed hooks but do not test the core build logic including discoverReactUIs, buildDiscoveredUIs, and AST parsing integration. Recommendation: Add integration tests that mock a simple server entry with defineReactUI calls, verify components are discovered correctly, and test that HTML output is generated. Security ConsiderationsNo security issues identified. The tests do not introduce any vulnerabilities. Test Coverage AnalysisCoverage is being added for:
Overall: The PR achieves the 50% threshold but leaves significant gaps in critical functionality. Recommendations
VerdictApprove with recommendations This PR successfully adds the infrastructure for code coverage enforcement and addresses significant testing gaps. While there are areas for improvement, the overall quality is good and aligns with the project testing conventions. The 50% threshold is a reasonable starting point, but I encourage the team to plan for incremental increases over time. |
- Add value verification tests for generators using fc.sample() to verify actual generated values meet constraints (strings within length bounds, integers within range, arrays with correct shape, etc.) - Add default options and path handling tests for vite-plugin - Fix incorrect assumption about standalone default value (is false, not true)
Code Review: Add 50% Code Coverage Quality GateOverviewThis PR successfully implements a 50% code coverage threshold in CI and adds comprehensive test coverage across multiple packages. Overall this is a solid quality improvement that will help maintain code standards going forward. ✅ Strengths1. Comprehensive Test Coverage
2. Test Quality
3. CI Integration
🔍 Code Quality ObservationsMinor Issues1. Test Duplication in
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/testing/tests/unit/eval/generators.test.ts (1)
42-140: Tests provide coverage but are shallow.These tests verify that generators can be created without throwing, which is useful for coverage, but they don't validate the generators' behavior or the constraints they encode. Consider this acceptable for meeting coverage thresholds, but note that deeper behavioral testing would strengthen the test suite.
The validation tests at lines 239-321 provide more thorough verification, which is good.
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (4)
182-208: Consider verifying logger behavior, not just plugin creation.The current tests only confirm the plugin is created with different logger configurations but don't verify that:
- The silent logger actually suppresses output
- The custom logger is invoked during plugin operations
While testing actual logger invocation would require mocking buildStart dependencies, consider at least verifying that the console spy isn't called for the silent logger case.
💡 Example enhancement
it("should use silent logger when logger is false", () => { const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const plugin = mcpReactUI({ serverEntry: "./src/index.ts", logger: false, }); - // Plugin should be created without logging expect(plugin).toBeDefined(); + // Verify console.log wasn't called during plugin creation + expect(consoleSpy).not.toHaveBeenCalled(); consoleSpy.mockRestore(); });
219-223: Test doesn't meaningfully verify default outDir behavior.The test only confirms plugin creation succeeds, which doesn't verify that outDir has a default value or that the default is used correctly. The comment on line 222 acknowledges this limitation.
Consider either:
- Removing this test as it doesn't add value
- Enhancing it to verify the default value through plugin behavior (though this would require more complex mocking)
226-245: Path handling tests are superficial.The tests only verify that plugin creation doesn't throw with various path formats but don't validate that the paths are actually processed or used correctly by the plugin. While these serve as basic smoke tests, consider whether they provide sufficient value for the coverage they add.
61-69: Consider adding tests for buildStart hook.While the presence of
buildStartis verified, its behavior—which includes discovering React UIs and building them—is not tested. This is the plugin's core functionality. Consider adding tests that mock the file system and verify thatbuildStartproperly discovers and processes UI definitions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Zod 4 (not Zod 3) - be aware of breaking changes from v3, check migration if something looks wrong
Use Express 5 (not Express 4) - async error handling works differently
UsedefineToolanddefineUI(ordefineReactUIfor React components) to provide type inference
Middleware must alwaysawait next()or the chain breaks - follow Koa-style middleware pattern
Files:
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Noanytypes allowed - useunknownwith type narrowing. Implicit any causes production bugs
Always useexport typefor type-only exports to prevent runtime imports of types
Files:
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Tests should mirror source structure in
tests/directory
Files:
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use properly typed mocks from vitest instead of relying on mocks with
anytypes in test files
Files:
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Maintain 80% test coverage - CI fails below this threshold
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use properly typed mocks from vitest instead of relying on mocks with `any` types in test files
📚 Learning: 2026-01-10T23:36:21.868Z
Learnt from: CR
Repo: AndurilCode/mcp-apps-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T23:36:21.868Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use properly typed mocks from vitest instead of relying on mocks with `any` types in test files
Applied to files:
packages/testing/tests/unit/eval/generators.test.tspackages/ui-react-builder/tests/unit/vite-plugin.test.ts
🧬 Code graph analysis (2)
packages/testing/tests/unit/eval/generators.test.ts (2)
packages/testing/src/eval/property/generators.ts (4)
generators(98-211)isLazyArbitrary(57-64)ensureFastCheckLoaded(37-39)resolveArbitrary(70-78)packages/testing/src/eval/property/index.ts (4)
generators(8-8)isLazyArbitrary(11-11)ensureFastCheckLoaded(9-9)resolveArbitrary(10-10)
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (1)
packages/ui-react-builder/src/vite-plugin.ts (6)
isPathWithinRoot(232-241)mcpReactUI(455-536)resolveId(493-498)load(500-505)config(508-520)generateBundle(523-534)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-and-lint
- GitHub Check: claude-review
🔇 Additional comments (9)
packages/testing/tests/unit/eval/generators.test.ts (4)
8-24: LGTM! Clean import structure and appropriate optional dependency handling.The dynamic detection of fast-check availability using
requirewith proper ESLint suppression is a reasonable approach for handling optional dependencies in tests.
143-167: Excellent type guard testing with comprehensive edge case coverage.These tests thoroughly verify the
isLazyArbitraryfunction, including the important edge case where an object has__lazyArbitrary: false.
239-321: Excellent property-based testing validation!These tests properly validate generated values using
fc.sample()to verify that generators respect their constraints. The tests check both individual value properties and statistical distributions (e.g., ensuring both true and false appear in boolean samples), which is the right approach for property-based testing.
1-322: Verify the coverage threshold reduction from 80% to 50%.Based on learnings, the previous standard was to "Maintain 80% test coverage - CI fails below this threshold." This PR sets the threshold at 50%, which is a significant reduction in the quality gate. Please confirm this reduction is intentional and aligns with updated project standards.
Based on learnings, the documented standard is 80% coverage.
packages/ui-react-builder/tests/unit/vite-plugin.test.ts (5)
1-3: LGTM! Imports follow best practices.The imports correctly use
vifrom vitest for mocking and thetypekeyword for type-only imports, adhering to the coding guidelines.
46-52: LGTM! Good edge case coverage.The new tests properly validate edge cases for path validation, including same-path scenarios and parent directory traversal attempts.
56-69: LGTM! Plugin structure tests are appropriate.The tests correctly verify the plugin's name and presence of required hooks.
71-180: LGTM! Comprehensive standalone mode coverage.The tests thoroughly validate the plugin's behavior in both standalone and non-standalone modes, correctly testing the virtual entry resolution, loading, configuration, and bundle generation hooks.
1-246: > Likely an incorrect or invalid review comment.
2 participants
Enable code coverage thresholds in vitest.config.ts that block CI:
Add tests to meet the threshold: