stryker-mutator / stryker-net

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

Compile error survives as mutant #2136

Open Dane0890 opened 2 years ago

Dane0890 commented 2 years ago

I have a method which looks like this

Public async Task<Enum> DeleteAsync(string myString)
{
   Try
   {
        DoSomeLogic,
        return Enum.Success
    }
    catch (Exception)
    {
        return Enum.Fail
    }
}

Stryker mutates my catch block to

catch(Exception)
{}

The return type is unit tested and fails if changed, and due to the missing return statement this mutation would cause a compile error

nicojs commented 2 years ago

I've transferred this issue because it seems to be related to Stryker.NET and not StrykerJS.

dupdob commented 2 years ago

Thanks @nicojs. Stryker.Net preventively injects a return default; statement at the end of method to maximize the number of generated mutations (when there is no return statement at the end of a method). I guess this mutant turns out to be hard to kill ?

nicojs commented 2 years ago

In that case it sounds like a great mutant! 😅

Maybe you could fill return default; as replacement text in these cases? I do agree that it is somewhat difficult to kill these if you don't know how to insert them in your source code.

dupdob commented 2 years ago

Currently, this is part of the 'helper' logic of Stryker.NET which automatically injects safeguarding code (such as out variables initialization) to avoid variable initializations related errors. So this is not a replacement code, in practice, Stryker will generates code that will look like that

Public async Task<Enum> DeleteAsync(string myString)
{
   Try
   {
if (Styker.IsActive(0))
{
        DoSomeLogicMutated,
}
else
{
        DoSomeLogic,
}
if (!Stryker.IsActive(1)) {
        return Enum.Success
}
    }
    catch (Exception)
    {
if (!Stryker.IsActive(2)) {
        return Enum.Fail
}
    }
return default;
}

As you can see, the return default; statement is useful for both mutant (1) and (2), and it cannot be shown as part of the report because it is far from the mutations of interest (note: this is a simplified version of the actual mutated code, which have at least 4 more mutations).

Adding return default; is not the ideal solution, but it is the best we have for now. Thank you for your feedback. It provides justification for trying to find a better solution.

nicojs commented 2 years ago

That sounds very reasonable.

Maybe this can be documented in an FAQ or troubleshooting guide. We started documenting known issues for StrykerJS in a troubleshooting guide on the website and I have the feeling it works quite well.

Dane0890 commented 2 years ago

Yeah it's stryker.NET that's my bad. And returns default passes the unit test so it was just something I was missing, thanks all!

Dane0890 commented 2 years ago

Ah I've closed the above erroneously, so you may need to throw it in a backlog elsewhere or reopen it, I do apologize.

RebelMage commented 1 year ago

I've just experienced this same issue. At least this explains why, because I was scratching my head, but it'd be nice to have a better solution.