Skip to content

Fix flaky ServicePropertyDiscoveryConventionTest.Finds_service_properties_in_hierarchy#38128

Merged
AndriySvyryd merged 1 commit intomainfrom
copilot/fix-flaky-service-property-test
Apr 17, 2026
Merged

Fix flaky ServicePropertyDiscoveryConventionTest.Finds_service_properties_in_hierarchy#38128
AndriySvyryd merged 1 commit intomainfrom
copilot/fix-flaky-service-property-test

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 17, 2026

Service_provider_cache_can_be_cleared calls Clear() on the global ServiceProviderCache.Instance singleton. When running concurrently with Finds_service_properties_in_hierarchy, the cache gets cleared between the two using blocks, causing the second context to resolve a new service provider with an empty in-memory store → 0 entries loaded.

  • Root cause fix: Service_provider_cache_can_be_cleared now uses a local new ServiceProviderCache() — it's testing Clear() behavior, not the global singleton
  • Defensive fix: ServicePropertiesContext now uses a shared InMemoryDatabaseRoot so the store survives service provider recreation
// Before: global singleton cleared during parallel test execution
var cache = ServiceProviderCache.Instance;
cache.Clear(); // races with any test relying on cached service providers

// After: isolated instance
var cache = new ServiceProviderCache();
cache.Clear();

The test was flaky because ServiceProviderCacheTest.Service_provider_cache_can_be_cleared
called Clear() on the global ServiceProviderCache.Instance singleton, which could race
with the hierarchy test when running in parallel. When the cache was cleared between the
two using blocks, the second context got a new service provider with an empty in-memory
database, resulting in 0 loaded entries.

Two fixes applied:
1. Changed Service_provider_cache_can_be_cleared to use a local ServiceProviderCache
   instance instead of the global singleton (root cause fix)
2. Added a shared InMemoryDatabaseRoot to ServicePropertiesContext so the in-memory
   database survives service provider changes (defensive fix)

Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/c7ac9a3c-4847-4884-8223-c969d57ac007

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
@AndriySvyryd AndriySvyryd marked this pull request as ready for review April 17, 2026 01:25
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner April 17, 2026 01:25
Copilot AI review requested due to automatic review settings April 17, 2026 01:25
@AndriySvyryd AndriySvyryd assigned roji and unassigned AndriySvyryd and Copilot Apr 17, 2026
@AndriySvyryd AndriySvyryd enabled auto-merge (squash) April 17, 2026 01:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a flaky test caused by concurrent clearing of the global ServiceProviderCache.Instance, which could force recreation of a service provider and unintentionally lose the InMemory store between two DbContext instances in the same test.

Changes:

  • Updated Service_provider_cache_can_be_cleared to use an isolated new ServiceProviderCache() instance rather than the global singleton.
  • Made ServicePropertiesContext use a shared InMemoryDatabaseRoot so the InMemory store survives service provider recreation during the test.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/EFCore.Tests/ServiceProviderCacheTest.cs Avoids clearing the global cache by using a local ServiceProviderCache instance in the Clear() behavior test.
test/EFCore.Tests/Metadata/Conventions/ServicePropertyDiscoveryConventionTest.cs Uses a shared InMemoryDatabaseRoot to keep InMemory data stable across multiple contexts even if the internal service provider changes.

@AndriySvyryd AndriySvyryd merged commit 6151954 into main Apr 17, 2026
20 checks passed
@AndriySvyryd AndriySvyryd deleted the copilot/fix-flaky-service-property-test branch April 17, 2026 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants