Inject built-in ConsoleLogger from the composition root instead of reflection-activating it - #16248
Merged
Jakub Jareš (nohwnd) merged 2 commits intoJul 9, 2026
Conversation
…flection-activating it ConsoleLogger reached for CommandLineOptions.Instance because it was reflection-activated through the logger manager and had nowhere to get the parsed options from. Give the composition root (TestRequestManager) a closed internal factory on RequestData that hands the logger manager a pre-built instance for our own built-in extensions, keyed by extension URI. ConsoleLogger now takes CommandLineOptions by injection and its parameterless constructor is gone. Third-party loggers are untouched: the factory returns null for anything it does not know and they fall through to reflection exactly as before. The seam is internal and deliberately not on IRequestData, so it never becomes an extension point. ConsoleLogger is registered in run settings by assembly-qualified name (AddConsoleLogger), not discovered through the extension manager, so the seam sits in InitializeLoggerByType keyed off the resolved type's [ExtensionUri]; the by-URI path is covered too. --ListLoggers now names loggers from discovery metadata instead of constructing them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors built-in logger activation so vstest.console can inject dependencies (notably parsed CommandLineOptions) into its own shipped loggers—specifically ConsoleLogger—instead of relying on reflection activation and process-wide singletons. It does this by adding a closed, internal “known extension instance” factory on RequestData that TestLoggerManager consults before falling back to the existing reflection-based extension activation for third-party loggers.
Changes:
- Introduces an internal
RequestData.KnownExtensionInstanceFactoryhook that allows the composition root to provide pre-configured built-in extension instances keyed by Extension URI. - Updates
TestLoggerManagerto prefer the injected instance for both URI-based initialization and assembly-qualified-name (type) initialization, with reflection fallback unchanged for unknown loggers. - Adjusts
--ListLoggersoutput to avoid instantiating loggers, and adds unit tests covering injected-instance and reflection-fallback paths.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs | Adds coverage for the injected-instance factory on both URI and AQN activation paths, plus a reflection-fallback negative control. |
| src/vstest.console/TestPlatformHelpers/TestRequestManager.cs | Wires the known-extension instance factory into the composition root’s RequestData creation and supplies an injected ConsoleLogger. |
| src/vstest.console/Processors/ListExtensionsArgumentProcessor.cs | Updates --ListLoggers to print logger type names from discovery metadata without forcing logger construction. |
| src/vstest.console/Internal/ConsoleLogger.cs | Removes the parameterless constructor and switches to constructor injection of CommandLineOptions. |
| src/Microsoft.TestPlatform.CrossPlatEngine/Client/TestLoggerManager.cs | Consults the factory before reflection activation for both URI and AQN initialization; adds InitializeKnownLogger for shared init/dedup logic. |
| src/Microsoft.TestPlatform.Common/RequestData.cs | Adds the internal KnownExtensionInstanceFactory property to support the closed injection seam. |
The regression that removing ConsoleLogger's parameterless constructor could reintroduce only shows up end-to-end on the explicit /logger:console path, which is registered in run settings by assembly-qualified name with no URI. The unit tests cover the seam but not the real activation through vstest.console. This runs a real assembly with /logger:console and asserts the summary the console logger prints, on both the net481 and netcore runners. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Jakub Jareš (nohwnd)
added a commit
that referenced
this pull request
Jul 9, 2026
…tance (#16255) After #16248 the console logger is activated from the composition root with an injected CommandLineOptions, so the ?? CommandLineOptions.Instance fallback was already dead in production. Remove it, make the field non-nullable, and default the test constructor to a fresh CommandLineOptions. The logger no longer reads the process-wide singleton, and its tests use a per-instance options object instead of resetting and poking the static one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ConsoleLogger reached for
CommandLineOptions.Instancebecause it was reflection-activated through the logger manager and had nowhere to get the parsed options from. This gives the composition root (TestRequestManager) a closed internal factory onRequestDatathat hands the logger manager a ready-made instance for our own built-in extensions, keyed by extension URI.ConsoleLoggernow takesCommandLineOptionsby constructor injection and its parameterless constructor is gone.Third-party loggers are untouched: the factory returns null for anything it doesn't recognize and they fall through to reflection exactly as before. The seam is
internaland deliberately not onIRequestData, so it never turns into an extension point. That is the point of the shape here — our own extensions can be injected, and.Instancestays only behind the boundary for other people's code.One thing worth knowing while reading: ConsoleLogger isn't discovered through the logger extension manager.
AddConsoleLogger/UpdateConsoleLoggerIfExistswrite it into run settings by assembly-qualified name, so it's always activated inInitializeLoggerByType. That's where the seam had to go, keyed off the[ExtensionUri]on the resolved type. The by-URI path is covered too, so both entry points behave the same and a logger the user names by friendly name or URI is serviced identically.--ListLoggersnow names loggers from their discovery metadata instead of constructing them, since it can no longernewup ConsoleLogger.Tests: added URI-path, AQN-path, design-mode + friendly-name, and a reflection-fallback negative control to
TestLoggerManagerTests. Ran the full class (65/65 on net11.0 + net481) and the smoke suite (dotnet test --logger:Console, Acceptance 5/5 + Library 4/4) green. The AQN-path test is the one that guards the regression removing the parameterless constructor would otherwise reintroduce.