microsoft / vstest

Visual Studio Test Platform is the runner and engine that powers test explorer and vstest.console.
MIT License
870 stars 316 forks source link

InProcDataCollection not loading tested assemblies #3810

Closed richardwerkman closed 1 year ago

richardwerkman commented 2 years ago

This issue is more of a question than a bug. I didn't see github discussions here so I created this issue. Please sent me to the right place if this isn't the place to ask questions.

What should the order of assemblies loaded be when using InProcDataCollection? I'd expect the tested assembly would be loaded once the tests are being run. But this is not always the case for me.

Steps to reproduce

I'll add if needed

Expected behavior

InProcDataCollector loads tested assemblies before test is started so they can be requested using: AppDomain.CurrentDomain.GetAssemblies() or AppDomain.CurrentDomain.AssemblyLoad += OnAssemblyLoaded; And the assembly can be used to call a class inside the tested assembly

Actual behavior

The tested assembly is not loaded and also hasn't loaded when the first testcase is finished.

This breaks my app.

Diagnostic logs

Please share test platform diagnostics logs. Instructions to collect logs are here.
The logs may contain test assembly paths, kindly review and mask those before sharing.

Environment

I'm running on windows 10 using vstest.console version 17.2.0

MarcoRossignoli commented 2 years ago

The tested assembly is not loaded and also hasn't loaded when the first testcase is finished.

Do you mean that if you subscribe to TestCaseEnd and do a foreach (Assembly assembly in AppDomain.CurrentDomain.GetAssemblies()) you don't see the test assembly loaded?

Can you provide a repro?

In coverlet we're using end session event to flush hits and we're able to find all tested assembly, I expect to have in the same appdomain also the test assembly, it's not possible unload single dll in .NET Framework(it's possible on .NET Core with AssemblyLoadContext) https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.collector/InProcDataCollection/CoverletInProcDataCollector.cs#L56

richardwerkman commented 2 years ago

Yes that's exactly the behaviour I'm seeing! I spotted it while working on a new feature for Stryker. You can check out the branch and see the repro there.

  1. Clone the Stryker repo and check out this branch: https://github.com/stryker-mutator/stryker-net/tree/vstest_assembly_load_repro

  2. Add a launchSettings.json to the Stryker.CLI project containing:

    {
    "profiles": {
    "Integrationtest solution": {
      "commandName": "Project",
      "commandLineArgs": "-s C:\\Dev\\Repos\\stryker-net\\integrationtest\\TargetProjects\\IntegrationTestApp.sln",
      "workingDirectory": "C:\\Dev\\Repos\\stryker-net\\integrationtest\\TargetProjects"
    }
    }
    }
  3. Run the launch profile in debug and the debugger should connect to the data collector at this line: https://github.com/stryker-mutator/stryker-net/blob/a96e366996d19b617fd7eb79d7c79654b64068de/src/Stryker.DataCollector/Stryker.DataCollector/CoverageCollector.cs#L212

I've added comments above that line that explain what is going wrong (in my eyes).

If you could take a look that would be great :)

MarcoRossignoli commented 2 years ago

@richardwerkman by design test project is loaded inside an app domain for .NET Framework application(I did some test with xunit) to isolate platform assemblies from test adapter ones and it's done by the adapter self(we don't have control on it) The InProcess datacollector is loaded in the "default" app domain and this is the reason why you cannot access to the test asm from it. You can disable the test app domain using a runsetting or the commandline

dotnet test -- RunConfiguration.DisableAppDomain=true
vstest.console -- RunConfiguration.DisableAppDomain=true

RunSettings

<RunSettings>
    <RunConfiguration>
        <DisableAppDomain>True</DisableAppDomain>
    </RunConfiguration>
</RunSettings>

You can verify it using Process Explorer .NET Assemblies tab for the host process.

The issue does not affect .NET Core because AppDomains have been removed.

I'll tag this by-design, feel free to close the issue if solved.

richardwerkman commented 1 year ago

Just to clarify we don't run for .NET Framework. We run for .NET 6. I've tested and disabling the app domain had no effect, as expected since we don't run .NET Framework. Do you have any other ideas as to what could be the cause for this?

And I'd also like to add that the issue doesn't occur in all cases. When running the DataCollector against a single project it seems to work. When running it against multiple projects it seems to work the second time. But the first time the assemblies are not loaded.

So the case that fails is: use Buildalyzer to get all projects in a solution. It finds 2 projects. For both projects, we initialize a vstest.console instance. Then we try to get coverage for the first project using a data collector. It fails. Then we try to get coverage for the second project using a data collector. It succeeds. It doesn't matter what project is located first, it chooses a project order random. But somehow the first run always fails and the second run succeeds. So the project itself doesn't seem to be the problem. More the order in which we execute things maybe?

rouke-broersma commented 1 year ago

When running it against multiple projects it seems to work the second time. But the first time the assemblies are not loaded.

Note that first and second time refers to first and second test assembly. The first test assembly always fails to be loaded at the point in the datacollector where we can hook into the process, the second assembly is found.

richardwerkman commented 1 year ago

The issue does not affect .NET Core because AppDomains have been removed.

This statement is irrelevant as our issue is on .NET Core. Could the by-design label be removed?

MarcoRossignoli commented 1 year ago

Sorry for late response I was on holiday, I'll take a look asap, I wasn't able to run the repro and so I've tested with default xunit template.

InProcDataCollector loads tested assemblies before test is started so they can be requested using:

As info, collectors don't load tests..it's the test adapter(xunit, mstest, nunit etc...) that is not in our control that load, discovery, run tests, InProcess/OutOfProc data collector get callbacks during tests execution but they don't know about how tests are implemented.

rouke-broersma commented 1 year ago

Sorry by loading test assemblies we simply mean that we ask for the loaded assemblies so that we can find the assembly containing the tests and so we have a reference to that assembly. We inject some code into this assembly before starting the testrun so we can communicate with the test assembly during testrun execution in order to gather coverage information etc. Stuff like that.

What we expect is that when we receive a callback, we should be able to find the assembly. This is always true when one test assembly is passed to vstest, this is not true for the first test assembly when passing multiple test assemblies to vstest.

MarcoRossignoli commented 1 year ago

@richardwerkman @rouke-broersma I did repro...I see the library loaded...what is not here is the class you expect

image

I don't know how stryker works but looks like this is not the instumented test lib if you're looking to something injected.

rouke-broersma commented 1 year ago

@MarcoRossignoli It looks like you're looking at the test project (project containing unit tests) and not the project under test (project being tested by unit test project). It is the project under test that is instrumented. I see that I made a typo in my comment, my apologies. All this test assembly and assembly under test is prone to errors and confusion imo.

I will try to gather the actual names of the projects to try and make this less confusing..

MarcoRossignoli commented 1 year ago

I need to have a repro like above sample where you do a AppDomain.GetAssemblies() and print somewhere the list of names and files path and be sure to avoid any filtering on types(the code inside the InProc above filters using a LINQ query). I don't expect different behavior than the sample I've shown for any other .NET Core test run also if it's a child of this one. If you're able to give me the tree of processes and when "the correct breakpoint" will fire I can take another look.

I've followed this one https://github.com/microsoft/vstest/issues/3810#issuecomment-1171561634 at the moment.

rouke-broersma commented 1 year ago

The repro is not invalid, it is our explanations of the problem that are confusing I think. I will wait for @richardwerkman to be back and we will try to provide a complete scenario and repro.

richardwerkman commented 1 year ago

@MarcoRossignoli I've updated the repro to hopefully make it more clear!

  1. Clone the Stryker repo and check out this branch: https://github.com/stryker-mutator/stryker-net/tree/vstest_assembly_load_repro (I've updated the branch so pull if you still have it locally)

  2. Add a launchSettings.json to the Stryker.CLI project containing: (update the paths to your local paths)

    {
    "profiles": {
    "Integrationtest solution": {
      "commandName": "Project",
      "commandLineArgs": "-s C:\\Dev\\Repos\\stryker-net\\integrationtest\\TargetProjects\\IntegrationTestApp.sln",
      "workingDirectory": "C:\\Dev\\Repos\\stryker-net\\integrationtest\\TargetProjects"
    }
    }
    }
  3. Place a breakpoint on this line: https://github.com/stryker-mutator/stryker-net/blob/ee080a0a377db0475f24cbda424140dea84f8f54/src/Stryker.Core/Stryker.Core/StrykerRunner.cs#L148 When the breakpoint is hit the variable project shows what project is analysed first. It's either TargetProject or ExtraProject (depends on the order buildalyzer returns the projecs). You can find the project name in the property ProjectComponents.

  4. Run the launch profile in debug and the debugger should connect to the data collector at this line: https://github.com/stryker-mutator/stryker-net/blob/a96e366996d19b617fd7eb79d7c79654b64068de/src/Stryker.DataCollector/Stryker.DataCollector/CoverageCollector.cs#L212

  5. Use the assemblies variable to see if the assembly of the project found in step 3 is loaded. For us it doesn't load even though we expect it to be loaded at this point.

I hope this updated repro is more clear! We would appreciate it if you could take another look at this as this issue is blocking us at the moment.

richardwerkman commented 1 year ago

Did you have time to retest this? Sadly we still face this issue 😿

MarcoRossignoli commented 1 year ago

Before to debug it again..can you print the list of assemblies returned by AppDomain.CurrentDomain.GetAssemblies() and ensure that you're in the correct "process"(I don't know if tool run more than one time and every time starts a different testhost as consequence and maybe the one with the dll inside is already closed)?

richardwerkman commented 1 year ago

Sure, give me a moment

richardwerkman commented 1 year ago

I was struggling to produce a list of current assemblies. I always checked the list by using the visual studio debugger and searched for the assembly name. This never resulted in a match, even though other assemblies could be found. But now I changed the line to:

var assemblies = AppDomain.CurrentDomain.GetAssemblies().Select(x => x.GetName().Name);

And I was surprised! Suddenly the assembly was found in this list. So it looks like you were right. After further investigation, I found a bug in our code that also could explain this behavior. I'll close this issue as it seems like we were just checking the facts wrong. I'm sorry for the inconvinience. Thanks for your help

MarcoRossignoli commented 1 year ago

Glad to hear that is not a bug 😄 thanks for your patience.

rouke-broersma commented 1 year ago

For context, we were rebuilding the test project for the next target project before we had collected coverage, which overwrite our injected assembly...

We were convinced the issue couldn't be there because we could see the injected assembly being written to disk.. 🙃