Add chat client provider registration coverage

[Copilot]

ChatClientManager registration and service-wiring behavior lacked direct coverage, leaving duplicate provider registration and builder integration unprotected.

• Manager coverage

  • Covers null factory validation, duplicate registration rejection, empty builds, and provider factory invocation/service registration.

• Public extension coverage

  • Covers invalid builder rejection.
  • Verifies provider registration flows through a real TestApplicationBuilder.
  • Exercises the duplicate-provider guard through the public API.

• API documentation

  • Documents the single-provider-per-application invariant and relevant exceptions on AddChatClientProvider.

builder.AddChatClientProvider(_ => provider);
builder.ChatClientManager.BuildChatClients(serviceProvider);

Assert.AreSame(provider, serviceProvider.GetService(typeof(IChatClientProvider)));

• Fixes [quality-improver] ChatClientManager registration logic (AddChatClientProvider/BuildChatClients) has zero unit test coverage #11096

[Copilot]

Copilot wasn't able to review any files in this pull request.

[Copilot]

🟡 Changes recommended

The unresolved null-validation and localization-sensitive test issues should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 4
• Review effort level: Balanced

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed the review comments in 80ce504:

• Added null validation and updated exception XML documentation for AddChatClientProvider and GetChatClientAsync.
• Added unit tests for parameter validation in ChatClientProviderExtensionsTests.
• Replaced hard-coded English exception message assertions with PlatformResources.ChatClientProviderAlreadyRegistered in ChatClientManagerTests and ChatClientProviderExtensionsTests.
• Asserted the locked builder type fragment in ChatClientProviderExtensionsTests for cross-culture stability.

[Copilot]

🟢 Approval recommended

The reviewed changes have appropriate coverage and no unresolved issues.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.

[github-actions]

🧵 Parallel-safety audit — PR #11101

Nothing audited here touches process-global state, shared filesystem paths, or [ResourceLock] / [DoNotParallelize] declarations. The changes are limited to null-argument validation in ChatClientProviderExtensions and corresponding unit tests that operate on local ChatClientManager/ServiceProvider/mocked ITestApplicationBuilder instances — no shared global state or fixture members are involved. Nothing to flag for parallel-safety.

Audited Microsoft.Testing.Platform.UnitTests (project uses no [assembly: Parallelize]/[assembly: DoNotParallelize] visible in the changed diff; scope not altered by this PR).

_{Re-run with /parallel-audit.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 34.9 AIC · ⌖ 3.83 AIC · ⊞ 24.9K · [◷]( · ◷)

[github-actions]

🧪 Expert test review — PR #11101

Reviewed the new/modified tests in ChatClientManagerTests.cs and ChatClientProviderExtensionsTests.cs, which cover the new argument-null validation and the "already registered" guard for AddChatClientProvider/GetChatClientAsync.

Grade	Test	Mutation	Notes	How to improve
F (0–59)	new ChatClientManagerTests. BuildChatClients_ WhenProviderIsRegistered_ InvokesFactoryAndRegistersProvider	N/A	Lambda parameter serviceProvider shadows the enclosing local of the same name (CS0136) — this test does not compile.	Rename the lambda parameter (e.g. sp) so it no longer collides with the outer local.
A (90–100)	new ChatClientManagerTests. AddChatClientProvider_ WhenFactoryIsNull_ Throws	2/2 killed	Verifies both exception type and ParamName for the null-factory guard.	—
A (90–100)	new ChatClientManagerTests. AddChatClientProvider_ WhenProviderAlreadyRegistered_ Throws	2/2 killed	Registers once, asserts the second call throws and compares message via the shared resource accessor (localization-safe).	—
A (90–100)	new ChatClientManagerTests. BuildChatClients_ WhenNoProviderIsRegistered_ DoesNotAddAProvider	1/1 killed	Confirms no service is registered absent a factory.	—
A (90–100)	new ChatClientProviderExtensionsTests. AddChatClientProvider_ WhenTestApplicationBuilderIsNull_ Throws	2/2 killed	Covers the new null-builder guard with correct ParamName.	—
A (90–100)	new ChatClientProviderExtensionsTests. AddChatClientProvider_ WhenChatClientProviderIsNull_ Throws	2/2 killed	Covers the new null-factory guard at the public extension boundary with the correct public ParamName.	—
A (90–100)	new ChatClientProviderExtensionsTests. AddChatClientProvider_ WhenBuilderIsNotTestApplicationBuilder_ Throws	1/1 killed	Asserts on the locked type-name fragment, so it survives localization.	—
A (90–100)	new ChatClientProviderExtensionsTests. AddChatClientProvider_ RegistersProviderWithBuilder	2/2 killed	End-to-end check that the builder wires the provider through to the service provider.	—
A (90–100)	new ChatClientProviderExtensionsTests. AddChatClientProvider_ WhenProviderAlreadyRegistered_ Throws	1/1 killed	Duplicate-registration message compared via the shared PlatformResources accessor.	—
A (90–100)	new ChatClientProviderExtensionsTests. GetChatClientAsync_ WhenServiceProviderIsNull_ Throws	2/2 killed	Covers the new null-serviceProvider guard on the async extension.	—

Only one issue found: a compile-breaking variable-shadowing bug in ChatClientManagerTests.BuildChatClients_WhenProviderIsRegistered_InvokesFactoryAndRegistersProvider (inline suggestion posted). All other new tests are precise, assert on the correct exception type/param name/message, and use the localization-safe resource/locked-fragment comparisons already requested in prior review feedback on this PR.

_{This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with /review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 118.7 AIC · ⌖ 2.72 AIC · ⊞ 17K · [◷]( · ◷)

[github-actions]

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 118.7 AIC · ⌖ 2.72 AIC · ⊞ 17K · ◷