Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to use a different DOTNET host than the one located in the path #1749

Closed
ViktorHofer opened this issue Aug 23, 2018 · 16 comments · Fixed by #2359
Closed

Add option to use a different DOTNET host than the one located in the path #1749

ViktorHofer opened this issue Aug 23, 2018 · 16 comments · Fixed by #2359
Assignees
Milestone

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 23, 2018

Follow-up on the offline discussion that we had. We (the .NET team) want to enable VS Text Explorer support for corefx.

We are building the shared framework and the runtime ourselves, we do things differently as the average user. Still we see a strong demand for enabling Test Explorer like in a regular app.

  • Framework
    We (corefx) are the ones that build the framework itself. Therefore we can’t use the shared framework located under %PATH%/dotnet/shared/Microsoft.NETCore.App/x.x.x/ as we want to test our live build.
  • Host
    We don’t take a dependency on the globally installed .NET Core SDK and its dotnet.exe host and use nightly builds instead. That’s because we rely on features that aren’t yet available and because we don’t need that dependency.

We need a way to tell the VS Text Framework (Test Explorer) to use our dotnet host under corefx instead of the global one in the path. Probably via a switch in the .runsettings file that allows to set the custom path.

How are we running the tests? do we use vstest.console or dotnet test verb ?

On the command-line we run tests in a different way by directly invoking xunit.console.dll but with Visual Studio we just want to use the Test Explorer which I'm not sure what it uses behind the scenes. For xunit we are using its test adapter: https://www.nuget.org/packages/xunit.runner.visualstudio/

Can’t we temporarily set the environment variable “PATH” to point to the new path in a script before invoking the test command ?

Can we do that when we just use VS Test Explorer?

cc @singhsarab @danmosemsft

@singhsarab
Copy link
Contributor

singhsarab commented Aug 29, 2018

@ViktorHofer

Can we do that when we just use VS Test Explorer?

Have you tried setting the environment variable before invoking the devenv in cmd/shell.

@singhsarab
Copy link
Contributor

@ViktorHofer Gentle ping.

@ViktorHofer
Copy link
Member Author

Sorry for the late response, I was OOF for a few days.

Have you tried setting the environment variable before invoking the devenv in cmd/shell.

Sure that's an option but we want to use a different host for testing than for building (in corefx we currently use a minimal dotnet host which is only used during testing). Also for an optimal inner loop dev scenario we want to avoid setting local environment variables and launching VS from bash.

@singhsarab
Copy link
Contributor

@ViktorHofer
Any reason, why we avoid setting local environment variables and launching VS from bash?

One basic principle we follow is to avoid adding new tags in Runsettings unless absolutely necessary. Reason being, adding is easy, removing is difficult. This ask is very specific to this particular scenario. And given there is already a workaround for this, I am not able to make a case.

Another concern I have is if we add something on lines you suggest, we'd have to document it. Even a normal user will also be able to set the path to whatever location he/she wants and that might result in unexpected behavior.

/cc: @PBoraMSFT @cltshivash

@ViktorHofer
Copy link
Member Author

Any reason, why we avoid setting local environment variables and launching VS from bash?

Yes two reasons:

  • We use a different dotnet host for testing than for building
  • We want to provide a dev flow that doesn't require setting environment variables and launching VS from bash.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Sep 20, 2018

So we discussed that topic internally and it seems in fact we don't need to tell the test framework to use a different host but to use the same as the project-system does. We pass the host via the StartProgram/RunCommand (launchSettings.json) same as the usual customer does.

If I'm right the test-framework currently determines the host in the specific order (https://devdiv.visualstudio.com/DevDiv/_git/DotNet-Source-Build-Tarball?path=%2Fsrc%2Fvstest%2Fsrc%2FMicrosoft.TestPlatform.TestHostProvider%2FHosting%2FDotnetTestHostManager.cs&version=GBmaster&line=172&lineEnd=183&lineStartColumn=13&lineEndColumn=14&lineStyle=plain):

  1. if the current process is dotnet.exe use it
  2. otherwise look into the path

I spoke with @davkean yesterday who agrees that the test-framework should just ask the project-system for it. Ideally it should also use the environment variables that are passed in from the launchSettings.json.

cc @eerhardt

@singhsarab
Copy link
Contributor

@ViktorHofer Thank you for note.
This seems like a better approach to me. Can you please help me get a better understanding of these environment variables passed in via launchSettings.json ?
If you think it is a small change in the code you pointed out, feel free to contribute. :)

@ViktorHofer
Copy link
Member Author

@davkean can you please assist here? How can the test framework ask the project system for the right executable and other stuff defined in the launchSettings.json / Run props?

@jayaranigarg
Copy link
Member

@ViktorHofer : Any updates here?

@ViktorHofer
Copy link
Member Author

I didn't yet get a response from Dave, I also contacted him offline. Let me reach out to him again as I don't know how to follow-up here.

@jayaranigarg
Copy link
Member

@ViktorHofer : Did you get a chance to follow up with Dave?

@ViktorHofer
Copy link
Member Author

Not yet. Unfortunately I couldn't find the right person who could help here. Can you assist here?

@danmoseley
Copy link
Member

@davkean?

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Feb 18, 2020

@ManishJayaswal @nohwnd I want to resume the discussion for this ask. To make the ask more specific, we need the following feature:

Allow specifying the path to the .NET Host dotnet.exe that is used for invoking the VSTest testhost. This should be an additional setting in the .runsettings file. Together with #2273, we will then be able to fully switch to VSTest in https://github.com/dotnet/runtime and use the dotnet test verb on CLI.

For .NET Framework we don't need an additional hook as we can add the DEVPATH environment variable (which allows to load from a different .NET Framework directory) into the .runsetting's environment variable section.

Proposed .runsettings change:

<?xml version="1.0" encoding="utf-8"?>
<RunSettings>
  <RunConfiguration>
    <DotNetHostPath>%absolute-path-to-muxer%\dotnet.exe</DotNetHostPath>
  </RunConfiguration>
</RunSettings>

Relevant code pieces to enable this feature:

public virtual TestProcessStartInfo GetTestHostProcessStartInfo(
IEnumerable<string> sources,
IDictionary<string, string> environmentVariables,
TestRunnerConnectionInfo connectionInfo)
{
var startInfo = new TestProcessStartInfo();
// .NET core host manager is not a shared host. It will expect a single test source to be provided.
var args = string.Empty;
var sourcePath = sources.Single();
var sourceFile = Path.GetFileNameWithoutExtension(sourcePath);
var sourceDirectory = Path.GetDirectoryName(sourcePath);
// Probe for runtimeconfig and deps file for the test source
var runtimeConfigPath = Path.Combine(sourceDirectory, string.Concat(sourceFile, ".runtimeconfig.json"));
if (this.fileHelper.Exists(runtimeConfigPath))
{
string argsToAdd = " --runtimeconfig " + runtimeConfigPath.AddDoubleQuote();
args += argsToAdd;
EqtTrace.Verbose("DotnetTestHostmanager: Adding {0} in args", argsToAdd);
}
else
{
EqtTrace.Verbose("DotnetTestHostmanager: File {0}, doesnot exist", runtimeConfigPath);
}
// Use the deps.json for test source
var depsFilePath = Path.Combine(sourceDirectory, string.Concat(sourceFile, ".deps.json"));
if (this.fileHelper.Exists(depsFilePath))
{
string argsToAdd = " --depsfile " + depsFilePath.AddDoubleQuote();
args += argsToAdd;
EqtTrace.Verbose("DotnetTestHostmanager: Adding {0} in args", argsToAdd);
}
else
{
EqtTrace.Verbose("DotnetTestHostmanager: File {0}, doesnot exist", depsFilePath);
}
var runtimeConfigDevPath = Path.Combine(sourceDirectory, string.Concat(sourceFile, ".runtimeconfig.dev.json"));
string testHostPath = string.Empty;
// If testhost.exe is available use it
bool testHostExeFound = false;
if (this.platformEnvironment.OperatingSystem.Equals(PlatformOperatingSystem.Windows))
{
var exeName = this.architecture == Architecture.X86 ? "testhost.x86.exe" : "testhost.exe";
var fullExePath = Path.Combine(sourceDirectory, exeName);
// check for testhost.exe in sourceDirectory. If not found, check in nuget folder.
if (this.fileHelper.Exists(fullExePath))
{
EqtTrace.Verbose("DotnetTestHostManager: Testhost.exe/testhost.x86.exe found at path: " + fullExePath);
startInfo.FileName = fullExePath;
testHostExeFound = true;
}
else
{
// Check if testhost.dll is found in nuget folder.
testHostPath = this.GetTestHostPath(runtimeConfigDevPath, depsFilePath, sourceDirectory);
if (testHostPath.IndexOf("microsoft.testplatform.testhost", StringComparison.OrdinalIgnoreCase) >= 0)
{
// testhost.dll is present in path {testHostNugetRoot}\lib\netcoreapp2.1\testhost.dll
// testhost.(x86).exe is present in location {testHostNugetRoot}\build\netcoreapp2.1\{x86/x64}\{testhost.x86.exe/testhost.exe}
var folderName = this.architecture == Architecture.X86 ? "x86" : "x64";
var testHostNugetRoot = new DirectoryInfo(testHostPath).Parent.Parent.Parent;
var testHostExeNugetPath = Path.Combine(testHostNugetRoot.FullName, "build", "netcoreapp2.1", folderName, exeName);
if (this.fileHelper.Exists(testHostExeNugetPath))
{
EqtTrace.Verbose("DotnetTestHostManager: Testhost.exe/testhost.x86.exe found at path: " + testHostExeNugetPath);
startInfo.FileName = testHostExeNugetPath;
testHostExeFound = true;
}
}
}
}
if (!testHostExeFound)
{
var currentProcessPath = this.processHelper.GetCurrentProcessFileName();
if (string.IsNullOrEmpty(testHostPath))
{
testHostPath = this.GetTestHostPath(runtimeConfigDevPath, depsFilePath, sourceDirectory);
}
// This host manager can create process start info for dotnet core targets only.
// If already running with the dotnet executable, use it; otherwise pick up the dotnet available on path.
// Wrap the paths with quotes in case dotnet executable is installed on a path with whitespace.
if (currentProcessPath.EndsWith("dotnet", StringComparison.OrdinalIgnoreCase)
|| currentProcessPath.EndsWith("dotnet.exe", StringComparison.OrdinalIgnoreCase))
{
startInfo.FileName = currentProcessPath;
}
else
{
startInfo.FileName = this.dotnetHostHelper.GetDotnetPath();
}
EqtTrace.Verbose("DotnetTestHostmanager: Full path of testhost.dll is {0}", testHostPath);
args = "exec" + args;
args += " " + testHostPath.AddDoubleQuote();
}
EqtTrace.Verbose("DotnetTestHostmanager: Full path of host exe is {0}", startInfo.FileName);
args += " " + connectionInfo.ToCommandLineOptions();
// Create a additional probing path args with Nuget.Client
// args += "--additionalprobingpath xxx"
// TODO this may be required in ASP.net, requires validation
// Sample command line for the spawned test host
// "D:\dd\gh\Microsoft\vstest\tools\dotnet\dotnet.exe" exec
// --runtimeconfig G:\tmp\netcore-test\bin\Debug\netcoreapp1.0\netcore-test.runtimeconfig.json
// --depsfile G:\tmp\netcore-test\bin\Debug\netcoreapp1.0\netcore-test.deps.json
// --additionalprobingpath C:\Users\username\.nuget\packages\
// G:\nuget-package-path\microsoft.testplatform.testhost\version\**\testhost.dll
// G:\tmp\netcore-test\bin\Debug\netcoreapp1.0\netcore-test.dll
startInfo.Arguments = args;
startInfo.EnvironmentVariables = environmentVariables ?? new Dictionary<string, string>();
startInfo.WorkingDirectory = sourceDirectory;
return startInfo;
}

@eerhardt
Copy link
Member

Small question on the naming: Do we use the term Muxer anywhere else in public settings/APIs/docs? I don't think we should use that term publicly. What about just TestHost or TestHostPath? Or if that is confusing with the testhost.exe binary, then maybe DotNetHostPath.

@ViktorHofer
Copy link
Member Author

You are right, DotNetHostPath is a much better term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants