test(o11ylogsdb): unit test suite (82 tests, 8 files)#182
Conversation
Locks down the engine's invariants before the codec stack starts swapping in Rust/WASM behind it. All tests are TS-only against the existing `src/` implementation. Files and what they cover: - chunk.test.ts: ChunkBuilder freeze semantics (empty chunk, time range, severity zone-map, resource/scope hoisting); wire-format serialize/deserialize round-trip; chunkWireSize == serializeChunk length; ChunkPolicy hooks (preEncode/postDecode meta blob, encodePayload/decodePayload bypass). - stream.test.ts: StreamRegistry interns same id for ref-equal and structurally-equal (resource, scope) tuples, separates by service or scope, preserves chunk insertion order, throws on unknown id. - drain.test.ts: tokenize, similarity, mergeTemplate; Drain matchOrAdd / matchTemplate / reconstruct; whitespace normalization; default config matches the published reference. - engine.test.ts: LogStore append/flush/iterRecords; rowsPerChunk cap closes chunks; stats() bytes/log; policyFactory creates one policy per stream and reuses across chunks. - policy-roundtrip.test.ts: every shipped ChunkPolicy (Default/ColumnarRaw/ColumnarDrain/Drain/TypedColumnar) round- trips records on templated, single-record, and empty chunks. Adds slot-detector cases for TypedColumnar (PREFIXED_INT64, UUID, SIGNED_INT, sub-threshold STRING fallback). - query.test.ts: time range, severity zone-map pruning, resourceEquals, bodyContains, bodyLeafEquals dot-paths, limit; chunksPruned / streamsPruned stats accuracy. - compact.test.ts: compactChunk re-encodes payload, preserves records, no-ops when target == current, doesn't mutate input. - public-api.test.ts: smoke check that the index.ts surface exports every documented symbol; catches accidental drops on refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds comprehensive test coverage to the o11ylogsdb package via seven new Vitest modules. Tests cover chunk creation/serialization, compaction and codec conversion, Drain template clustering, LogStore engine behaviors (append, chunking, iteration, stats), chunk policy encode/decode round-trips, public API exports, query filtering/pruning, and StreamRegistry internment. All changes are test-only; no exported or public entity declarations were modified. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review 👍 Approved with suggestions 0 resolved / 1 findingsComprehensive unit test suite for o11ylogsdb coverage. Update the checkout comment on line 23 to accurately reflect the seven records appended.
💡 Quality: Comment says "8 records" but only 7 are appended for checkout📄 packages/o11ylogsdb/test/query.test.ts:23 The comment on line 23 says Suggested fix🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Important Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
| const resA: Resource = { attributes: [{ key: "service.name", value: "checkout" }] }; | ||
| const resB: Resource = { attributes: [{ key: "service.name", value: "payments" }] }; | ||
| const store = new LogStore({ rowsPerChunk: 4 }); | ||
| // checkout: 8 records spanning t=1000-7000, mixed severities |
There was a problem hiding this comment.
💡 Quality: Comment says "8 records" but only 7 are appended for checkout
The comment on line 23 says // checkout: 8 records spanning t=1000-7000 but only 7 store.append(resA, …) calls follow (t=1000, 2000, 3000, 4000, 5000, 6000, 7000). The test assertions themselves are consistent with 7 records (e.g., recordsScanned = 11 = 7 + 4), so the tests pass correctly — only the comment is misleading.
Suggested fix:
- // checkout: 8 records spanning t=1000-7000, mixed severities
+ // checkout: 7 records spanning t=1000-7000, mixed severities
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
Comment said 8 records but only 7 appends follow. Test math (recordsScanned = 7 + 4 = 11) was already correct; just the comment was wrong. Bot-flagged on PR #182. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Locks down the engine's invariants before the codec stack starts swapping in Rust/WASM behind it. All TS-only against the existing `src/`. 82 tests across 8 files; full repo suite goes from 479 → 561 tests.
Why now
Next milestone is M0 (codec workspace migration) and M1/M2 (Rust crates for FSST, binary fuse, Drain). Each one swaps in a new implementation behind the same interface. Tests give a regression net before that work starts.
Test plan
🤖 Generated with Claude Code