Fix failing Cosmos tests - #38808
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce test flakiness across environments by (1) skipping known-problematic Cosmos emulator scenarios on Linux and (2) improving test fixture cleanup by disposing the fixture’s service provider. It also adjusts a relational UDF test to run under non-RELEASE builds.
Changes:
- Dispose
SharedStoreFixtureBase’sServiceProviderduring fixture teardown. - Skip oversized-request Cosmos tests on the Linux Cosmos emulator via
ConditionalFact. - Update a relational UDF test to include non-RELEASE behavior (but the current expectation likely breaks Debug builds).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/EFCore.Specification.Tests/SharedStoreFixtureBase.cs | Adds service provider disposal during async fixture teardown. |
| test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs | Changes conditional compilation for a nullable-return UDF test to run in non-RELEASE builds. |
| test/EFCore.Cosmos.FunctionalTests/CosmosTransactionalBatchTest.cs | Skips oversized payload tests on Linux emulator where behavior differs (timeouts). |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
c58d7f4 to
4a26370
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs:135
- Same issue as above: without specifying an internal service provider, this path will create/cache another root provider that isn't deterministically disposed. Reuse the fixture's provider so cleanup happens via
SharedStoreFixtureBase.DisposeAsync().
.Options;
test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs:71
- After removing the explicitly managed
ServiceProvider, these options will cause EF to build/cache an internal root service provider (viaServiceProviderCache) which isn't deterministically disposed during test cleanup. Since the fixture already owns and disposes a provider, reuse it here to avoid leaking providers/resources across the test run.
This issue also appears on line 135 of the same file.
.Options;
4a26370 to
4f3f7d2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs:126
- The locally created
serviceProvideris never used (it's not passed viaUseInternalServiceProvideror otherwise referenced), so this allocates and disposes a provider for no effect. This adds unnecessary overhead to the test and makes the intent unclear.
await using var serviceProvider = new ServiceCollection()
.AddEntityFrameworkCosmos()
.BuildServiceProvider();
test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs:412
DeleteDocumentsAsyncqueriesSELECT VALUE c FROM c, which downloads entire documents (including any large payload properties) and then buffers all(id, partitionKey)pairs in memory before deleting. For shared stores this can significantly slow down cleanup and increase memory usage. Consider projecting only the needed fields (id + partition key store names) and deleting as you iterate pages.
var documents = new List<(string Id, PartitionKey PartitionKey)>();
using var iterator = container.GetItemQueryIterator<JsonElement>("SELECT VALUE c FROM c");
while (iterator.HasMoreResults)
{
Cleanup service providers proactively in tests
4f3f7d2 to
f90e2ed
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs:1077
- The non-RELEASE branch expects UnreachableException, but the translation pipeline throws InvalidOperationException for nullable-value-type translations regardless of build configuration (see RelationalMethodCallTranslatorProvider throwing InvalidOperationException for nullable value types). This will make the test fail in non-RELEASE builds.
#if RELEASE
var exception = Assert.Throws<InvalidOperationException>(
() => context.Customers.Where(c => c.Id == UDFSqlContext.NullableValueReturnType()).ToList());
Assert.Equal(
RelationalStrings.DbFunctionNullableValueReturnType(
context.Model.FindDbFunction(typeof(UDFSqlContext).GetMethod(nameof(UDFSqlContext.NullableValueReturnType))!)!.ModelName,
"int?"),
exception.Message);
#else
Assert.Throws<UnreachableException>(
() => context.Customers.Where(c => c.Id == UDFSqlContext.NullableValueReturnType()).ToList());
#endif
test/EFCore.Cosmos.FunctionalTests/CosmosConcurrencyTest.cs:140
- A ServiceProvider is created and disposed, but it isn’t actually used by EF for internal services since it isn’t passed into the options. If the intent is to proactively dispose the internal provider/CosmosClient resources, pass it via UseInternalServiceProvider; otherwise remove the ServiceProvider creation.
await using var serviceProvider = new ServiceCollection()
.AddEntityFrameworkCosmos()
.BuildServiceProvider();
var options = Fixture.TestStore.AddProviderOptions(
Fixture.AddOptions(
new DbContextOptionsBuilder()
.UseCosmos(o =>
{
if (contentResponseOnWriteEnabled != null)
{
#pragma warning disable CS0618 // Type or member is obsolete
o.ContentResponseOnWriteEnabled(contentResponseOnWriteEnabled.Value);
#pragma warning restore CS0618 // Type or member is obsolete
}
})))
.EnableServiceProviderCaching(false)
.Options;
Uh oh!
There was an error while loading. Please reload this page.