Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Dec 28, 2025

Summary by CodeRabbit

  • Tests

    • Added comprehensive test suites for common utility functions, covering edge cases and various input types.
  • Chores

    • Added test script and Vitest configuration to enable automated testing.
    • Added testing dependency to development environment.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 28, 2025 06:03
@coderabbitai
Copy link

coderabbitai bot commented Dec 28, 2025

📝 Walkthrough

Walkthrough

This PR establishes comprehensive test coverage for the @zenstackhq/common-helpers package by adding 10 new test suites covering utility functions (clone, case conversion, enumerable, etc.), along with Vitest configuration and package.json updates to enable test execution.

Changes

Cohort / File(s) Summary
Package Configuration
packages/common-helpers/package.json, packages/common-helpers/vitest.config.ts
Added test script (test: vitest run), registered @zenstackhq/vitest-config dependency, and configured Vitest with merged base configuration.
Test Suites
packages/common-helpers/test/case-conversion.test.ts, test/clone.test.ts, test/enumerable.test.ts, test/is-plain-object.test.ts, test/param-case.test.ts, test/safe-json-stringify.test.ts, test/sleep.test.ts, test/tiny-invariant.test.ts, test/zip.test.ts
Added comprehensive test coverage for 9 utility functions, validating edge cases, type handling, and expected behaviors across primitives, arrays, objects, nested structures, and special types (Date, RegExp, BigInt).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • test: add edge-runtime e2e #458: Adds and updates Vitest configuration files across packages, including the shared @zenstackhq/vitest-config base configuration that this PR depends on.
  • merge dev to main (v3.0.0-alpha.18) #142: Modifies packages/common-helpers/package.json to add version updates and dependencies, overlapping with this PR's package.json changes.
  • merge dev to main (v3.0.0-beta.2) #234: Reorganizes root and workspace Vitest configurations while updating the packages/vitest-config package, related to the test infrastructure setup in this PR.

Poem

🐰 Hippity hop, tests take the stage,
Nine helpers tested, page by page,
Clone and case, zip and sleep,
Coverage growing, promises to keep!
With Vitest running, bugs beware,
Quality code beyond compare! 🧪✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding comprehensive test suites for the common-helpers package, which is the primary focus of all modifications in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/common-helpers-tests

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f13886b and daa7649.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • packages/common-helpers/package.json
  • packages/common-helpers/test/case-conversion.test.ts
  • packages/common-helpers/test/clone.test.ts
  • packages/common-helpers/test/enumerable.test.ts
  • packages/common-helpers/test/is-plain-object.test.ts
  • packages/common-helpers/test/param-case.test.ts
  • packages/common-helpers/test/safe-json-stringify.test.ts
  • packages/common-helpers/test/sleep.test.ts
  • packages/common-helpers/test/tiny-invariant.test.ts
  • packages/common-helpers/test/zip.test.ts
  • packages/common-helpers/vitest.config.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests

Applied to files:

  • packages/common-helpers/test/clone.test.ts
  • packages/common-helpers/test/is-plain-object.test.ts
  • packages/common-helpers/test/param-case.test.ts
  • packages/common-helpers/test/enumerable.test.ts
  • packages/common-helpers/test/zip.test.ts
  • packages/common-helpers/test/safe-json-stringify.test.ts
  • packages/common-helpers/test/tiny-invariant.test.ts
  • packages/common-helpers/test/case-conversion.test.ts
  • packages/common-helpers/test/sleep.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects

Applied to files:

  • packages/common-helpers/test/clone.test.ts
  • packages/common-helpers/test/is-plain-object.test.ts
  • packages/common-helpers/test/param-case.test.ts
  • packages/common-helpers/test/enumerable.test.ts
  • packages/common-helpers/test/zip.test.ts
  • packages/common-helpers/test/safe-json-stringify.test.ts
  • packages/common-helpers/test/case-conversion.test.ts
  • packages/common-helpers/test/sleep.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command

Applied to files:

  • packages/common-helpers/test/clone.test.ts
  • packages/common-helpers/test/zip.test.ts
  • packages/common-helpers/package.json
  • packages/common-helpers/test/case-conversion.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests

Applied to files:

  • packages/common-helpers/test/is-plain-object.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use Turbo for build orchestration and run `pnpm build`, `pnpm watch`, `pnpm lint`, and `pnpm test` for development tasks

Applied to files:

  • packages/common-helpers/package.json
🧬 Code graph analysis (9)
packages/common-helpers/test/clone.test.ts (1)
packages/common-helpers/src/clone.ts (1)
  • clone (6-24)
packages/common-helpers/test/is-plain-object.test.ts (1)
packages/common-helpers/src/is-plain-object.ts (1)
  • isPlainObject (5-23)
packages/common-helpers/test/param-case.test.ts (1)
packages/common-helpers/src/param-case.ts (1)
  • paramCase (5-22)
packages/common-helpers/test/enumerable.test.ts (1)
packages/common-helpers/src/enumerable.ts (1)
  • enumerate (9-17)
packages/common-helpers/test/zip.test.ts (1)
packages/common-helpers/src/zip.ts (1)
  • zip (4-11)
packages/common-helpers/test/safe-json-stringify.test.ts (1)
packages/common-helpers/src/safe-json-stringify.ts (1)
  • safeJSONStringify (4-12)
packages/common-helpers/test/tiny-invariant.test.ts (1)
packages/common-helpers/src/tiny-invariant.ts (1)
  • invariant (4-14)
packages/common-helpers/test/case-conversion.test.ts (2)
packages/common-helpers/src/lower-case-first.ts (1)
  • lowerCaseFirst (1-3)
packages/common-helpers/src/upper-case-first.ts (1)
  • upperCaseFirst (1-3)
packages/common-helpers/test/sleep.test.ts (1)
packages/common-helpers/src/sleep.ts (1)
  • sleep (1-5)
⏰ 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). (5)
  • GitHub Check: Agent
  • GitHub Check: CodeQL analysis (javascript-typescript)
  • GitHub Check: claude-review
  • GitHub Check: build-test (22.x, postgresql)
  • GitHub Check: build-test (22.x, sqlite)
🔇 Additional comments (13)
packages/common-helpers/package.json (1)

10-10: LGTM! Test infrastructure properly configured.

The test script and Vitest configuration dependency are correctly added to enable test execution for the package.

Also applies to: 33-34

packages/common-helpers/vitest.config.ts (1)

1-4: LGTM! Standard Vitest configuration.

The configuration appropriately extends the base config without unnecessary overrides.

packages/common-helpers/test/zip.test.ts (1)

1-96: LGTM! Comprehensive test coverage for the zip utility.

The test suite thoroughly covers edge cases including empty arrays, mismatched lengths, null/undefined values, objects, nested arrays, and readonly arrays. All test cases appear correct and well-structured.

packages/common-helpers/test/sleep.test.ts (1)

1-70: LGTM! Well-structured sleep utility tests.

The test suite provides excellent coverage including timing accuracy, promise behavior, concurrency, and fake timer scenarios. The timing tolerances (5ms for 100ms sleep, etc.) are reasonable and should be stable in most CI environments.

packages/common-helpers/test/case-conversion.test.ts (2)

5-29: LGTM! Thorough test coverage for lowerCaseFirst.

The test cases appropriately cover basic transformations, edge cases (empty string), and special scenarios (numbers, special characters).


31-55: LGTM! Comprehensive test coverage for upperCaseFirst.

The test suite mirrors the lowerCaseFirst tests with appropriate coverage of transformations and edge cases.

packages/common-helpers/test/is-plain-object.test.ts (1)

1-50: LGTM! Excellent test coverage for isPlainObject.

The test suite comprehensively validates plain object detection across various scenarios including primitives, arrays, class instances, functions, and edge cases like Object.create(null) and custom prototypes.

packages/common-helpers/test/param-case.test.ts (1)

1-71: LGTM! Comprehensive paramCase test coverage.

The test suite thoroughly validates case conversion including real-world scenarios (XMLHttpRequest, HTTPSConnection), edge cases (empty string, single character), and proper handling of numbers, spaces, and special characters.

packages/common-helpers/test/safe-json-stringify.test.ts (1)

1-81: LGTM! Thorough test coverage for safeJSONStringify.

The test suite excellently validates BigInt serialization across various data structures including nested objects, arrays, and mixed types. Edge cases like zero, negative, and very large BigInt values are appropriately covered.

packages/common-helpers/test/tiny-invariant.test.ts (1)

1-70: Excellent test coverage for the invariant utility!

The test suite is comprehensive and well-structured, covering:

  • All truthy and falsy value types
  • Type guard functionality (nice validation of TypeScript's assertion behavior)
  • Custom error messages
  • Complex conditions and real-world usage patterns (array checks, object property existence)
  • Edge cases like NaN

The tests align perfectly with the implementation and will provide strong regression protection.

packages/common-helpers/test/enumerable.test.ts (1)

1-55: Well-crafted test suite with excellent edge case coverage!

The tests thoroughly validate the enumerate function's behavior:

  • Proper handling of null/undefined → empty array
  • Reference equality checks (lines 15, 28, 39) confirm arrays are returned as-is without unnecessary copying
  • Important edge cases for falsy-but-not-null values (0, empty string) that should still be wrapped
  • Complete coverage of different value types (primitives, objects, functions, nested arrays)

The test structure is clear and the assertions are precise.

packages/common-helpers/test/clone.test.ts (2)

1-149: Comprehensive clone test suite with excellent structure!

The tests provide thorough coverage across:

  • All primitive types
  • Shallow and deep cloning of arrays and objects
  • Reference independence validation (using .not.toBe() checks)
  • Non-plain objects correctly returned as-is (Date, RegExp, class instances, functions)
  • Complex mixed structures

The test organization with nested describe blocks makes the suite easy to navigate and maintain.


83-89: isPlainObject correctly handles null-prototype objects.

The isPlainObject utility properly identifies Object.create(null) as a plain object (returns true when constructor is undefined), and the clone function successfully clones such objects. The test expectations are met.


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 and usage tips.

@claude
Copy link

claude bot commented Dec 28, 2025

Claude finished @ymc9's task —— View job


PR Review: Adding Tests for common-helpers Package

Review Tasks

  • Analyze the common-helpers source code structure
  • Review test file implementations for quality and correctness
  • Check test coverage completeness
  • Assess vitest configuration
  • Identify potential issues and improvements
  • Provide comprehensive feedback

Overall Assessment

This is a well-structured test suite that adds comprehensive coverage to the common-helpers package. The tests are thorough, well-organized, and follow good testing practices.

✅ Strengths

Comprehensive Test Coverage

  • All utility functions have corresponding test files
  • Edge cases are well covered (empty inputs, null/undefined, type boundaries)
  • Tests for both expected behavior and error conditions

Test Quality & Organization

  • Clear, descriptive test names that explain expected behavior
  • Good use of describe blocks for logical grouping
  • Comprehensive edge case testing (empty strings, null, undefined, etc.)
  • Tests validate both functionality and type safety

Best Practices

  • Proper use of vitest test framework
  • Good assertion patterns using expect()
  • Tests are isolated and don't depend on each other
  • Configuration extends workspace base config properly

📝 Detailed Review

clone.test.ts - Excellent packages/common-helpers/test/clone.test.ts:1

  • Comprehensive testing of all object types (primitives, arrays, plain objects, non-plain objects)
  • Good test coverage for deep cloning behavior
  • Tests verify reference inequality for cloned objects
  • Edge cases well covered (empty objects/arrays, null prototype objects)

enumerable.test.ts - Good packages/common-helpers/test/enumerable.test.ts:1

  • Tests cover all expected inputs and edge cases
  • Properly tests the array-returning behavior for null/undefined
  • Good coverage of scalar wrapping functionality

param-case.test.ts - Excellent packages/common-helpers/test/param-case.test.ts:1

  • Comprehensive test coverage for various string formats
  • Tests all major naming conventions (camelCase, PascalCase, snake_case, etc.)
  • Edge cases like empty strings, special characters, and consecutive uppercase letters
  • Good real-world examples (XMLHttpRequest, HTTPSConnection)

safe-json-stringify.test.ts - Excellent packages/common-helpers/test/safe-json-stringify.test.ts:1

  • Thorough testing of BigInt handling (the main feature)
  • Tests nested objects and arrays with BigInt values
  • Edge cases like zero, negative, and very large BigInt values
  • Validates JSON output format correctly

is-plain-object.test.ts - Good packages/common-helpers/test/is-plain-object.test.ts:1

  • Tests all major object types (plain objects, arrays, class instances, etc.)
  • Good coverage of prototype chain scenarios
  • Tests null prototype objects correctly

sleep.test.ts - Good with Minor Issues packages/common-helpers/test/sleep.test.ts:1

  • Good test coverage including timing validation
  • Tests concurrent execution properly
  • ⚠️ Timing Sensitivity: Tests rely on actual timeouts which could be flaky in CI
  • ✅ Good: Includes fake timer tests to mitigate timing issues

tiny-invariant.test.ts - Good packages/common-helpers/test/tiny-invariant.test.ts:1

  • Tests all falsy conditions properly
  • Tests type guard functionality
  • Minor: Line 36 comment about production check could be clearer

zip.test.ts - Excellent packages/common-helpers/test/zip.test.ts:1

  • Comprehensive testing of array length differences
  • Tests different data types and complex structures
  • Good edge case coverage (empty arrays, single elements)
  • Tests readonly arrays correctly

case-conversion.test.ts - Good packages/common-helpers/test/case-conversion.test.ts:1

  • Tests both lowerCaseFirst and upperCaseFirst functions
  • Good edge case coverage (empty strings, special characters)
  • Clear, focused tests

🔧 Configuration

vitest.config.ts - Good packages/common-helpers/vitest.config.ts:1

  • Properly extends workspace base configuration
  • Clean, minimal configuration
  • Follows project patterns

package.json - Good packages/common-helpers/package.json:10

  • Test script properly configured with vitest run
  • Dependencies correctly reference workspace vitest config

🚨 Issues & Recommendations

Minor Issues

  1. Test Coverage Gap: Missing test coverage for any error scenarios in utility functions (they appear to be designed not to throw, which is good)
  2. Timing Tests: sleep.test.ts has some timing-sensitive tests that could be flaky, though the inclusion of fake timer tests helps

Performance Considerations

  • All utilities appear to be lightweight with no performance concerns
  • Clone function uses recursion but has proper safeguards for non-plain objects
  • No memory leaks or resource management issues identified

Security Assessment

  • No security vulnerabilities identified
  • Functions handle edge cases safely without throwing unexpected errors
  • No exposure of sensitive data or unsafe operations

🎯 Verdict: APPROVE

This PR significantly improves the codebase by adding comprehensive test coverage to the common-helpers package. The tests are well-written, thorough, and follow testing best practices. The minor timing sensitivity in sleep tests is acceptable given the inclusion of fake timer tests.

Test Coverage: ~100% of exported functions
Code Quality: High
Test Quality: High
Security: No concerns
Performance: No concerns

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive test coverage for the common-helpers package using Vitest. The tests cover all utility functions in the package including array operations, string transformations, object manipulation, and timing utilities.

Key Changes:

  • Added Vitest configuration and test script to the package
  • Created 9 test files covering all utility functions in the package
  • Added vitest-config as a dev dependency

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/common-helpers/package.json Added test script and vitest-config dev dependency
packages/common-helpers/vitest.config.ts Created Vitest configuration using shared base config
packages/common-helpers/test/zip.test.ts Comprehensive tests for the zip function covering edge cases like empty arrays, different lengths, and various data types
packages/common-helpers/test/tiny-invariant.test.ts Tests for the invariant assertion function including type guard behavior and various falsy conditions
packages/common-helpers/test/sleep.test.ts Tests for the sleep function including timing validation, concurrency, and fake timer support
packages/common-helpers/test/safe-json-stringify.test.ts Tests for JSON stringification with bigint support covering various data types and nested structures
packages/common-helpers/test/param-case.test.ts Tests for param-case conversion covering camelCase, PascalCase, snake_case, and special characters
packages/common-helpers/test/is-plain-object.test.ts Tests for plain object detection including edge cases with class instances, arrays, and null prototypes
packages/common-helpers/test/enumerable.test.ts Tests for the enumerate function that wraps scalars in arrays and handles null/undefined
packages/common-helpers/test/clone.test.ts Comprehensive tests for deep cloning covering primitives, arrays, objects, and non-plain objects
packages/common-helpers/test/case-conversion.test.ts Tests for lowerCaseFirst and upperCaseFirst string utilities with various input formats
pnpm-lock.yaml Updated lock file with vitest-config workspace dependency and package metadata
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ymc9 ymc9 merged commit 549a026 into dev Dec 28, 2025
11 checks passed
@ymc9 ymc9 deleted the chore/common-helpers-tests branch December 28, 2025 06:26
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