search

rpl / flow-coverage-report

Generate flow coverage reports in JSON, HTML and in the console.
MIT License
505 stars 42 forks source link

Display coverage percent with decimals #148

Closed AmauryLiet closed 6 years ago

AmauryLiet commented 6 years ago

Hi! First of all, thanks for the great lib :) It really helps us tracking+finding how to improve our coverage!

I wanted to suggest a feature, it would be great to be able to see the decimals of the coverage in the final summary. For instance:

project percent total covered uncovered
app 70.97 % 35616 25277 10339

(instead of just 70%)

It would allow to have a more micro visibility on the impact of the recent changes the developer made.

What do you think about it? I would be happy to help ;)

benadamstyles commented 6 years ago

I don't mean to +1 unnecessarily but I just want to add that this would make a huge difference to me, because my code base is large enough that 1% represents many lines of code, so the difference between e.g. 96% and 97% is a lot.

AmauryLiet commented 6 years ago

@rpl What's your input on this? Against it? "Why not but I don't have time"? :)

rpl commented 6 years ago

@AmauryLiet :+1: sounds definitely like a reasonable feature

I'm just unsure if it should be the default (I guess that there are projects on which the current behavior could be still be reasonable and the summaries easier to read in those cases, and developers that may prefer the current behavior for that reason).

I didn't had too much time to spent on this recently, but it looks that @Leeds-eBooks has started to look into it in #157 (thanks @Leeds-eBooks), I'm going to look into that pull request.

AmauryLiet commented 6 years ago

Yes, adding a flag -d --withDigits would make sense (I'll leave you the choice of wording 😛)

benadamstyles commented 6 years ago

Yeah I thought initially about making it optional (and making the number of decimal places optional too), but that would mean a much more extensive PR and therefore a much depper understanding of the codebase, which I didn't have time for! The percentage value is passed around as a number, so the conversion to a string for display happens in each reporter – so I guess it would be fairly simple to avoid rounding the number at all in getCoveredPercent() and just using something like .toFixed(options.decimalPlaces) in all the places it gets converted to a string.