Report test extension load failures to the user - #16382
Report test extension load failures to the user#16382Jakub Jareš (nohwnd) wants to merge 2 commits into
Conversation
TestPluginDiscoverer only told the user about a failing extension file when Assembly.Load threw FileLoadException. The general Exception handler, which is the one that catches the FileNotFoundException thrown when an extension or one of its dependencies is missing, and the ReflectionTypeLoadException handler for an assembly that loads but whose types do not, both wrote to EqtTrace only. A user whose adapter half-loaded got fewer tests than expected, or a hang, and could only find out why by re-running with /diag. Both now report through TestSessionMessageLogger with the existing, already localised FailedToLoadAdapaterFile message. Scanning stays best effort: nothing throws, nothing aborts, and a partially loaded assembly is still scanned for every extension type. Each file is reported once per run, and the two C++ UWP adapters that are probed speculatively when no extension was found are not reported, since they are absent everywhere except UWP. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves user-facing diagnostics when test extension (adapter/logger/etc.) discovery fails, ensuring failures that previously only surfaced in /diag logs are reported as warnings while keeping extension scanning best-effort.
Changes:
- Report extension load failures via
TestSessionMessageLoggerfor general load failures andReflectionTypeLoadExceptionpartial-type-load scenarios. - Deduplicate user-visible warnings so the same failing extension file is not repeatedly reported across scans.
- Add/extend unit tests to validate warning emission, deduplication, and continued scanning behavior on partial type-load failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginDiscoverer.cs | Adds user-visible warning reporting for additional extension load failure paths and introduces per-file warning deduplication. |
| test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/TestPluginDiscovererTests.cs | Adds tests that capture TestSessionMessageLogger warnings for missing/partially-loaded extensions and verifies dedupe behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The set that stops a failing extension being reported once per scan was static and never emptied, so in a design mode process that lives for hours it reported once per process, not once per run, and only grew. Clear it from TestPluginCache.ClearExtensions, which the runner already calls before every discovery and run request, and compare paths ignoring case so two spellings of one file on Windows do not warn twice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
This does the same thing as #16390 and both are green and mergeable, so whichever goes in first makes the other conflict. I read both diffs and moved the one piece that only exists here over to #16390, so this one can be closed without losing anything. Where #16390 is better:
Where this PR was right and #16390 was not: The warning in #16390 was deduplicated on I settled that with a test rather than by reading the code. Same test source, run on both branches, Release, net11.0 and net481: scan a file that cannot be loaded, assert the warning, call
So the reset here was fixing something real, and it is now in #16390. It keeps that PR's structure and separates the two concerns instead of coupling them: Tests carried over from here into #16390:
Dropped: the two tests here that assert a warning on a partial type load. Under #16390 that case is deliberately quiet, so the assertion is inverted rather than ported.
🤖 |
TestPluginDiscovereronly told the user about a failing extension file whenAssembly.LoadthrewFileLoadException. Two other paths were silent and wrote toEqtTraceonly:Exceptionhandler inGetTestExtensionsFromFiles, which is the one that catches theFileNotFoundExceptionthrown when an extension or one of its dependencies cannot be found;ReflectionTypeLoadExceptionhandler inGetTestExtensionsFromAssembly, which carries on with a half-loaded type list.So an adapter that half-loaded gave the user fewer tests than expected, or a hang, with no way to find out why short of re-running with
/diag. Both now report throughTestSessionMessageLoggerusing the existing, already localisedFailedToLoadAdapaterFilemessage, so no.resxor.xlfchange is needed.Scanning stays best effort. Nothing throws and nothing aborts, a partially loaded assembly is still scanned for every extension type, each file is reported once per run, and the two C++ UWP adapters that are probed speculatively when no extension was found are not reported because they are absent everywhere except UWP.
With an adapter whose dependency was deleted, the user now sees:
and the run still discovers every test.
This is the diagnostic half of #15577. The packaging cause behind the report on that issue, the missing net462 companion assemblies, was fixed separately in #15739 and shipped in 18.9.0, so this does not close #15577 on its own.
Verified:
build.cmd -c Releasecompiles clean,Microsoft.TestPlatform.Common.UnitTestspasses on net481 and net11.0 (401 tests), and a locally builtvstest.consolereproduces both failure modes end to end.Related to #15577
🤖