Fix Oracle EF Core mapping test and run the Oracle suite in CI (#394) - #395
Merged
Conversation
can_map_entity_to_table asserted PrimaryKeyColumns contains a lowercase "id". The mapping has never produced that. MapToTable sets PreserveIdentifierCase, so column names carry EF Core's casing verbatim - the names given to HasColumnName() where the model configures one, the CLR property name otherwise. Dumping the mapped table confirms it: Id, BoolValue, CASCADE_VAL, DateOnlyValue, DT_OFFSET_VAL, DateTimeValue, GuidValue, IntValue, NULL_*, StringValue, TimeOnlyValue So the product was right and the assertion was wrong. My first reading in #394 - that PrimaryKeyColumns was failing to lowercase along with the other columns - was mistaken: nothing lowercases them. The reason that went unnoticed is worth fixing too. Oracle's Table compares names with OrdinalIgnoreCase, so the surrounding HasColumn("intvalue") calls succeed against a column actually named "IntValue" and assert nothing about casing at all. PrimaryKeyColumns.ShouldContain was the one case-sensitive assertion in the test, which is why it alone failed. Replaced the HasColumn series with one assertion on the full column-name set, so the test pins the casing contract instead of passing regardless of it, and fixed the comment claiming "Oracle lowercases column names in Weasel". Predates the xUnit v3 migration: verified the identical failure on the pre-migration commit. CI never caught it because ci-build-efcore.yml has no Oracle service and filters the folder out. Verified against local Docker: EF Core Oracle 3 passed / 1 pre-existing skip, and Weasel.Oracle.Tests 186 on both target frameworks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…el#394) The EF Core workflow ran with --filter "FullyQualifiedName!~Oracle" because it had no Oracle service, so the Oracle/ folder never executed in CI. That is how can_map_entity_to_table sat red for anyone running the suite locally without anything noticing - fixing the assertion is only half the issue. Oracle comes up through docker compose rather than as a GitHub service container, matching ci-build-oracle.yml: the container needs docker/oracle mounted as init SQL to grant the weasel user its schema privileges, which a service container cannot do. Scoped to the net9.0 leg with an if condition. The Oracle EF provider has no net10.0 support, so the csproj already compiles the Oracle folder out of that build - starting a database it cannot use would just add several minutes. The start step goes before the SDK install and the health wait after the build, so Oracle warms up during restore and build rather than blocking on its own. The job timeout goes to 45 minutes to match ci-build-oracle.yml, and a `docker compose down` runs with if: always(). The test filter is dropped entirely rather than narrowed: with Oracle available on net9.0, and both the Oracle and MySql folders compiled out of net10.0, each framework should simply run everything it built. This is the first time the Oracle EF tests will execute in CI, so CI is the only place it can be verified; locally they pass 3 with 1 pre-existing skip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Fixes #394. Two parts: the failing assertion, and the CI gap that let it stay failing.
1. The test asserted a casing the mapping never produced
Oracle.end_to_end.can_map_entity_to_tablefailed against a live Oracle instance:There is no product bug.
MapToTablesetsPreserveIdentifierCase = truedeliberately, so column names carry EF Core's casing verbatim. Dumping the mapped table confirms it:Columns take the name given to
HasColumnName()where the model configures one, and the CLR property name otherwise. Nothing is lowercased.MyEntity.Idhas noHasColumnName(), soPrimaryKeyColumnsreturning["Id"]is correct.My initial reading in #394 — that
PrimaryKeyColumnswas failing to lowercase along with the other columns — was mistaken, and I have corrected the issue.Why it went unnoticed, and why this is not a one-character fix.
Weasel.Oracle'sTablecompares names withOrdinalIgnoreCase, andHasColumnuses that comparison, sosucceeds against a column actually named
"IntValue"— and would succeed for any casing whatsoever. The test held fourteen assertions that could not fail plus one case-sensitive assertion written against a convention the code never had. Flipping"id"to"Id"would leave it just as unable to catch a casing regression, so theHasColumnseries is replaced with a single assertion on the full column-name set, and the// Oracle lowercases column names in Weaselcomment is corrected.2. CI could not see any of this
ci-build-efcore.ymlran with--filter "FullyQualifiedName!~Oracle"because the workflow had no Oracle service, so theOracle/folder never executed. Fixing the assertion without fixing that would leave the next Oracle EF regression equally invisible.Oracle now comes up through docker compose rather than as a GitHub service container, matching
ci-build-oracle.yml— the container needsdocker/oraclemounted as init SQL to grant theweaseluser its schema privileges, which a service container cannot do.ci-build-oracle.yml, withdocker compose downonif: always().Verification
Locally against Docker: EF Core Oracle tests 3 passed / 1 skipped — the skip is the pre-existing
ORA-03048one, untouched — andWeasel.Oracle.Tests186 passed on both net9.0 and net10.0.The CI wiring itself can only be verified in CI, since this is the first time these tests will run there. The EF Core net9.0 check on this PR is the real test of part 2.
🤖 Generated with Claude Code