stryker-mutator / stryker-net

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

Fatal error on partial sealed generated class "enumeration type" #1506

Closed cmenzi closed 2 months ago

cmenzi commented 3 years ago

Describe the bug When executing stryker it tries to mutate a partial class where one part is generated through T4 template. It tries it several times and finally fails, with the message I should report the bug 😃

Logs log-20210413.txt VsTest-log.host.21-04-13_12-07-32_06992_9.txt VsTest-log.host.21-04-13_12-07-33_78670_9.txt VsTest-log.txt

Expected behavior Stryker should skip those mutation and not fatal failing.

Desktop (please complete the following information):

dupdob commented 3 years ago

Hi any chance we could the could access the source project? or maybe identify a reproducible scenario?

First, no mutator tries to change a base class on purpose, so it looks that Stryker parser was somehow fooled by some code construction it does not handle properly, so having the source code could help.

Partial declarations of 'xxx' must not specify different base classes is a worst case for Stryker as the error message appears may not be linked to the actual offending file, and it appears to be the case here. Afterward, Stryker tries to remove mutations, but none of them is the cause of the problem.

The log shows a known bug where Stryker stubbornly retry a build without changing anything (fixed in the code base).

As of know, I don't think there is not much we can do about this without some reproduction capacity

cmenzi commented 3 years ago

I will try to reproduce it and push the code to my github repo. I cannot share the whole code in public.

cmenzi commented 3 years ago

@dupdob

I was able to seperate the "malicious" code :-)

https://github.com/cmenzi/stryker-repro

dupdob commented 3 years ago

that's awesome. thank. I'll try to have a stab at it today

dupdob commented 3 years ago

Thanks. I can reproduce. I must say that I am stumped at this stage, as I have not the faintest clue at what is the cause for this compilation error. I mean, mutated source code looks ok....

dupdob commented 3 years ago

My best guess at this stage is that this is a red herring. The actual problem is that Stryker may not actually support TT files, due to compilation happening in memory. I don't have time now for further debugging. I'll update when I have time, hopefully later today

rouke-broersma commented 3 years ago

T4 templating sounds like some visual studio magic that uses a Custom Tool called TextTemplatingFilePreprocessor. Since this is a part of visual studio, and stryker does not (cannot) use visual studio during the build process, this custom tool is not executed and the code is not generated. As @dupdob guesses it seems like the generated part of the partial class simply does not exist.

This stackoverflow posts seems to suggest you can also have the code generation invoked using msbuild targets. Do you use these targets already? If not, could you try adding them?

cmenzi commented 3 years ago

No, we usually generate them upfront and also check the generated code into the repo in order to track changes. So, it's not invoked as part of the build.

Just for my understanding:

This seems to be the latest documentation about T4: https://docs.microsoft.com/en-us/visualstudio/modeling/code-generation-in-a-build-process?view=vs-2019

I will test it and give feedback

rouke-broersma commented 3 years ago

We copy the code from the filesystem into memory, and we perform the mutating and compilation in-memory. We do this for a couple reasons, mostly notably it's very very fast and also we get very specific feedback on compilation errors so we can remove bad mutations easily.

cmenzi commented 3 years ago

But the initial generated files also lying on the filesystem in this case.

rouke-broersma commented 3 years ago

Mhm yes, they should be picked up if they are already on the filesystem..

2021-04-13T12:07:06.1359155+02:00  [DBG] SourceFile "C:\data\iot\iot-library\gdm\source\src\Buhler.IoT.GenericDataModel\SupportedTypes.cs" (1607a25b)
2021-04-13T12:07:06.1359185+02:00  [DBG] SourceFile "C:\data\iot\iot-library\gdm\source\src\Buhler.IoT.GenericDataModel\SupportedTypes.generated.cs" (1607a25b)

They are detected 🤔

cmenzi commented 3 years ago

I've figured it out! https://github.com/cmenzi/stryker-repro/compare/fix/partial-class

There was even a little sign to it on latest version of stryker:

Partial declarations of 'SupportedTypes' must not specify different base classes.

I've removed the derived class from the generated file and it worked. So, the message should be called:

Partial declarations of 'SupportedTypes' must not specify ~different~ base classes

[11:57:50 WRN] Stryker.NET encountered an compile error in C:\data\github\stryker-repro\src\StrykerRepro\SupportedTypes.generated.cs (at 14:34) with message: Partial declarations of 'SupportedTypes' must not specify different base classes (Source code: internal sealed partial class SupportedTypes : ReadOnlyDictionary<Type, string>
    {
        private SupportedTypes()
            : base(new Dictionary<Type, string> { { typeof(byte), "byte" }, { typeof(sbyte), "sbyte" }, { typeof(short), "short" }, { typeof(ushort), "ushort" }, { typeof(int), "int" }, { typeof(uint), "uint" }, { typeof(long), "long" }, { typeof(ulong), "ulong" }, { typeof(float), "float" }, { typeof(double), "double" }, { typeof(decimal), "decimal" }, { typeof(bool), "bool" }, { typeof(char), "char" }, { typeof(DateTimeOffset), "DateTimeOffset" }, { typeof(DateTime), "DateTime" }, { typeof(string), "string" } })
        {
        }
    })
cmenzi commented 3 years ago

My guess is that each file is treated individually, so they get mutated differently and that's why they don't compile anymore.

rouke-broersma commented 3 years ago

My guess is that each file is treated individually, so they get mutated differently and that's why they don't compile anymore.

That should not be happening because of the auto-generated annotations. We detect those and skip mutating those files. I'm also not aware of any mutations we perform on base class declarations 😯

dupdob commented 3 years ago

let me share some early findings to guide your guessing:

dupdob commented 3 years ago

This is not a mutation issue, but a build issue. I still face this problem when disabling mutation.

dupdob commented 3 years ago

New findings: this is related to partial classes. I still have this issue without TT. My guess is that some specific care must be taken to compile partial classes with Roslyn, at least partial classes where the base class is repeated across segments

dupdob commented 3 years ago

There is an issue on Roslyn about a somewhat similar issue https://github.com/dotnet/roslyn/issues/45960

cmenzi commented 3 years ago

There is an issue on Roslyn about a somewhat similar issue dotnet/roslyn#45960

@dupdob Yes, the roslyn issue results in a similar error. But it doesn't seems it have much attention.

At least we can guide people with partial classes to only specify the derived classes or interfaces only once, like I did on my branch.

dupdob commented 3 years ago

For the short term, this is probably the best course of action, indeed. That being said, I am not sure the roslyn issue is an actual issue: I am not familiar enough on the design of the updated nullable/non nullable logic, but it is likely it relies on specific types, hence difference in types with or without nullable logic.

I have made some attempts to reproduce the issue with partial classes in unit tests but so far, every attempt results in successful compilation. It needs further analysis, but I my attention has been diverted on what looks like a regression on V.22.2 (see issues #1511 and #1512). I'll keep you posted.

dupdob commented 2 months ago

hello Sorry for the late answer: I just tested the repro project against the latest Stryker version and it ran properly. So I consider this is no longer an issue. Feel free to reopen it if the problem persists