Skip to content

Conversation

@ar-saeedi
Copy link

@ar-saeedi ar-saeedi commented Oct 31, 2025

What I did

Added comprehensive test coverage for the utility functions in code/core/src/common/utils/utils.ts which was recently added but lacked test coverage.

This PR introduces 13 well-structured test cases covering all functionality and edge cases for the groupBy and invariant utility functions.

Changes Made:

  • ✅ Created code/core/src/common/utils/utils.test.ts with comprehensive test coverage
  • ✅ Added 8 test cases for groupBy function
  • ✅ Added 11 test cases for invariant function
  • ✅ Covered edge cases, performance optimizations, and TypeScript type assertions

Checklist for Contributors

Testing

  • unit tests - 13 comprehensive test cases added

Documentation

  • Tests serve as documentation for the utility functions

Summary by CodeRabbit

Release Notes

  • Tests
    • Enhanced test coverage for core utility functions, including comprehensive validation of grouping operations and assertion behavior across various scenarios and edge cases.

Note: This release contains internal quality improvements with no user-facing changes.

- Add 13 test cases for groupBy and invariant functions
- Cover edge cases, type narrowing, and lazy evaluation
- Achieve 100% coverage for utils.ts utility functions

Tests include:
- groupBy: various key types, empty arrays, index handling
- invariant: lazy messages, type assertions, edge cases
@yannbf yannbf self-assigned this Nov 3, 2025
@yannbf yannbf added build Internal-facing build tooling & test updates ci:normal labels Nov 3, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

📝 Walkthrough

Walkthrough

Adds comprehensive unit tests for groupBy and invariant utility functions, covering multiple scenarios including edge cases, different key types, message evaluation strategies, and type narrowing behavior.

Changes

Cohort / File(s) Summary
Test Coverage for Utility Functions
code/core/src/common/utils/utils.test.ts
Adds tests for groupBy utility (grouping by key, empty arrays, single items, index-aware selectors, numeric and symbol keys, same-key scenarios) and invariant utility (truthy/falsy conditions, default messages, lazy/eager evaluation, expensive computation avoidance, complex conditions, type narrowing)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10-15 minutes

  • Test additions follow consistent patterns across multiple utility scenarios
  • No production logic changes or complex control flow to evaluate
  • Edge cases and assertions are clearly defined and straightforward to verify
✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
code/core/src/common/utils/utils.test.ts (1)

45-55: Consider verifying the grouping result.

While this test correctly verifies that indices are passed to the key selector, it doesn't assert the actual grouping result. Consider also verifying that the returned object matches expectations for completeness.

Apply this diff to also verify the grouping result:

     it('should pass index to key selector', () => {
       const items = ['a', 'b', 'c', 'd'];
       const indices: number[] = [];
 
-      groupBy(items, (_item, index) => {
+      const result = groupBy(items, (_item, index) => {
         indices.push(index);
         return index % 2 === 0 ? 'even' : 'odd';
       });
 
       expect(indices).toEqual([0, 1, 2, 3]);
+      expect(result).toEqual({
+        even: ['a', 'c'],
+        odd: ['b', 'd'],
+      });
     });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d496297 and 8f367da.

📒 Files selected for processing (1)
  • code/core/src/common/utils/utils.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
code/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx

Files:

  • code/core/src/common/utils/utils.test.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together

Files:

  • code/core/src/common/utils/utils.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/core/src/common/utils/utils.test.ts
**/*.@(test|spec).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests using vi.mock() (e.g., filesystem, loggers)

Files:

  • code/core/src/common/utils/utils.test.ts
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/core/src/common/utils/utils.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/core/src/common/utils/utils.test.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/common/utils/utils.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together

Applied to files:

  • code/core/src/common/utils/utils.test.ts
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns

Applied to files:

  • code/core/src/common/utils/utils.test.ts
⏰ 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: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/common/utils/utils.test.ts (3)

1-3: LGTM! Clean imports for testing pure utilities.

The imports are minimal and appropriate. No mocks are needed since groupBy and invariant are pure utility functions without external dependencies.


5-106: Excellent test coverage for groupBy!

The test suite is comprehensive, covering basic functionality, edge cases (empty arrays, single items), different key types (string, numeric, symbol), and index-aware key selectors. The tests are well-structured and effectively validate the function's behavior.


108-224: Excellent test coverage for invariant!

The test suite thoroughly validates the invariant function, including:

  • Truthy/falsy condition handling
  • Default error messages
  • Lazy message evaluation optimization (critical for performance)
  • Type narrowing behavior
  • Complex real-world scenarios

The lazy evaluation tests (lines 159-182) are particularly valuable as they verify the performance optimization benefit.

@nx-cloud
Copy link

nx-cloud bot commented Nov 3, 2025

View your CI Pipeline Execution ↗ for commit 8f367da

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 46s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-03 14:48:52 UTC

@github-actions github-actions bot added the Stale label Nov 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants