test(shared): add unit tests for deep-merge utility #745
Conversation
|
All contributors have signed the CLA. Thank you! ✅ |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 4/5
- Test coverage in
src/shared/deep-merge.test.tsis weak: the MAX_DEPTH override scenario only checks that the nested structure exists and never asserts the value remains "override", so regressions in deep-merge behavior could slip through. - Pay close attention to
src/shared/deep-merge.test.ts- strengthen the MAX_DEPTH override assertion to ensure overrides aren’t merged back with the base.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/shared/deep-merge.test.ts">
<violation number="1" location="src/shared/deep-merge.test.ts:328">
P2: Weak assertion: test claims to verify MAX_DEPTH override behavior but only checks structure existence. Should assert that the value at depth 51+ is `"override"` (not merged with base) to actually verify the stated behavior.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
I have read the CLA Document and I hereby sign the CLA |
|
Hi, I've signed the CLA but the workflow is failing with: It seems the branch protection rules are preventing the CLA Assistant from updating |
We are fixing it rn! |
Add comprehensive unit tests for the deep-merge.ts utility functions: - isPlainObject: 11 test cases covering null, undefined, primitives, Array, Date, RegExp, and plain objects - deepMerge: 15 test cases covering: - Basic object merging - Deep nested object merging - Edge cases (undefined handling) - Array replacement behavior - Prototype pollution protection (DANGEROUS_KEYS) - MAX_DEPTH limit handling
LTS2
force-pushed
the
test/deep-merge-unit-tests
branch
from
2782cea to
2042a29
Compare
|
Fixed the weak assertion in depth limit test (P2 feedback). Before expect(current.nested).toBeDefined() // Only checks structure existsAfter // Use different keys to distinguish base vs override
const base = createDeepObject(55, { baseKey: "base" })
const override = createDeepObject(55, { overrideKey: "override" })
// At depth 55, only override's key should exist
expect(current.overrideKey).toBe("override")
expect(current.baseKey).toBeUndefined()This now properly verifies that at depth 51+ (beyond MAX_DEPTH of 50), override replaces base entirely instead of merging. |
|
lgtm, thanks for the pr! |
2 participants
Summary
deep-merge.tsutility functionsChanges
src/shared/deep-merge.test.tswith 26 test casesisPlainObject: 11 tests covering null, undefined, primitives, Array, Date, RegExp, and plain objectsdeepMerge: 15 tests covering:__proto__,constructor,prototypekeys are ignored)Testing
Related Issues
N/A - This is a proactive improvement to increase test coverage for the
sharedmodule.Summary by cubic
Added unit tests for the shared deep-merge utility to increase confidence and prevent regressions. Covers plain object detection, deep merge behavior, array replacement, undefined handling, prototype pollution protection, and max-depth handling.
Written for commit 2042a29. Summary will update on new commits.