Promote Json tests to Core so that they can be utilized for Cosmos.#34355
Promote Json tests to Core so that they can be utilized for Cosmos.#34355
Conversation
| WHERE ((c["Discriminator"] = "Basic") AND (ARRAY( | ||
| SELECT VALUE o["OwnedReferenceLeaf"]["SomethingSomething"] | ||
| FROM o IN c["OwnedReferenceRoot"]["OwnedCollectionBranch"] | ||
| WHERE (o["Enum"] = -3))[0] = "e1_r_c2_r")) |
There was a problem hiding this comment.
@ajcvickers should we map enums to negative values like that?
| """ | ||
| SELECT VALUE c | ||
| FROM root c | ||
| WHERE ((c["Discriminator"] = "AllTypes") AND (c["Reference"]["TestEnumWithIntConverter"] != -3)) |
There was a problem hiding this comment.
same here, sus enum value
| """ | ||
| SELECT VALUE c | ||
| FROM root c | ||
| WHERE ((c["Discriminator"] = "AllTypes") AND (c["Reference"]["TestNullableEnum"] != -1)) |
| // TODO: remove all this infra once we support converters on cosmos | ||
| // also undo virtual on base | ||
| // issue #34026 | ||
| public override IReadOnlyDictionary<Type, object> EntityAsserters { get; } = new Dictionary<Type, Action<object, object>> |
There was a problem hiding this comment.
this is not great, but I wanted to do as little disruption as possible - we need custom asserters because some properties with converters are not supported on cosmos and therefore are being ignored in the model. This causes entity asserters to fail (not seeing the expected data for those ignored columns). Asserters are static on the base so I just copied them here and removed the unwanted parts. Once #34026 is fixed we can remove all this code.
| // TODO: issue #34067 (?) | ||
| AssertSql( | ||
| """ | ||
| SELECT VALUE c |
maumar
force-pushed
the
cosmos_tests
branch
2 times, most recently
from
dba0891 to
5b5a964
Compare
There was a problem hiding this comment.
See comments below, and let's wait also for @ajcvickers comments, but otherwise LGTM - this adds some good coverage to Cosmos and uncovered some bugs. We can always improve this further later.
Re the specific issues/bugs, I can investigate after this is merged.
| .IncludeDiscriminatorInJsonId() | ||
| .HasDiscriminator<string>("Discriminator").HasValue("Basic"); | ||
|
|
||
| modelBuilder.Entity<JsonEntityBasic>().Property(x => x.Id).ValueGeneratedNever(); |
There was a problem hiding this comment.
We should maybe move away from value-generated IDS by default... As they're not supported across all databases, and have little value where we're defining the test data - better to just hardcode the ID values like we hardcode the rest of the properties in the test data?
| { | ||
| bb.OwnsOne(x => x.OwnedReferenceLeaf); | ||
| bb.OwnsMany(x => x.OwnedCollectionLeaf); | ||
| bb.Property(x => x.Fraction).HasPrecision(18, 2); |
maumar
force-pushed
the
cosmos_tests
branch
2 times, most recently
from
d3f598a to
a0b085d
Compare
2 participants
No description provided.