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

Stryker mutates Debug.Assert statements #637

Closed apollux closed 5 years ago

apollux commented 5 years ago

Describe the bug Stryker.Net mutates Debug.Assert expressions. It looks like Stryker.Net build the code in Release config, therefore the Debug.Assert statement won't be executed. Making Stryker believe that the mutation survived.

Expected behavior I think Stryker.Net should not attempt to mutate Debug.Assert and Trace.Assert expressions. These are essentially tests as well, therefor it does not seem logical to change them. Alternatively Stryker.Net should define the Debug and Trace symbols while compiling, to have the assertions run.

rouke-broersma commented 5 years ago

You're right, we don't compile in debug mode. Do you know if setting the DEBUG constant is all that is required to compile in debug mode? We don't use msbuild to compile so the concept of debug or release build doesn't really exist and we have to recreate it ourselves unfortunately.

apollux commented 5 years ago

I think defining the symbol is sufficient to enable Debug.Assert. https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Debug/ref/System.Diagnostics.Debug.cs#L16 seems to indicate the same.

I am not sure if Stryker should mutate Debug/Trace Asserts or ignore them.

rouke-broersma commented 5 years ago

I don't think stryker should make assumptions about whether the user wants code tested or not. Trace.Assert statements should be enabled in release builds according to the docs so it could be used in production code to do some kind of logging. If we assume they should always be ignored then we are limiting users that do and it tested. Debug asserts could be ignored perhaps but even then, it is code that exists in your project, and it's not tested, that result is 100 percent valid whether it's by choice or not.

On Fri, Jul 26, 2019, 13:17 apollux notifications@github.com wrote:

I think defining the symbol is sufficient to enable Debug.Assert. https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Debug/ref/System.Diagnostics.Debug.cs#L16 seems to indicate the same.

I am not sure if Stryker should mutate Debug/Trace Asserts or ignore them.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stryker-mutator/stryker-net/issues/637?email_source=notifications&email_token=AA3N5HDEHZDLLIY4PYJ7NSDQBLMLFA5CNFSM4IGM6SAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD24JQ4Q#issuecomment-515414130, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3N5HADR4BRXFONTYDHU2DQBLMLFANCNFSM4IGM6SAA .

richardwerkman commented 5 years ago

@apollux Using our new feature #646 you could exclude Debug.Assert from mutation. The feature will be released with the next version.

apollux commented 5 years ago

That will work, thanks for getting back to this!