[DEV-1779] Fix event revision returned as a number instead of a bigint#520
Conversation
PR Summary by QodoRely on @kurrent/bridge 0.2.1 for runtime event type materialization Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. Ungated position type assertions
|
| expect(typeof event.position?.commit).toBe("bigint"); | ||
| expect(typeof event.position?.prepare).toBe("bigint"); |
There was a problem hiding this comment.
1. Ungated position type assertions 🐞 Bug ≡ Correctness
readStream’s new "runtime types" test always expects event.position.commit/prepare to be bigint, but position is only returned on server versions >= 22.6.0 so older servers will yield undefined and fail the test. This will make the test suite flaky/failing depending on the server version under test.
Agent Prompt
### Issue description
A new test asserts `event.position.commit/prepare` are `bigint` unconditionally, but log `position` is not available on older server versions, causing failures when tests run against <22.6.0.
### Issue Context
This file already gates log-position-related assertions via `const supported = matchServerVersion`>=22.6.0`` and `optionalDescribe(supported)(...)` later in the suite.
### Fix Focus Areas
- packages/test/src/streams/readStream.test.ts[92-111]
### Suggested fix
Split the test assertions:
- Always assert `revision` is `bigint` and `created` is a `Date`.
- Wrap only the `position` type assertions in `optionalDescribe(matchServerVersion`>=22.6.0`)(...)`, or otherwise conditionally assert them based on `supported`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
cab55ee to
951d0dd
Compare
2ba46ed to
3b32a66
Compare
#520) * Upgrade @kurrent/bridge to 0.2.2 * Simplify convertRustEvent to trust bridge-materialized types and honor optional position * Add runtime-type tests and fix a no-op created assertion
resolvedEvent.event.revisionis now abigintat runtime, matching its declared type. It was previously anumber, sorevision === 0nreturned false andrevision + 1nthrew.Closes #513