[fix] Warn when testhost is not found in project deps.json (missing Microsoft.NET.Test.Sdk)#16077
[fix] Warn when testhost is not found in project deps.json (missing Microsoft.NET.Test.Sdk)#16077nohwnd wants to merge 2 commits into
Conversation
…NET.Test.Sdk) When a .NET test project has a deps.json and runtimeconfig.dev.json (indicating a proper managed project) but the testhost package is not listed in its deps.json, vstest silently falls back to the testhost from the test runner installation. This fallback can cause confusing runtime errors about assembly version mismatches in testhost.deps.json when the versions are incompatible. This change emits a user-visible warning in that scenario, helping users diagnose the root cause: a missing reference to the 'Microsoft.NET.Test.Sdk' NuGet package. Fixes #15403 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostics in the .NET test host resolution path by warning when a managed test project appears to be missing the Microsoft.NET.Test.Sdk dependency before falling back to the runner-side testhost.dll.
Changes:
- Stores the initialized
IMessageLoggerinDotnetTestHostManager. - Emits a warning when
.deps.jsonand.runtimeconfig.dev.jsonexist but no project-local testhost is resolved. - Adds the new warning resource to
.resx, generated designer code, and all locale.xlffiles.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs |
Adds logger storage and warning emission before fallback testhost resolution. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/Resources.resx |
Adds the new MissingTestSdkWarning resource string. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/Resources.Designer.cs |
Adds the strongly typed accessor for the new resource. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.cs.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.de.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.es.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.fr.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.it.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.ja.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.ko.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.pl.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.pt-BR.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.ru.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.tr.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.zh-Hans.xlf |
Adds the new localization unit. |
src/Microsoft.TestPlatform.TestHostProvider/Resources/xlf/Resources.zh-Hant.xlf |
Adds the new localization unit. |
Files not reviewed (1)
- src/Microsoft.TestPlatform.TestHostProvider/Resources/Resources.Designer.cs: Language not supported
| if (_fileHelper.Exists(depsFilePath) && _fileHelper.Exists(runtimeConfigDevPath)) | ||
| { | ||
| string message = string.Format(CultureInfo.CurrentCulture, Resources.MissingTestSdkWarning, sourcePath); | ||
| EqtTrace.Warning("DotnetTestHostManager.GetTestHostStartInfo: {0}", message); | ||
| _messageLogger?.SendMessage(TestMessageLevel.Warning, message); |
| if (_fileHelper.Exists(depsFilePath) && _fileHelper.Exists(runtimeConfigDevPath)) | ||
| { | ||
| string message = string.Format(CultureInfo.CurrentCulture, Resources.MissingTestSdkWarning, sourcePath); | ||
| EqtTrace.Warning("DotnetTestHostManager.GetTestHostStartInfo: {0}", message); |
nohwnd
left a comment
There was a problem hiding this comment.
Review Summary
Dimensions activated: Process Architecture & Host Resolution · Testhost Assembly Loading & Resolution · Error Reporting & Diagnostic Clarity
What was checked
Testhost Assembly Loading & Resolution — The warning condition (depsFilePath exists AND runtimeConfigDevPath exists AND GetTestHostPath returned empty) is a sound heuristic. runtimeconfig.dev.json only appears in development builds of SDK-style projects, so its presence is a reliable signal that Microsoft.NET.Test.Sdk should be referenced. The fallback path logic is unchanged and correct.
Process Architecture & Host Resolution — Flow analysis confirms the warning fires only when both GetTestHostPath calls return empty and we're about to fall back to the runner-side testhost. On Windows, if testHostPath was set from the nuget-based lookup at line 338 (even without a testHostExeFound), the warning is correctly suppressed at line 375 since testHostPath.IsNullOrEmpty() is false.
Error Reporting & Diagnostic Clarity — The user-visible message is accurate and actionable. One minor issue noted inline: the EqtTrace.Warning call uses GetTestHostStartInfo rather than the actual method name GetTestHostProcessStartInfo.
_messageLogger null safety — The ?. guard is appropriate; Initialize() can be called with a null logger and this is handled safely.
No public API changes — _messageLogger is private; Resources.MissingTestSdkWarning is internal. No PublicAPI.Unshipped.txt update needed.
Localization — All 12 .xlf files correctly updated with state="new" per convention.
One minor inline finding (incorrect trace method name). No blocking issues.
🧠 Reviewed by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
| if (_fileHelper.Exists(depsFilePath) && _fileHelper.Exists(runtimeConfigDevPath)) | ||
| { | ||
| string message = string.Format(CultureInfo.CurrentCulture, Resources.MissingTestSdkWarning, sourcePath); | ||
| EqtTrace.Warning("DotnetTestHostManager.GetTestHostStartInfo: {0}", message); |
There was a problem hiding this comment.
[Error Reporting & Diagnostic Clarity] The EqtTrace.Warning call uses the method name GetTestHostStartInfo, but the actual method is GetTestHostProcessStartInfo. This won't affect user-visible output, but it makes log correlation harder when debugging.
// Current (incorrect method name)
EqtTrace.Warning("DotnetTestHostManager.GetTestHostStartInfo: {0}", message);
// Should be:
EqtTrace.Warning("DotnetTestHostManager.GetTestHostProcessStartInfo: {0}", message);There was a problem hiding this comment.
Fixed — updated the EqtTrace.Warning call to use GetTestHostProcessStartInfo as the method name.
🔧 Iterated by PR Iteration Agent 🔧
…rtInfo Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
Fixes #15403
Root Cause
When a .NET test project has a proper
deps.jsonandruntimeconfig.dev.json(indicating a managed .NET project), but doesn't referencemicrosoft.testplatform.testhostin its deps.json,DotnetTestHostManagersilently falls back to using the testhost from the vstest.console installation directory. This fallback can succeed at process launch but then fail at runtime with a confusing error like:The root cause is typically a missing reference to
Microsoft.NET.Test.Sdkin the test project, but the error message gives no indication of this.Fix
Added a user-visible
Warningmessage emitted byDotnetTestHostManager.GetTestHostStartInfowhen:deps.jsonfile (managed project), ANDruntimeconfig.dev.jsonfile (proper SDK-style .NET project), ANDtesthost.dllis not found in the project's deps.json (i.e.,Microsoft.NET.Test.Sdkis likely missing)The warning reads:
This warning is shown before the fallback occurs, giving the user actionable information to diagnose the real cause of any subsequent failure.
Changes
DotnetTestHostManager: Added_messageLoggerfield to store the logger passed toInitialize(), and emit a warning when falling back to runner-side testhost with evidence of a managed project that lacksMicrosoft.NET.Test.Sdk.Resources.resx+Resources.Designer.cs: AddedMissingTestSdkWarningresource string..xlffiles updated with the new resource string.