Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow percent numbers with decimals in --min-msi and --min-covered-msi #694

Closed
arudolph-cxt opened this issue May 28, 2019 · 3 comments · Fixed by #701
Closed

Allow percent numbers with decimals in --min-msi and --min-covered-msi #694

arudolph-cxt opened this issue May 28, 2019 · 3 comments · Fixed by #701

Comments

@arudolph-cxt
Copy link

Is your feature request related to a problem? Please describe.
We calculate the MSI and mutation coverage manually from the summary log and store the values with 2 decimals. When we add those values to the ci command-line script like so: ... --min-msi=15.77 it fails:
[ERROR] The minimum required MSI percentage should be 15.77%, but actual is 15%. Improve your tests!
Although the actual percentage should be compared with 15.77424023154848% to be exact and therefore pass.

Describe the solution you'd like
The comparison should use float values instead of rounded-down integers to decide if the ci fails.

Describe alternatives you've considered
Rounding down the value to an int, with the drawback of having much less accuracy on bigger projects.

@arudolph-cxt
Copy link
Author

According to the code, the flooring seems to be deliberate:

$detectionRateAll = floor(100 * $defeatedTotal / $this->totalMutantsCount);

Is there a reason for this? Since the values seem to be available as floats, this looks as simple as removing the floor call and ensuring the values are calculated as float and not auto-casted into integer values.
Unless i'm missing something and there was a good reason for this?

This commit seems to be the last change b2a7a2d and i understand why the change from round to floor was done, but why there was actual rounding in the first place seems confusing to me.

@maks-rafalko
Copy link
Member

maks-rafalko commented May 29, 2019

Personally, I don't remember any reasons for rounding. PR is always welcome ;)

@arudolph-cxt
Copy link
Author

I'm currently working on one and as expected, two tests fail as they compare against rounded values.

My current approach would be to just use float values instead, a more nuanced approach would be to allow control over the accuracy/decimal amount maybe with a parameter.
I think I can provide a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants