Add comprehensive test coverage across all packages#26
Conversation
Agent-Logs-Url: https://github.com/Curstantine/jabascript/sessions/f30249b5-a789-44c4-b657-2220e1b44c91 Co-authored-by: Curstantine <69104500+Curstantine@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds targeted Vitest coverage for previously untested/under-tested exported helpers across the monorepo packages, focusing on edge cases without changing production source.
Changes:
@jabascript/query: adds tests forgetLiteralValue, additionalcreateSearchParamsfallthroughs, andparseSearchParamsmulti-value behavior.@jabascript/form-data: adds tests for deeper dot-notation nesting, duplicate key aggregation, and indexed arrays inside nested paths.@jabascript/core: adds tests for sequentialdebounceruns,getInitialsoption interactions, andcreateEnumdirect property access / empty input.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/query/tests/index.test.js | Expands query utility coverage including getLiteralValue and edge-case param parsing/serialization. |
| packages/form-data/tests/index.test.js | Adds coverage for deeper nesting, repeated keys, and complex indexed paths in parseFormData. |
| packages/core/tests/index.test.js | Adds coverage for sequential debounce usage and getInitials option combinations. |
| packages/core/tests/enum.test.js | Adds coverage for direct enum property access and empty enum behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("should skip null/undefined array entries by default", () => { | ||
| const searchParams = new URLSearchParams("ids[]=1&ids[]=null&ids[]=2"); | ||
| const result = parseSearchParams(searchParams); | ||
| expect(result).toEqual({ "ids[]": ["1", null, "2"] }); | ||
| }); |
There was a problem hiding this comment.
Test description says it "should skip null/undefined array entries by default", but the assertion expects the literal "null" entry to be preserved in the resulting array. This reads as contradictory and will mislead future readers; either update the test name to match the current behavior (arrays include null/undefined literals even when allowFalsy=false) or change the expectation/input to actually verify skipping behavior.
| expect(getInitials("John", undefined)).toBe("Jo"); | ||
| }); | ||
|
|
||
| it("should use default none fallback when options object provides only separator", () => { |
There was a problem hiding this comment.
The test name says it "should use default none fallback" when only { separator } is provided, but the expectation is undefined (i.e., no fallback applied). Rename the test to reflect the actual behavior being asserted, or (if the default is intended to be "N/A" per the function docs) adjust the expectation and implementation accordingly.
| it("should use default none fallback when options object provides only separator", () => { | |
| it("should not apply none fallback when options object provides only separator", () => { |
Several exported functions had zero test coverage, and key edge cases were untested across all packages. This adds targeted tests for uncovered paths without modifying source code.
@jabascript/querygetLiteralValue— exported but completely untested; adds fulldescribeblock covering"null"→null,"undefined"→undefined, and passthrough stringscreateSearchParams— empty array value, 2-element array not matching the Option pattern (falls through to regular array append), Option tuple with a missing key (serializes as"undefined")parseSearchParams—null/undefinedwithin array entries, 3+ occurrences of the same non-[]key@jabascript/form-dataorder.items[0].sku)@jabascript/corecreateEnum— direct property access, empty object inputdebounce— sequential independent calls both firing correctlygetInitials— partial options object (onlyseparator, nonone), both options provided together, lowercase input with separator that must uppercase