Navigation test refactoring - improving structure for scenarios that don't support collections #35709
Conversation
…don't support collections Fixes #35707
| @@ -40,6 +40,7 @@ public class InMemoryComplianceTest : ComplianceTestBase | |||
| typeof(RelationshipsInProjectionQueryTestBase<>), | |||
| typeof(EntityRelationshipsIncludeQueryTestBase<>), | |||
| typeof(RelationshipsIncludeQueryTestBase<>), | |||
| typeof(RelationshipsNoCollectionsInProjectionQueryTestBase<>), | |||
There was a problem hiding this comment.
We should have a better name here, ReferenceRelationships? (also below)
| public abstract class RelationshipsNoCollectionsInProjectionQueryTestBase<TFixture>(TFixture fixture) : RelationshipsInProjectionQueryTestBase<TFixture>(fixture) | ||
| where TFixture : RelationshipsQueryFixtureBase, new() | ||
| { | ||
| public sealed override Task Project_trunk_collection(bool async) |
There was a problem hiding this comment.
IMHO this is IMHO the kind of pattern we should be avoiding - it would mean that every time we add a test that exercises collection relationships, we'd have to add an ignore here. Ideally, we'd have a test base class with only reference relationships (which can be extended multiple times - for JSON, for table splitting) and a another one which has tests for collection relationships.
In other words, organize our tests based on what's supported and what isn't; just like we want to separate tests that require navigations, since those can't work on Cosmos and other document databases.
There was a problem hiding this comment.
That was my initial approach, but I didn't like the outcome because of test class explosion (essentially doubling the number of classes) - will see if I can make it better.
|
converting to draft, see #35711 instead |
Plus minor cleanup
Fixes #35707