stryker-mutator / stryker-net

Mutation testing for .NET core and .NET framework!
https://stryker-mutator.io
Apache License 2.0
1.76k stars 175 forks source link

Ignore weird test projects for the solution run #2391

Closed psfinaki closed 1 year ago

psfinaki commented 1 year ago

Describe the bug Time for a bug from the category "why the hell is he doing this".

So basically I'm finally trying the solution run for our huge SDK. This seems to be a good load test for Stryker :D

2023-01-27T15:37:24.0576022Z [15:37:24 INF] Identifying projects to mutate in C:\__w\1\s\SDK.sln. This can take a while.
2023-01-27T16:22:19.5364920Z [16:22:19 INF] Found 268 source projects
2023-01-27T16:22:19.5404589Z [16:22:19 INF] Found 234 test projects
2023-01-27T16:45:26.7633799Z [16:45:26 INF] The project X will be mutated.

Things were somehow executing for about 2 hours (usually our full mutation testing runs for about 10 hours FYI) until Stryker encountered a weird test project which completely crashed the whole run.

The particular case is that we have this X.csproj project which actually only contains .ps1 scripts and then we have X.Tests.csproj project which contains a reference to X.csproj but actually doesn't do much since, well, there is no C# code to test.

Why do we need this test project then? That's because we have some weird pipelines/policies/whatever and this is somehow needed for our correct internal code coverage analysis.

Now, from what I understand, Stryker would normally identify this X.Tests.csproj as a test target project, not a test project. But since we have so many projects in the repository, we make heavy use of those Directory.Build.Props files which, in case of the test projects' folder, contain all the xunit references so they get propagated to this X.Tests.csproj as well. Because of this, Stryker identifies this as a test project, tries to do something, and then fails, crashing the whole thing.

The exact error happening in our pipeline looks like this:

[17:59:39 INF] The project X.csproj will be mutated.
[17:59:39 INF] Analysis complete.
[17:59:47 INF] Total number of tests found: Unable to detect.
[17:59:47 INF] Initial testrun started.
[17:59:52 INF] Time Elapsed 02:22:28.6038303
Stryker.NET failed to mutate your project. For more information see the logs 
below:

Project X.Tests.csproj' is not supported by VsTest because it is missing an appropriate VstTest
adapter for 'xunit.core'. Adding 'xunit.runner.visualstudio' to this project 
references may resolve the issue.

(this is somewhat misleading btw as the adapter is also transitively referenced in the project)

Logs I am adding a kind of repro for this: Repro.zip

It's not exactly the same error, but the same idea - a weird (but legitimate) test project crashes the whole execution: image

Expected behavior So there are two things here: 1) On a lower level, the test project detection can be improved. After all, I can see a more reasonable scenario here: imagine a project containing some shared xUnit utilities (extensions, assertions, etc) which is used by real test projects in the solution - this utility project should be considered a test project itself. Maybe, from the practical point, you could just require a test project to have at least one test.

2) On a higher level, in case of solution runs, whatever crap happens to a particular source/test project pair, shouldn't stop the whole party. People report here all kinds of strange projects which can cause something similar, you know this much better :) So some general exception handler would probably be useful. On that note, I would also appreciate some kind of summary of all this crap at the end of the mutation testing run, similar to what dotnet build does after the whole build.

Desktop (please complete the following information):

dupdob commented 1 year ago

Thanks for the report. I am looking into this.

On a higher level, in case of solution runs, whatever crap happens to a particular source/test project pair, shouldn't stop the whole party...

Yes, you are right. Stryker has been designed around the 'mutate one project' use case, and it clearly shows there.

On that note, I would also appreciate some kind of summary of all this crap at the end of the mutation testing run..

Makes sense. I guess it is going to be an incremental work, but we need to start somewhere

On a lower level, the test project detection can be improved...

I have reviewed the test detection logic, and I am not sure how it can be improved. It does not rely on the dependence on test frameworks. Instead, it checks if the IsTestProject property is true or if the project type is 'Test Project'. I think both needs to be set explicitly on the project. A sure sign of this would be that X.Tests is seen has a test project from VStudio perspective. I do think this is a fair criteria; checking the presence of some tests may also fail due to a missing vstest adapter (which happens when people do not use VsTest nor dotnet test).

psfinaki commented 1 year ago

@dupdob thanks for taking a look. It would be awesome to have this addressed - we have about 5 of similar cases in the codebase.

A sure sign of this would be that X.Tests is seen has a test project from VStudio perspective.

Wait so this means VS is relying on the project name? Not that I would be completely surprised but...

rouke-broersma commented 1 year ago

Thanks for the report. I am looking into this.

On a higher level, in case of solution runs, whatever crap happens to a particular source/test project pair, shouldn't stop the whole party...

Yes, you are right. Stryker has been designed around the 'mutate one project' use case, and it clearly shows there.

On that note, I would also appreciate some kind of summary of all this crap at the end of the mutation testing run..

Makes sense. I guess it is going to be an incremental work, but we need to start somewhere

On a lower level, the test project detection can be improved...

I have reviewed the test detection logic, and I am not sure how it can be improved. It does not rely on the dependence on test frameworks. Instead, it checks if the IsTestProject property is true or if the project type is 'Test Project'. I think both needs to be set explicitly on the project. A sure sign of this would be that X.Tests is seen has a test project from VStudio perspective. I do think this is a fair criteria; checking the presence of some tests may also fail due to a missing vstest adapter (which happens when people do not use VsTest nor dotnet test).

IsTestProject is set by all major test frameworks so simply having the test framework referenced will cause the project to be identified as test project.

psfinaki commented 1 year ago

Interesting. I mean, on that note, just a basic empty test project with an installed test framework (i.e. when you create one using builtin VS templates) fails the execution with a misleading error message, both in the project and solution mode: image

I'd argue this isn't normal anyway. If you want, I can create a separate issue although I'm surprised this hasn't popped up before :) Repro.zip

dupdob commented 1 year ago

Actually, Stryker cannot distinguish between a test project where test detection failed and an empty test project; so far, we had several occurrences of the first issue (improper test projects) and none of the second (no test), hence the error message. I will alter the message to expose that the issue may be an indication of a total lack of tests

psfinaki commented 1 year ago

Alright, thanks!