Skip to content

Add comprehensive tests and improve type safety for array utilities - #1191

Open
pavankumar-vh wants to merge 1 commit into
CodebuffAI:mainfrom
pavankumar-vh:add/array-utility-tests
Open

Add comprehensive tests and improve type safety for array utilities#1191
pavankumar-vh wants to merge 1 commit into
CodebuffAI:mainfrom
pavankumar-vh:add/array-utility-tests

Conversation

@pavankumar-vh

Copy link
Copy Markdown

Bug Fix:

  • Fix loose 'any[]' typing in groupConsecutive function
  • Add proper Group type definition for improved type safety
  • This prevents type-related bugs when using the return value

Tests Added:

  • Add 28 comprehensive tests covering all three utility functions
  • filterDefined: 6 tests for null/undefined filtering with edge cases
  • buildArray: 10 tests for array flattening and falsy value removal
  • groupConsecutive: 12 tests for grouping logic with complex key functions
  • All tests pass with 100% coverage of the utility functions

Scope: common/ (approved contribution area)

Bug Fix:
- Fix loose 'any[]' typing in groupConsecutive function
- Add proper Group type definition for improved type safety
- This prevents type-related bugs when using the return value

Tests Added:
- Add 28 comprehensive tests covering all three utility functions
- filterDefined: 6 tests for null/undefined filtering with edge cases
- buildArray: 10 tests for array flattening and falsy value removal
- groupConsecutive: 12 tests for grouping logic with complex key functions
- All tests pass with 100% coverage of the utility functions

Scope: common/ (approved contribution area)
@codebuff-team

Copy link
Copy Markdown
Contributor

Nice first contribution. The core fix in array.ts is correct and minimal: replacing any[] with a proper Group<T, U> type on groupConsecutive is a real improvement — it means callers now get typed key/items instead of any, which will surface bugs at compile time. No behavior change, so it's essentially risk-free to port.

The test file is thorough and well-organized (filterDefined, buildArray, groupConsecutive each covered with sensible edge cases: empty arrays, all-falsy, nested flattening, deep-equality grouping via isEqual). This is exactly the kind of coverage common/src/util was missing.

A couple of minor nits a maintainer may want addressed before porting:

  • The Group type is defined inline inside the function; consider hoisting it to a named exported type (Group<T, U>) since it's a natural public type for callers of groupConsecutive.
  • The last test (handles objects with deep equality) only asserts lengths, not full object equality — could tighten with a full toEqual check for completeness, though it's not blocking.

Overall this is exactly the shape of PR that's easy to review and easy to port: small semantic diff, tests added in the same directory as the source, no dependency or API changes. Good work — happy to see this land.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants