SDK 2.1.500 fails to set IsTestProject in net40 projects

[livarcocc]

From @jnm2 on November 22, 2018 16:3

Here's a full repro: DotnetTestBug.zip

Projects like this worked in all SDKs up to 2.1.500 because Microsoft.NET.Test.Sdk sets <IsTestProject>true</IsTestProject>:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <!-- This works in all SDK versions until 2.1.500: -->
    <TargetFramework>net40</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="NUnit" Version="3.11.0" />
    <PackageReference Include="NUnit3TestAdapter" Version="3.11.0" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
  </ItemGroup>

</Project>

E.g. using SDK 2.1.403, you get this output for dotnet test:

Test run for C:\Users\Joseph\Desktop\DotnetTestBug\Using SDK 2.1.403\bin\Debug\net40\TestProject1.dll(.NETFramework,Version=v4.0)
Microsoft (R) Test Execution Command Line Tool Version 15.8.0
...
Test Run Successful.

Starting with SDK 2.1.500, <IsTestProject>true</IsTestProject> is no longer set. dotnet test gives:

Skipping running test for project C:\Users\Joseph\Desktop\DotnetTestBug\Using SDK 2.1.500\TestProject1.csproj. To run tests with dotnet test add "\<IsTestProject>true\<IsTestProject>" property to project file.

Note that the same <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" /> is used in both cases.
Wasn't sure if this belonged here or at https://github.com/Microsoft/vstest/issues.

Issues

• Is it your intention to require test projects which include a net40 target to have to manually include <IsTestProject>true</IsTestProject> directly in the test csproj? Why isn't this the case for net45, since VSTest 15.9 supports net40 and net45 equally?

• To run tests with dotnet test add "<IsTestProject>true<IsTestProject>"

  This is invalid XML. Second tag should be a closing tag: <IsTestProject>true</IsTestProject>

Copied from original issue: dotnet/sdk#2690

[singhsarab]

@livarcocc Seems like a problem with build and the way nuget is resolving the targets for net40 framework.
Building a project targeting net40, I obeserved that the props (Microsoft.Net.Test.Sdk) file is not getting honored.

[livarcocc]

Do you have a binlog to see why they are not being honored? And what do you mean by not being honored? Are they not being set? Are they being overwritten?

[singhsarab]

@livarcocc

Attached are the diagnostic logs for the build.
IsTestProject is being set to true for net451 project but not for net40 project.
diag_net40.txt
diag_net451.txt

[singhsarab]

@livarcocc Gentle ping.

[ViktorHofer]

Is it your intention to require test projects which include a net40 target to have to manually include true directly in the test csproj? Why isn't this the case for net45, since VSTest 15.9 supports net40 and net45 equally?

Inspecting https://www.nuget.org/packages/Microsoft.NET.Test.Sdk/15.9.0 I can only see a tfm for net45 but none for net40. Why do you believe that VSTest supports net40?

[jnm2]

@ViktorHofer Because VSTest 15.9 supports /Framework:Framework40 and is the first version not to support 3.5:

Framework35 is not supported. For projects targeting .Net Framework 3.5, please use Framework40 to run tests in CLR 4.0 "compatibility mode".

[ViktorHofer]

Ok makes sense, but the nuget package doesn't have a tfm for it. Intentional?

[jnm2]

@ViktorHofer I guess that's my question.

[singhsarab]

@jnm2 While we identify and fix this issue, I hope adding the tag in the csproj works for you.

@ViktorHofer There was no need for a separate net40 folder, as net45 used to suffice.
With changes to dotnet test to actually check for IsTestProject property before running the tests, we identified that props(which adds the IsTestProject property) wasn't getting honored for net40 projects.
I have shared the log outputs in my previous comment, in case you want to take a look.

Will creating another folder net40 resolve this? Is that the guidance ?

[ViktorHofer]

Instead of adding an additional net40 folder I would just downgrade the net45 one to net40. With that both the props and targets files will be honored. I can send a PR for that if you agree.

[singhsarab]

Sounds fine to me.
/cc: @mayankbansal018

[mayankbansal018]

Downgrading to net20 if fine, as long as the props file gets picked up.

[ViktorHofer]

Why? Net40 is the minimum supported version.

[singhsarab]

@ViktorHofer That was just a sarcastic remark I assume, please go ahead make the change. :)

[mayankbansal018]

@ViktorHofer, @singhsarab , I didn't mean it to be sarcastic. I just misread your earlier comment & thought you were moving it to net20, because when we tried it locally net40 did not work for us. Please go ahead and make your change.