Make Run target and VS F5 work for test projects

[ViktorHofer]

In dotnet/runtime we are currently setting the following properties to make F5 work:

https://github.com/dotnet/runtime/blob/21e8298bfdeb833bdcd000ff2cf90a765c05a02a/eng/testing/xunit/xunit.targets#L7-L15

I feel like this shouldn't be needed to be set by individual consumers and should come via the Microsoft.Net.Test.Sdk package. Ideally the --settings switch wouldn't be required to be set: #2418.

cc @tmat @nohwnd @ManishJayaswal

[nohwnd]

What is in the runsettings that is specifically allowing this to work? We recently added the ability to add settings on project level for dotnet test #2272 remember? 🙂

The actual execution is not as simple, to run a test you need vstest.console and testhost. Test host is quite easy to bundle with the project to make it self-host, but adding vstest console is a bit more difficult, and seems unnecessary.

[ViktorHofer]

What is in the runsettings that is specifically allowing this to work?

Nothing, you can ignore the runsettings argument for the sake of this issue.

We recently added the ability to add settings on project level for dotnet test #2272 remember? 🙂

Right, in this case I was talking about invoking a test assembly from dotnet test which currently doesn't auto discover a .runsettings file in the same directory: #2418.

The actual execution is not as simple, to run a test you need vstest.console and testhost. Test host is quite easy to bundle with the project to make it self-host, but adding vstest console is a bit more difficult, and seems unnecessary.

Run and VS F5 use the RunCommand, RunArguments and RunWorkingDirectory properties to configure what is being invoked: https://github.com/dotnet/sdk/blob/beb0a88f74e641d585a31b217e04eb75bcc5eb9b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets#L706-L758

My ask is that the Microsoft.Net.Test.Sdk nuget package overwrites these properties (and I'm willing to submit a PR to make that work 😀):

<PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
    <!-- No need to overwrite RunCommand as it already points to dotnet. -->
    <RunArguments>test $(TargetPath)</RunArguments>
  </PropertyGroup>

  <PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
    <RunCommand>$(DevEnvDir)Extensions\TestPlatform\vstest.console.exe</RunCommand>
    <RunArguments>$(TargetPath)</RunArguments>
  </PropertyGroup>

• TargetPath points to the compiled assembly, ie bin\Debug\net5.0\testapp.dll. As the cwd is already the test app's outdir we could alternatively just use $(TargetName).
• DevEnvDir points to the Visual Studio's installation folder where vstest.console.exe resides in. As F5 means running inside VS, I think we can safely assume that the variable is set. Otherwise I would throw an error saying that the DevEnvDir environment variable needs to be set.

This will make F5 and /t:Run work with test apps but to successfully debug the application (meaning the module is loaded and breakpoints hit), these additional steps need to be done: https://github.com/dotnet/runtime/blob/master/docs/workflow/testing/visualstudio.md#visual-studio-f5-debugging-support. I suggest to document them officially as I only stumbled upon that info via Twitter :D

[ViktorHofer]

Submitted a PR to better to better illustrate the proposal: #2428.

[plioi]

Would this interfere with test frameworks that treat the test project as a runnable console app already? Those are naturally F5-debuggable.

[ViktorHofer]

Which ones of the supported test frameworks are runnable by default?

[plioi]

Fixie (full disclosure, this is my pet project). Letting the test project be truly a runnable console app solved a lot of the problems introduced when .NET Core arrived and AppDomains disappeared.

More to the point for the larger community, I've seen past tweets from the xUnit team that they were at least considering this model for xUnit v3 which is actively under development.

[nohwnd]

This won't be done for the current test platform, overriding more user settings is something we don't want to do now.