stryker-mutator / stryker-handbook

A comprehensive guide to the stryker ecosystem
Apache License 2.0
71 stars 11 forks source link

Discussion: mutation score on 0 mutants #30

Open hugo-vrijswijk opened 4 years ago

hugo-vrijswijk commented 4 years ago

After a short discussion with @nicojs yesterday I think it's best to open an issue.

Right now there's no discussed way of calculating what the score should be when there are 0 mutants. I'm not talking about showing the score as there's already a PR for this in elements: https://github.com/stryker-mutator/mutation-testing-elements/pull/52.

We already have standardized ways of calculating metrics, and I think this is a hole in that standardized way that should be filled.

There's 3 options (I think) that all have a valid argument:

Personally, I prefer NaN. Would like to hear your thoughts. cc: @nicojs @simondel @richardwerkman @Mobrockers

nicojs commented 4 years ago

I agree with NaN.

A reporter should display N/A (or something like that), since that makes the most sense for the end user. I feel like NaN most closely resembles the meaning of "N/A".

The also doesn't (strictly) break the current apis of the calculation modules, so cheap to implement.

@hugo-vrijswijk
One more question: what do you mean with "0 mutants"? 0 total mutants, or 0 valid mutants? See https://github.com/stryker-mutator/stryker-handbook/blob/master/mutant-states-and-metrics.md#metrics

hugo-vrijswijk commented 4 years ago

One more question: what do you mean with "0 mutants"? 0 total mutants, or 0 valid mutants?

Valid mutants. The score is based on valid. So for mutation score based on covered code it would be based on covered mutants. Or when mutants are ignored, that also doesn't count towards the score

rouke-broersma commented 4 years ago

NaN is an implementation detail pretty specific to javascript no? I am at least not familiar with using NaN in any other language. So in what context should I place this discussion?

It feels weird to me to return NaN as a result from mutation score calculation because it implies to me that we tried to do the calculation but could not get a result that is a number. Instead, the calculation should never take place if there are no mutations. Having valid mutations is a precondition to calculating a mutation score. There is no result from the calculation, number or otherwise/Nan. optional/nullable makes most sense to me.

nicojs commented 4 years ago

NaN is an implementation detail pretty specific to javascript

Nope:

optional/nullable makes most sense to me.

I agree somewhat. It can be either way for me. Making it nullable / optional will introduce some more code on the scala side.

rouke-broersma commented 4 years ago

Let me rephrase, I've never used it in c# and I hadn't heard of it before I looked it up and noticed that it does indeed exist 😛 It doesn't feel like the natural choice to me simply because I haven't heard of it. I think it's more common to use optional or exceptions instead in c#.

hugo-vrijswijk commented 4 years ago

optional/nullable makes most sense to me.

I don't like this. It doesn't make sense. We should always be able to (and we can) calculate metrics when given valid data. Making it nullable/optional would take away from that. It's confusing from a library API standpoint to me.

return NaN as a result from mutation score calculation because it implies to me that we tried to do the calculation but could not get a result that is a number

But this is what happens. We try to divide by 0, and the result from that cannot be expressed as a number. This is also how the NaN constant is defined in both Java and .NET

rouke-broersma commented 4 years ago

But this is what happens. We try to divide by 0, and the result from that cannot be expressed as a number.

I disagree. Not having mutants for a file is a completely valid and expected result that we can account for by not trying to do calculations and simply returning no-result. You're still arguing from the point of doing the calculation, in which case you're right the result would be NaN. But we can simply not do the calculation and then NaN makes no sense.