Tests' build output for netcoreapp3+ mixes the system under test with VsTest implementation details.

[plioi]

Description

.NET Core 3 changed the default behavior of a project's build output folder (see "Build copies dependencies"). For test projects, this has the unfortunate side effect of polluting the test projects' build output folder with test-running infrastructure. In addition to being visually jarring (which is not important), this changes the system under test. The system under test is no longer the one the user intended:

Context in this image:

1. CustomConvention.Tests is a test project.
2. Fixie.TestAdapter is the adapter for the Fixie test framework.
3. Fixie.Integration is the intended system under test.
4. Mono.* is a dependency of the TestAdapter.
5. Shouldly is merely an assertion library referenced by the test project.
6. Everything else is the result of referencing the VsTest testhost infrastructure.

For instance, the popular Newtonsoft.Json.dll used by the VsTest infrastructure gets copied out here beside the system under test, which could negatively impact behavior if the system under test has its own reference to some other version of the assembly. This is just one example. Even if the user's desired version of Newtonsoft.Json happens to be the one present, the underlying problem remains: we're not testing what we think we are testing. The build output folder is a mix of the system under test, the test adapter's own dependencies, and VsTest testhost infrastructure.

There's hope that this mixing of multiple systems isn't truly necessary:

1. We see that under a netcoreapp2.1 build, VsTest was able to perform discovery and execution without needing to mix multiple systems into one folder. I presume it was able to use the "deps" files to locate the particular test adapter/etc under the nuget cache, which makes sense, and run it from there.
2. When debugging this custom TestAdapter, even under the .NET Core 3 target, the VS "Modules" screen shows that the actual testhost.dll loaded at runtime is the one under the nuget cache. So, it seems the VsTest files in our build output folder isn't strictly necessary for the actual runtime behavior, but that it's just an unfortunate side effect of .NET Core 3's change in build output behavior.

So, there are two questions:

1. Is there anything consumers can do right now that would prevent the pollution of the build output folder while still giving VsTest the chance to locate the right testhost under the nuget cache folder? Any way to provoke a test project to output "deps" similar to a .NET Core 2 build? Do we somehow have control with the "additional probing paths" concept?
2. Can VsTest or the test SDK nuget package itself be altered to avoid polluting the build output folder by default? I understand that recent optimizations forced *TestAdapter.dll to be present in the build output folder so that it can be detected, but could the VsTest-specific assets be suppressed at least?

It appears that the Test SDK's use of NuGet package dependencies essentially overwrites the system under test, and might be the root cause.

Steps to reproduce

Create a solution with a single test project which targets both netcoreapp2.1 and netcoreapp3.1. Build. Inspect the build output folders for both target frameworks.

Expected behavior

Build output folder contains the system under test.

Actual behavior

Build output folder contains a mix of the system under test, the test adapter's own dependencies, and the VsTest infrastructure.

[nohwnd]

@plioi I feel your pain, but could not find any option to disable the new behavior, I am trying to find someone who knows how to disable this, or to confirm that there is not an option to disable this.

[plioi]

@nohwnd Thanks. Understandable if there's no near-term workaround. My gut says that there would need to be a larger design change in VSTest itself to avoid the need to touch the build output folder, like with .NET Framework builds. It's a little hard to recommend that folks use Test Explorer and the test SDK package, knowing that it alters the system under test.

[ben3441]

Hi, I've been hit by this issue. My scenario is that I've got a console application originally targetting .net framework but now upgraded to netcoreapp3.1 - the console application is using Newtonsoft.Json at version 12.0.3. I also have a separate test solution running integration tests on the application by launching it from the executable - so no project reference from the test project to the project under test. Both solutions build to the same directory.

The application is fine when built and run manually, however if I then build and run the integration tests they fail as during building the test project the Newtonsoft.Json.dll has been replaced with the version used by Microsoft.NET.Test.Sdk.

I have worked around this for now by adding a nuget reference to Newtonsoft.Json at the version used by my application in the tests but this has cost me a few hours of digging and I don't think my workflow is too rare.

[plioi]

Some of the implementation details in VsTest, as it prepares to ultimately invoke the test host, makes it look like the primary intended path is for the bin/ folder to NOT contain all these extra assemblies and support files, and that the mode everyone is experiencing was supposed to just be a fallback case.

[IanKemp]

Microsoft, this is a critical bug in your product! Why has it not been addressed in over six months?

[IanKemp]

@plioi Would be helpful if you could maybe post a few notes on what you've found so far, to help anyone who might want to dig into this further (because apparently Microsoft doesn't care).

[plioi]

I'm uncertain what the answer should be, but things that come to mind:

Evidence that this was unintended is that DotnetTestHostManager.cs is written to treat the current behavior as a fallback edge case. The apparent primary case it was written for was the desirable one where the build output folder is not filled with a copy of VsTest overwriting your own dependencies, but instead would find the testhost under the nuget cache and run from there. See especially the comment here: https://github.com/microsoft/vstest/blob/master/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs#L516

            // If we are here it means it couldn't resolve testhost.dll from nuget cache.
            // Try resolving testhost from output directory of test project. This is required if user has published the test project
            // and is running tests in an isolated machine. A second scenario is self test: test platform unit tests take a project
            // dependency on testhost (instead of nuget dependency), this drops testhost to output path.

It seems that the intended-edge case has become the only case. All the work above this comment to traverse the deps file is probably no longer taking action in practice.

I suspect that the fact that .NET core test projects get marked as an EXE with an injected empty Main() method has the unintended side effect of dumping all the references into the build output folder, even these development-only dependencies. That didn't happen in the .NET Framework case, since a library test project wasn't ambiguous. Since a netstandard test project would be ambiguous, the test SDK marks it as a console exe in fact. So we get a build output folder using those semantics? To be clear though, as a test framework author myself, having test projects be in-fact console exe's is incredibly valuable.

So that makes me wonder whether the resolution can come from taking further control over the way the test SDK's referenced libraries are treated. A concept I've always struggled with, but which might be a part of the answer, is things like the <PrivateAssets>/<IncludeAssets> aspect of <PackageReference>, nuget's concept of a developmentDependency, etc. They seem to be levers one pulls when they need finer grained control over the build output and when distinguishing an asset that is needed by the project being built at runtime vs just at dev time, but every attempt I've made to work with these options has hit a brick wall. The documentation around these concepts could only mean something to someone who already knew all the details.

[nohwnd]

Unfortunately, this does not have an easy solution. 

Whether or not we copy the dlls into the output folder does not make a huge difference here.

User code will end up running in testhost.exe into which we load it, together with all the necessary dependencies, including Newtonsoft.Json. There is no AssemblyLoadContext isolation, so you will end up together in the process with Newtonsof.Json types, even if you load them from a nuget folder. 

Test platform keeps it's external dependencies to a minimum, there is Newtonsoft.Json, and NuGet.Frameworks. Newtonsoft.Json is copied into user folder, and previously the version used was very old (9.x.x), so the user can upgrade it when needed. This upgrade happens automatically by NuGet package version resolve during, so if user is choosing a newer version (e.g. 13.x.x) it will be used. 

Since then there were many security updates to Newtonsoft.Json and we pushed the version up to the latest (13.x.x), and so should your project. But you are right, if user depends on older version of Newtonsoft.Json we will upgrade them, and influence their main app. This is unfortunately a limitation of the current solution, and we cannot really fix without rolling our own json infrastructure, which is not something we want to commit to just now.

[microsoft-github-policy-service]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 10 days.

[IanKemp]

NOT STALE.

[microsoft-github-policy-service]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 10 days.

[microsoft-github-policy-service]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 10 days.