Validate historical hacks for test projects that may be harmful for MTP projects

[Youssef1313]

We need to revise usages of IsTestProject in SDK. For example:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

Lines 1358 to 1368 in 0b58937

	<PropertyGroup Condition="'$(IsTestProject)' == 'true' And '$(ValidateExecutableReferencesMatchSelfContained)' == ''">
	<!-- Don't generate an error if a test project references a self-contained Exe. Test projects
	use an OutputType of Exe but will usually call APIs in a referenced Exe rather than try
	to run it. -->
	<ValidateExecutableReferencesMatchSelfContained>false</ValidateExecutableReferencesMatchSelfContained>
	</PropertyGroup>
	<PropertyGroup Condition="'$(IsTestProject)' == 'true'">
	<!-- Don't generate an error if an Exe project references a test project. -->
	<ShouldBeValidatedAsExecutableReference>false</ShouldBeValidatedAsExecutableReference>
	</PropertyGroup>

With VSTest, test projects may have been special and requiring special hacks in SDK. For MTP though, it's just as any other typical console app and doesn't need any special handling, and the above may be actually hiding actual issues. In that case, we will need to make the special casing specific to VSTest via '$(IsTestProject)' == 'true' AND '$(IsTestingPlatformApplication)' != 'true'

There is also

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

Lines 114 to 121 in 0b58937

	<!-- Set the IsRidAgnostic property if this project should NOT accept global RuntimeIdentifier and SelfContained
	property values from referencing projects. -->
	<PropertyGroup Condition="'$(IsRidAgnostic)' == ''">
	<IsRidAgnostic Condition="('$(_IsExecutable)' == 'true' And '$(IsTestProject)' != 'true') Or
	'$(RuntimeIdentifier)' != '' Or
	'$(RuntimeIdentifiers)' != ''">false</IsRidAgnostic>
	<IsRidAgnostic Condition="'$(IsRidAgnostic)' == ''">true</IsRidAgnostic>
	</PropertyGroup>

and

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets

Line 98 in 0b58937

'$(_IsExecutable)' == 'true' and '$(IsTestProject)' != 'true' and

[Youssef1313]

cc @dsplaisted as you added some of these in #17594.

[dsplaisted]

Here's the background on why we added this error in the first place:

• Generate error if SelfContained Exe references framework dependent Exe, or vice versa #15117
• Fix references to Exe projects #14488 (comment)

Basically, if one Exe project references another Exe project, they will both go in the output folder. However, if one is self-contained and the other isn't, then one or both of them will fail to run.

I think with VSTest, test projects were (or could be) Exe projects, but the output wasn't run directly. With MTP projects, the output is actually run.

So, it does seem like it would make sense to re-enable this error for MTP projects. That means that if you're testing a SelfContained project, then your test project will also need to be SelfContained. But generating an error telling you the problem would be better than a weird hostfxr failure which may say it can't find the shared framework or something.

[nagilson]

Waiting for feedback to be responded to on the PR here.

[Youssef1313]

@nagilson Thanks for the ping. Sorry, so far I couldn't get to completing the PR 😕
Meanwhile, it's okay for me if someone wants to pick it up.

[Youssef1313]

Additional note:

With a simple project like:

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

Invoking dotnet build -p:SelfContained=true -bl:selfcontained.binlog will work normally and produce bin\Debug\net8.0\win-x64\ConsoleApp3.dll.

Invoking dotnet build -p:SelfContained=true -p:IsTestProject=true -bl:selfcontained.binlog will fail because no RID is set due to IsTestProject causing UseCurrentRuntimeIdentifier to not be set to true.

This is definitely very problematic for MTP where the core design is to be normal console apps. I looked a bit more into my PR today and it's not as straightforward as I originally imagined (I left details there).

NOTE that simply invoking dotnet build -p:SelfContained=true on MSTest+MTP project from template fails the same as above. The above is just a simplification of the scenario.

[Youssef1313]

FYI @bradwilson as well, as you are also setting IsTestProject here: https://github.com/xunit/xunit/blob/532a42a7787bad23936c0d23e765d2d3614e6e74/src/xunit.v3.core/Package/buildTransitive/xunit.v3.core.props#L11 which today breaks stuff.

IMO, the best way forward here is to try out microsoft/vstest#15020. Unfortunately, it's not super clear if microsoft/vstest#15020 is going to break anything or not, but I don't clearly see anything that can break. Then, if we avoided setting OutputType in VSTest package, all the special casing in .NET SDK can be safely removed. But, this might be a breaking change for SDK and can require users to move to newer Microsoft.NET.Test.Sdk packages (we may consider patching all supported versions though to reduce friction)