fix(test): keep adapter coverage aligned with EventStore semantics#87
fix(test): keep adapter coverage aligned with EventStore semantics#87
Conversation
yordis
commented
Apr 16, 2026
yordis
commented
- Reduces the risk of the in-memory adapter masking behaviors that only fail against the Postgres-backed EventStore.
- Keeps the shared adapter contract anchored in realistic domain events and snapshots instead of synthetic fixtures.
- Protects duplicate event-id semantics so compatibility gaps surface in adapter tests before they reach production paths.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryLow Risk Overview Test helpers were updated so Coverage is expanded to assert explicit event-id propagation and duplicate event-id rejection across streams, and to verify snapshot reads preserve key fields and metadata across adapters; telemetry/opentelemetry and projection tests were updated to use the new builders. Reviewed by Cursor Bugbot for commit e3c70ac. Bugbot is set up for automated code reviews on this repo. Configure here. |
WalkthroughA new test support module Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/event_store/support/adapter_test_data.ex (3)
17-28: Optional: forwardingoptstobuild_opened_event/1is harmless but opaque.
build_opened_event(opts)on Line 22 is handed the full opts keyword (including:correlation_id,:causation_id,:metadata); those keys are silently ignored bybuild_opened_event/1since it only reads:account_numberand:initial_balance. This works but makes it unclear which keys belong to which layer. Consider passing only the struct-relevant keys, e.g.build_opened_event(Keyword.take(opts, [:account_number, :initial_balance])).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/event_store/support/adapter_test_data.ex` around lines 17 - 28, build_opened_event_data currently forwards the entire opts to build_opened_event (which only uses :account_number and :initial_balance), making ownership of keys opaque; change build_opened_event_data to pass only the event-struct keys to build_opened_event (e.g. use Keyword.take(opts, [:account_number, :initial_balance]) or equivalent) while keeping correlation_id, causation_id, and metadata handled locally before calling Mapper.map_to_event_data so build_opened_event receives only the keys it intends to consume.
48-74: Naming:build_deposit_eventvsbuild_deposited_event/build_deposited_event_datais confusing.Three functions with similar names express different return types:
build_deposited_event/1→ raw%MoneyDeposited{}structbuild_deposited_event_data/1→ mapped%EventData{}build_deposit_event/2→ mapped%EventData{}(same asbuild_deposited_event_data) but with a positionalaccount_number
build_deposit_eventbeing a positional-arg wrapper that returns the same shape asbuild_deposited_event_data— whilebuild_deposited_eventreturns a different shape — is easy to misuse at call sites. Consider renaming for symmetry with the opened helpers (build_opened_event/build_opened_event_data/build_opened_events), e.g.,build_deposited_events/2or keeping the wrapper but naming itbuild_deposit_event_data/2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/event_store/support/adapter_test_data.ex` around lines 48 - 74, Rename the confusing helper build_deposit_event/2 to a name that clearly indicates it returns EventData (e.g., build_deposit_event_data/2 or build_deposited_events/2) so it matches build_deposited_event_data/1 and distinguishes from build_deposited_event/1 (the raw %MoneyDeposited{} struct); update the function definition (originally named build_deposit_event) and all call sites to the new name and ensure it still forwards to build_deposited_event_data by passing the account_number into the opts (use Keyword.put(opts, :account_number, account_number)) so behavior remains identical.
80-102: Minor:source_versiondoubling as defaultaccount_numberreads oddly.On Line 88, the default
account_numberfalls back tosource_version.source_versionsemantically represents an aggregate version (typically small, monotonically increasing), whileaccount_numberis an identifier. Callers insnapshot_test_case.expass100,101,102assource_versionpurely to get unique default account numbers, which obscures the test intent.Consider decoupling the defaults (e.g., default
source_versionto1and let callers passaccount_numberexplicitly when uniqueness matters), or renaming the positional arg. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/event_store/support/adapter_test_data.ex` around lines 80 - 102, build_snapshot_data currently uses source_version as the default account_number which conflates version with identifier; update build_snapshot_data to decouple these defaults: change the positional default for source_version to a stable default (e.g., 1) and stop using source_version for account_number — instead derive account_number from opts (Keyword.get(opts, :account_number, <a unique default like UUID.uuid4/0>)) so callers who need unique account numbers pass :account_number explicitly (adjust snapshot_test_case.ex callers to pass account_number where uniqueness was previously achieved via source_version).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/event_store/support/adapter_test_data.ex`:
- Around line 17-28: build_opened_event_data currently forwards the entire opts
to build_opened_event (which only uses :account_number and :initial_balance),
making ownership of keys opaque; change build_opened_event_data to pass only the
event-struct keys to build_opened_event (e.g. use Keyword.take(opts,
[:account_number, :initial_balance]) or equivalent) while keeping
correlation_id, causation_id, and metadata handled locally before calling
Mapper.map_to_event_data so build_opened_event receives only the keys it intends
to consume.
- Around line 48-74: Rename the confusing helper build_deposit_event/2 to a name
that clearly indicates it returns EventData (e.g., build_deposit_event_data/2 or
build_deposited_events/2) so it matches build_deposited_event_data/1 and
distinguishes from build_deposited_event/1 (the raw %MoneyDeposited{} struct);
update the function definition (originally named build_deposit_event) and all
call sites to the new name and ensure it still forwards to
build_deposited_event_data by passing the account_number into the opts (use
Keyword.put(opts, :account_number, account_number)) so behavior remains
identical.
- Around line 80-102: build_snapshot_data currently uses source_version as the
default account_number which conflates version with identifier; update
build_snapshot_data to decouple these defaults: change the positional default
for source_version to a stable default (e.g., 1) and stop using source_version
for account_number — instead derive account_number from opts (Keyword.get(opts,
:account_number, <a unique default like UUID.uuid4/0>)) so callers who need
unique account numbers pass :account_number explicitly (adjust
snapshot_test_case.ex callers to pass account_number where uniqueness was
previously achieved via source_version).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04e67cc9-9c7f-4f89-91bd-2f56361f789c
📒 Files selected for processing (14)
test/aggregates/aggregate_lifespan_test.exstest/event_store/adapters/in_memory/in_memory_test.exstest/event_store/recorded_event_test.exstest/event_store/support/adapter_test_data.extest/event_store/support/append_events_test_case.extest/event_store/support/event_store_prefix_test_case.extest/event_store/support/snapshot_test_case.extest/event_store/support/subscription_test_case.extest/event_store/telemetry_test.exstest/helpers/event_factory.extest/opentelemetry/event_store_test.exstest/projections/error_callback_test.exstest/projections/runtime_config_projector_test.exstest/support/factory.ex
1 participant
