Skip to content

Tests: broaden #4602 coverage across all four storage selectors#4604

Merged
jeremydmiller merged 1 commit into
masterfrom
tests/4602-cross-selector-coverage
Jun 2, 2026
Merged

Tests: broaden #4602 coverage across all four storage selectors#4604
jeremydmiller merged 1 commit into
masterfrom
tests/4602-cross-selector-coverage

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Follow-up to #4603, which fixed the QueryOnly ordinal mismatch and shipped a 5-test repro covering the headline case (tenant-mapped projection document). The bug envelope is meaningfully wider than that framing suggests, and the fix scope deserves a structural lock.

What this adds

Five probes covering the full bug envelope, each parameterized across all four storage selectors (QueryOnly, Lightweight, IdentityMap, DirtyTracking) for a 20-test matrix in src/EventSourcingTests/Bugs/Bug_4602_query_versioned_with_mapped_metadata.cs.

Probes (5)

# Probe What it catches
1 Non-projection doc + UseNumericRevisions(true) + mapped tenant_id Not projection-target-specific. User manually opting in via o.Schema.For<T>().UseNumericRevisions(true) hits the same crash.
2 Non-projection doc + UseOptimisticConcurrency(true) + mapped tenant_id Sibling code path in the builder (VersionColumn branch lines 70-81). If a fix only addresses the numeric path, this stays broken.
3 Projection target + mapped LastModified (no tenancy) Not tenant_id-specific. Any after-version mapped member shifts QueryOnly.
4 Non-projection doc + UseNumericRevisions(true) + mapped CorrelationId Combines #1 and #3 — no projection, no tenancy.
5 Hierarchical projection target + mapped tenant_id doc_type sits before the version binder so it reads correctly, but the version → tenant_id shift still trips QueryOnly. Error bound becomes "0 and 2" because the SELECT is one column wider.

Selector matrix (× 4)

Each probe runs against QueryOnly, Lightweight, IdentityMap, and DirtyTracking sessions:

  • QueryOnly — the regression proof. Failed on bare master, passes with Fix #4602: Query<T>() throws IndexOutOfRangeException on tenant-mapped projection documents #4603.
  • Lightweight / IdentityMap / DirtyTracking — canaries. Those selectors were never broken (their SELECTs include mt_version for the write-back path even without a member), so they stayed green on master. They pin the fix scope: a future refactor that accidentally trimmed ReadBinders itself (instead of just QueryOnlyReadBinders) would crash all 15 non-QueryOnly cases immediately.

Verification

State Result
Bare master (pre-#4603) 5/20 fail — exactly the 5 QueryOnly cases
Master @ 8921251682 (post-#4603) 20/20 pass

Re-ran the closed-shape regression bands as a sanity check:

  • CoreTests closed_shape_*82/82 pass
  • DocumentDbTests Concurrency + Metadata + Version168/168 pass

Why this lives in a separate file

Bug_4602_query_tenant_mapped_projection_document.cs (from #4603) is the public-facing bug repro and stays untouched. This file documents the broader envelope and is the structural canary; keeping them separate makes the issue's reproduction obvious and lets this matrix evolve independently if more after-version metadata column types get added later.

🤖 Generated with Claude Code

PR #4603 fixed the QueryOnly ordinal mismatch and shipped a 5-test repro
covering the headline case (tenant-mapped projection document). The bug
envelope is meaningfully wider:

  - NOT projection-target-specific. UseNumericRevisions /
    UseOptimisticConcurrency turned on directly on a non-projection
    mapping (e.g. via o.Schema.For<T>().UseNumericRevisions(true)) hits
    the same descriptor state and the same crash.
  - NOT tenant_id-specific. Any after-version metadata member mapped via
    .Metadata(m => m.X.MapTo(...)) shifts the QueryOnly ordinals:
    last_modified, created_at, correlation_id, causation_id,
    last_modified_by.
  - Affects hierarchical mappings too. doc_type sits before the version
    binder so it reads correctly, but the version → tenant_id shift
    still trips QueryOnly (error bound becomes "0 and 2" instead of
    "0 and 1" because the SELECT is one column wider).

Adds 5 probes covering this envelope, each parameterized across all
four selectors (QueryOnly, Lightweight, IdentityMap, DirtyTracking) for
a 20-test matrix. The QueryOnly case in each probe is the regression
proof; the Lightweight / IdentityMap / DirtyTracking cases are canaries
— those selectors were never broken (their SELECTs include mt_version
for the write-back path), so they pin the fix scope. A future refactor
that accidentally trimmed ReadBinders itself (instead of just
QueryOnlyReadBinders) would crash all 15 non-QueryOnly cases
immediately.

Verified on master:
  - Bare master:   5/20 fail (exactly the 5 QueryOnly cases)
  - After #4603:  20/20 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller jeremydmiller merged commit 3e57fb6 into master Jun 2, 2026
8 of 9 checks passed
@jeremydmiller jeremydmiller deleted the tests/4602-cross-selector-coverage branch June 2, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant