test: matrix coverage for canonically-divergent store types#28
Merged
Conversation
Adds end-to-end integration tests that exercise the EF Core write path with the alias forms PR #25 now preserves on RelationalTypeMapping.StoreType: - LineString / MultiLineString (new entities + contexts + fixture tables) - MultiPolygon insert path (read-only test existed previously) - Json(name String, age Int32) round-trip + system.columns DDL inspection Empirically confirms that the driver registers Point/Ring/LineString/Polygon/ MultiLineString/MultiPolygon/Geometry as first-class plain types and parses Json type hints via JsonType.Parse — so {p:Point}, {p:Json(name String, ...)} and friends all flow through parameter binding without driver-level errors. Also retires four unit-test expectations that asserted the pre-PR-25 behavior (expanding aliases to structural form on StoreType) and renames them to *_PreservesAlias / _PreservesHints to reflect the new contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds parameterized coverage for store types where the resolved mapping's canonical StoreType differs from the user's HasColumnType text — the class of bug PR #25 fixes generally. Each case below was either silently lossy before (aliased to a generic fallback whose StoreType was the alias target) or canonicalized to a different string by the underlying mapping: - BFloat16 (aliased to Float32Mapping → "Float32") - Date32 (aliased to DateOnlyMapping → "Date") - Enum16 (aliased to StringMapping → "String") - Decimal32/64/256(S) (canonicalize to "Decimal(P,S)") - Decimal(P, S) (the raw form, same canonicalization) - FixedString(N) - Time64(N) - DateTime('UTC'), DateTime64(p, 'tz') Covers both layers: - Unit: FindMapping(...).StoreType matches user text verbatim - DDL: HasColumnType text flows through CreateTable into generated SQL The pattern matches the existing PR #25 tests for Enum8/AggregateFunction — adding new aliased or parameterized types to StoreTypeMappings/ParseStoreType without also extending these theories should be a code-review red flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #25 — broadens test coverage for the class of bug fixed there ("
HasColumnType(...)silently emits the wrong store type when the resolver canonicalizes to a generic fallback"). The previous test surface only enumeratedLowCardinality/Nullablewrappers and a handful of #25-specific cases. This PR adds:End-to-end round-trip integration tests (in
EFCore.ClickHouse.Tests):LineStringandMultiLineString(neither had any coverage)Insert_MultiPolygon_RoundTrip(only read was tested)JsonHintedRoundTripTests—Json(name String, age Int32)end-to-end via EF Core write/read, plus asystem.columnsDDL inspection that documents how ClickHouse normalizes the type (uppercasesJson→JSON, alphabetizes hint fields)Unit-level matrix coverage for canonically-divergent store types:
FindMapping_CanonicallyDivergentStoreType_PreservesVerbatimtheory coveringBFloat16,Date32,Enum16(...),Decimal32/64/256(S),Decimal(P, S),Time64(N),FixedString(N),DateTime('UTC'),DateTime64(6, 'Europe/Berlin')Parallel DDL preservation tests in
MigrationSqlGeneratorTests:HasColumnType_*_preserved_in_CreateTable_DDLforEnum16,FixedString,SimpleAggregateFunction,NestedThe intent: future additions to
StoreTypeMappingsorParseStoreTypeNameshould be flagged in review if they don't also extend these theories — the test surface is now the contract for "yourHasColumnTypetext survives to DDL and parameter binding."Test plan
EFCore.ClickHouse.Tests)mainpost-Fix: Enum8 and AggregateFunction dropped #25 merge🤖 Generated with Claude Code