stryker-mutator / stryker-net

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

Stryker disable comment not working for global statements #3073

Closed cfuehrmann closed 2 weeks ago

cfuehrmann commented 1 month ago

Describe the bug

I have a .NET 8 Web API whose "Program.cs" contains the code below.

image

The variable SWAGGER_MODULE is set when the project compiles with the "DEBUG" configuration, and not set when it compiles with the "RELEASE" configuration. Eitherway, note how the // Stryker disable all is outside of the #if. Still, the code gets mutated. (This is also the case if I move the // Stryker disable all inside the #if.)

Why does // Strykler disable all not work?

Logs

Stryker takes several hours on this very big project. I can run it over night and deliver the logs if really required.

Expected behavior

I expect that the code between // Stryker disable all and // Stryker restore all is not mutated.

Desktop (please complete the following information):

dupdob commented 1 month ago

hi thanks for reporting this. I added a unit test to try to reproduce this issue but it appears to be working. Disregarding the placement of the ##if ... #endif block, the mutations are not generated. In other words, I do not reproduce your problem. The stryker comment is supposed to work and I have no explanation on why it fails in your case. I have questions for you:

  1. Could try using // Stryker disable once all on the app.UseSwagerUI(...) line and report the result ?

  2. Are you using an incremental feature (via 'since') ? this may the cause (there may be a bug when ignoring mutants via stryker comments is not properly handled)

  3. Do you use similar Stryker comments elsewhere in the project and do they work properly ? if not, could you make a test and report, please?

cfuehrmann commented 1 month ago

Hi, thanks for your quick reaction!

cfuehrmann commented 1 month ago

I should maybe also mention that the WebApi.csproj contains this:

    <PropertyGroup Condition="$(Configuration.Equals('Debug'))">
        <DefineConstants>SWAGGER_MODULE;SERILOG_SINK_SEQ_MODULE</DefineConstants>
    </PropertyGroup>
dupdob commented 1 month ago

Thanks. Any occurrence where you use disable all outside `#if...' constructs? I am trying to figure out if there is a true correlation.

PS: yes, it does not appear you use any form of diff mutation testing, so no lead there. It does look like an issue with preprocessor command and I cannot see how it could have an impact, but it looks like it does.

cfuehrmann commented 1 month ago

I checked with an overnight run, and indeed // Stryker disable all seems to work in other places. Below you see another screenshot from our "Program.cs". The highlighted area is full with mutations without having // Stryker disable all there.

image

dupdob commented 1 month ago

Many thanks, this helps me. I will try and reproduce this on a smaller project. If successful I can probably fix this straight away. I will revert to you if I am stuck and need extra information from your problem.

dupdob commented 1 month ago

Hummm I am not able to test it, but I have a theory: it is possible that the Stryker comment is linked to the compiler directives (#if). Those are ignored by Stryker parser, so any linked comments will be ignored as well.

You could introduce an empty statement (´;´) between the directive and the comment.

dupdob commented 1 month ago

Still unable to reproduce it, but at least, I know that Stryker tends to mess with #endif directives

cfuehrmann commented 1 month ago

I will add the extra semicolon, run it over night, and report back.

dupdob commented 1 month ago

Since I cannot reproduce it, no promises there. BTW could you share the later statement before the #if ? My current theory is that both the Stryker comment and the directive were linked to the previous statement for some reason.

cfuehrmann commented 1 month ago

Here we go:

image

Mind you, the disabling statement also failed when it was inside the pragma.

cfuehrmann commented 1 month ago

Hi, here the results with semicolons: image

dupdob commented 1 month ago

Well. They are still there. No idea why they appear in timeout. Interestingly, the first statement which was ignored in the original code is now mutated (and in timeout). As if disable all was handled like disable once all.

cfuehrmann commented 1 month ago

If there is any other info I can provide, please let me know.

dupdob commented 4 weeks ago

Well. I was about to abandon all hopes, but I got a bunch of weird results doing integrated tests (vs unit): some mutators are disabled but not others. So there is something fishy here and I have a reproducing project. So, I can work now

cfuehrmann commented 4 weeks ago

Wow, thanks for the heroic stamina!

dupdob commented 4 weeks ago

I have to revert my previous findings: I was so focused on the problem that I failed to notice there was another Stryker comment involved in my test. Long story short: everything works as documented in my test project.

So I still have no idea on why this happens and/or how to replicate it.

dupdob commented 4 weeks ago

I am afraid the log will not be able to help me, and I definitely need your help to move forward. I assume this project is not publicly accessible (if yes, please share the link). But, parsing happens at the file level, so maybe you can share me the 'offending' source file privately with me. Alternatively, devise a small project that reproduce the problem.

Finally, you can also try and debug Stryker locally and see what happens. I can give you pointers where to look and hook 😄

cfuehrmann commented 4 weeks ago

I've just looked through the offending file ("Program.cs"). While the whole API is a commercial product, I can easily defend sharing the "Program.cs" with you, if you provide me with an upload pointer.

Be warned though that the "Program.cs", being the "composition root" in the sense of dependency injection, is full of dependencies that I cannot provide.

dupdob commented 4 weeks ago

Thanks. I think the best way is for you to go to the project slack: https://stryker-mutator.slack.com/join/shared_invite/enQtOTUyMTYyNTg1NDQ0LTU4ODNmZDlmN2I3MmEyMTVhYjZlYmJkOThlNTY3NTM1M2QxYmM5YTM3ODQxYmJjY2YyYzllM2RkMmM1NjNjZjM#/shared-invite/email

And DM me there with the file. You can also email to me (I think my email is publicly displayed here)

cfuehrmann commented 4 weeks ago

Thanks, I just DMed you the file.

dupdob commented 4 weeks ago

Thanks for your help. I am now able to reproduce the issue. This is a problem with top level statements. It turns out we have no test for these (unit or otherwise). For some reason, // Stryker disable all applies only to the next statement.

I am looking into how to fix this. I suppose you consider this problem as a minor until we provide a fix, but if you want to have a workaround, you can either use the legacy construct for the main method. If you can't or it does not suit you, you can use // Stryker disable once all end enclose this bit of code in curly braces for the same result.

Ultimately, you can also use multiple // Stryker disable once.

Sorry for the inconvenience and thank you for bearing with me with this.

dupdob commented 4 weeks ago

I just opened PR #3087 that will fix that.

cfuehrmann commented 4 weeks ago

This problem is indeed a minor to me. Thanks for your effort! Should I close this issue?