Add regression tests for enumerable resolution with mixed open/closed generics

[Copilot]

Description

Reported issue claimed GetServices<T>() inconsistently resolves services when mixing closed generic registrations (IService<int> -> Impl) with open generic registrations (IService<> -> GenericImpl<>), particularly when GetService<T>() is called first.

Tests confirm the behavior works correctly. Added regression coverage for this scenario.

Changes

• New test cases in ServiceProviderContainerTests:
  • EnumerableResolutionIncludesOpenGenericRegisteredAfterClosedGeneric - validates GetService followed by GetServices returns both registrations
  • EnumerableResolutionWithoutSingleServiceCallIncludesOpenGeneric - validates direct GetServices call returns both registrations

Example scenario now covered:

services.TryAddEnumerable(ServiceDescriptor.Scoped(typeof(IService<int>), typeof(ClosedImpl)));
services.TryAddEnumerable(ServiceDescriptor.Scoped(typeof(IService<>), typeof(OpenImpl<>)));

var single = provider.GetService<IService<int>>();     // Returns ClosedImpl
var all = provider.GetServices<IService<int>>();       // Returns both ClosedImpl and OpenImpl<int>

All 1372 DI tests pass. No code changes required - existing implementation handles this correctly.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Enumerable dependencies resolving not consistent when open generic is used (area-Extensions-DependencyInjection)</issue_title>
<issue_description>

Description

Enumerable dependencies are not resolved consistently when open generic services are registered later.

Ex:

Setup

interface IProcessor<T> {}

record AnyProcessor<T> : IProcessor<T>;

record IntProcessor : IProcessor<int>;

Registration

var services = new ServiceCollection();
services.TryAddEnumerable(ServiceDescriptor.Scoped(typeof(IProcessor<int>), typeof(IntProcessor)));
// Fix: ordering open generic above concretes
services.TryAddEnumerable(ServiceDescriptor.Scoped(typeof(IProcessor<>), typeof(AnyProcessor<>)));

Resolving

var serviceProvider = services.BuildServiceProvider();

using var scope = serviceProvider.CreateAsyncScope();

// bug is reproducible only when below line enabled
var processor = scope.ServiceProvider.GetService<IProcessor<int>>();

var processors = scope.ServiceProvider.GetService<IEnumerable<IProcessor<int>>>() ?? Enumerable.Empty<IProcessor<int>>();

// prints "IntProcessor -- IntProcessor" instead of IntProcessor -- AnyProcessor`1 if line 17 commented.
Console.WriteLine(string.Join(" -- ", processors.Select(p => p.GetType().Name)));

To Reproduce

sample repo to reproduce the bug https://github.com/skclusive/EnumerableCallSiteBugConsole

Configuration

• `dotnet --info
  .NET SDK (reflecting any global.json):
  Version: 6.0.100-preview.7.21379.14
  Commit: 22d70b47bc

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22000
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\6.0.100-preview.7.21379.14\

Host (useful for support):
Version: 6.0.0-preview.7.21377.19
Commit: 91ba017

.NET SDKs installed:
5.0.301 [C:\Program Files\dotnet\sdk]
5.0.400 [C:\Program Files\dotnet\sdk]
6.0.100-preview.7.21379.14 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.All 2.1.28 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.28 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.0-preview.7.21378.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.28 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.0-preview.7.21377.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.0-preview.7.21378.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

• Microsoft Visual Studio Community 2022 Preview (64-bit) Version 17.0.0 Preview 3.0

Regression?

Other information

• Fixes Enumerable dependencies resolving not consistent when open generic is used (area-Extensions-DependencyInjection) #57333

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[stephentoub]

@copilot please invoke the code-review skill and post the analysis/results as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post the analysis/results as a comment on this PR

🤖 Copilot Code Review — PR (commits faff47a, e30f09c)

Holistic Assessment

Motivation: The PR claims to address a reported issue where GetServices<T>() inconsistently resolves services when mixing closed and open generic registrations, particularly when GetService<T>() is called first. However, the tests all pass, suggesting either the issue was already fixed, the reproduction is incomplete, or the problem doesn't exist as described.

Approach: Adding regression tests is valuable, but the PR lacks investigation into whether there's an actual bug. The tests verify current behavior but don't demonstrate that a problem was fixed or validate that the behavior is correct by design.

Summary: ⚠️ Needs Human Review. While the tests themselves are technically sound, the PR's value proposition is unclear. The tests document current behavior, but without evidence of a bug or confirmation that this behavior is intentionally correct, we're codifying potentially incorrect behavior. A human reviewer should determine: (1) whether the reported issue is real but not reproduced by these tests, (2) whether the issue was already fixed and these are legitimate regression tests, or (3) whether the issue never existed.

---

Detailed Findings

⚠️ Test Value & Coverage — Tests may be documenting incorrect behavior

The core concern: These tests assert specific ordering (allServices[0] = FakeService, allServices[1] = ClassWithNoConstraints<PocoClass>) without verifying this ordering is intentional or correct. The PR description states "Tests confirm the behavior works correctly," but provides no evidence that returning items in registration order is the specified contract for GetServices.

Specific issues:

1. Order assertions may be brittle: Lines 920-921 and 939-940 assert exact index-based ordering. If the DI container's enumerable resolution order is not guaranteed by contract (only that all services are returned), these assertions create a false dependency on implementation details.

2. Missing negative test cases: The tests don't validate what happens when:

   • Only an open generic is registered (no closed variant)
   • Multiple open generics are registered
   • Open generic has constraints that don't match the requested type
   • Services are registered with different lifetimes (singleton, scoped, transient)

3. Incomplete scenario coverage: The reported issue mentions the bug is "reproducible only when below line enabled" (referring to GetService), suggesting a caching/state issue. Yet only one of the two tests calls GetService first. Missing test: what if GetServices is called, then GetService, then GetServices again?

4. No validation of "last wins" semantics: DI typically follows "last registration wins" for single service resolution. These tests don't verify whether this contract extends to enumerables or whether both registrations should always be returned.

Recommendation: Before merging, either:

• Provide evidence the reported issue was real (e.g., a failing test before a fix, or a specific commit that resolved it), or
• Confirm with a DI maintainer that returning all registrations in registration order is the guaranteed contract, or
• Relax the assertions to only verify count and presence (using Contains instead of index-based checks), acknowledging order is not guaranteed.

💡 Test Naming & Documentation — Minor improvement opportunity

The test names are descriptive but slightly verbose. Consider:

• EnumerableResolutionIncludesOpenGenericRegisteredAfterClosedGeneric → GetServices_WithOpenGenericAfterClosed_ReturnsBoth (follows established xUnit naming: MethodUnderTest_Scenario_ExpectedBehavior)
• EnumerableResolutionWithoutSingleServiceCallIncludesOpenGeneric → GetServices_WithoutPriorGetService_ReturnsBoth

However, this is minor and the existing names are acceptable.

✅ Test Structure — Follows established patterns

The tests correctly:

• Use the abstract CreateServiceProvider method from the base class, ensuring they run across all provider implementations
• Use TryAddEnumerable (the correct API for enumerable registrations)
• Follow the Arrange-Act-Assert pattern
• Use existing fake types (FakeService, ClassWithNoConstraints<>, PocoClass) from the test infrastructure

✅ Code Style — Matches file conventions

The code follows the existing style in the file:

• Consistent indentation and bracing
• Appropriate use of var
• Clear comments
• Proper xUnit attributes

---

Verdict Justification

I'm escalating to "Needs Human Review" rather than approving because:

1. Unresolved claim about the bug: The PR states an issue was reported, but provides no evidence it exists or existed. The tests pass, which could mean: (a) the bug is real but not reproduced, (b) the bug was already fixed, or (c) there was never a bug. Each scenario has different implications for whether these tests should merge.

2. Order-dependent assertions without specification: The tests assert specific ordering, but I found no documentation that GetServices guarantees registration order. If order is not guaranteed, these tests create maintenance burden (they'll break if the implementation lawfully changes iteration order).

3. Incomplete investigation: A proper fix for a reported bug would include: repro of the broken behavior, identification of the root cause, a minimal fix, and tests that fail without the fix. This PR only has tests, with no investigation into whether a fix was needed or applied.

What a human reviewer should focus on:

• Is there a real issue this PR addresses? If so, where is the fix?
• Is GetServices order guaranteed by design? If not, should these tests use unordered assertions?
• Are there related scenarios (constraints, multiple open generics, different lifetimes) that should also be tested?