-
Notifications
You must be signed in to change notification settings - Fork 162
test: add comprehensive tests for BTreeIndex undefined value handling #1200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add unit tests to verify the sentinel mechanism that distinguishes "start from beginning" from "the key is literally undefined": - take(n, undefined) vs takeFromStart(n) disambiguation - takeReversed(n, undefined) vs takeReversedFromEnd(n) - Multiple items with undefined indexed values - Ordering of undefined relative to numbers, strings, and null - rangeQuery with explicit undefined bounds vs omitted parameter - Custom comparator receives actual undefined, not sentinel - equalityLookup(undefined) behavior - valueMapData getter returns denormalized keys Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: 0 B Total Size: 92 kB ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.7 kB ℹ️ View Unchanged
|
Add tests suggested by PR review: - inArrayLookup with undefined values in the lookup array - update operation transitioning to/from undefined values - take iteration past multiple undefined values Also includes code simplification from review: - Added createIndex helper function - Consolidated verbose assertions - Improved type safety (unknown vs any) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
kevin-dp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These unit tests are a great addition. We just need to rename that test file.
Rename btree-index-undefined-key-infinite-loop.test.ts to btree-index-undefined-values.test.ts since the tests now cover comprehensive undefined value handling, not just the infinite loop fix. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Adds comprehensive unit tests to verify the sentinel mechanism introduced in #1191 that fixes the infinite loop issue (#1186). These tests ensure the fix remains robust with 20 test cases covering all edge cases.
Test Coverage
Core Disambiguation
takevstakeFromStart- Verifiestake(n, undefined)returns items after the keyundefined, whiletakeFromStart(n)starts from the beginningtakeReversedvstakeReversedFromEnd- Same disambiguation for reverse iterationMultiple Undefined Values
undefinedindexed valuesOrdering
undefinedrelative to numbers, strings, and null valuesundefined, not the sentinel stringLookups & Queries
equalityLookup(undefined)- Lookup behavior for undefined valuesinArrayLookupwith undefined - Array lookups containing undefinedrangeQuerywith undefined bounds - Explicit{ from: undefined }vs omitted parameterOperations
updatetransitioning to/from undefined - Verifies index correctness when values change between defined and undefined statesvalueMapDatagetter - Verifies denormalization in returned Map (sentinel not exposed)Approach
Tests directly exercise the
BTreeIndexclass methods with explicit undefined values to verify:UNDEFINED_SENTINEL) correctly distinguishes "start from beginning" vs "key is undefined"undefinedvaluesVerification
pnpm --filter @tanstack/db test -- --run btree-index-undefined-valuesAll 20 tests pass.
Files Changed
packages/db/tests/btree-index-undefined-values.test.tsRelates to #1191, #1186
🤖 Generated with Claude Code