cucumber / react-components

React components for Cucumber
MIT License
32 stars 10 forks source link

Percentage of tests passed is rounded up #332

Open AbdelHajou opened 1 year ago

AbdelHajou commented 1 year ago

In the HTML report after running a test suite, the percentage of tests passed is rounded up. This should not be happening, as it could incorrectly indicate that 100% of tests passed even though 1 of the tests failed (the percentage would be 99,6% for example).

image

I'm using cucumber-java version 7.3.4 and the same version of cucumber-junit

mpkorstanje commented 1 year ago

Happy to take a pull request for this.

If some one wants to take a look, the code that calculates the percentage is here.

https://github.com/cucumber/react-components/blob/c27b94cd3c8ce89c899e2decc35755c4d50b2b7c/src/components/app/ExecutionSummary.tsx#L48-L51

mushahidq commented 1 year ago

Including maximumSignificantDigits in options for Intl.NumberFormat allows for decimal places.

const number = parseFloat((277/278).toFixed(10));
console.log(number)
// Expected output: 0.9964028777

console.log(new Intl.NumberFormat(undefined, { style: 'percent'}).format(number));
// Expected output: "100%"

console.log(new Intl.NumberFormat(undefined, { style: 'percent', maximumSignificantDigits: 5}).format(number));
// Expected output: "99.6%"

What would be the appropriate value for maximumSignificantDigits to use? I can make a PR with the same

mpkorstanje commented 1 year ago

I don't think that is the right solution. Try with const number = 0.999999999;

davidjgoss commented 1 year ago

We might consider 2 decimal places but always rounding down.

mushahidq commented 1 year ago

@mpkorstanje It rounds up to 100% for const number = 0.999999999; We. can parse to less decimal places if needed first.

let number = parseFloat((0.999999999).toFixed(10));
console.log(number)
// Expected output: 0.999999999

console.log(new Intl.NumberFormat(undefined, { style: 'percent'}).format(number));
// Expected output: "100%"

console.log(new Intl.NumberFormat(undefined, { style: 'percent', maximumSignificantDigits: 5}).format(number));
// Expected output: "100%"

number = parseFloat((0.9999).toFixed(10));
console.log(number)
// Expected output: 0.9999

console.log(new Intl.NumberFormat(undefined, { style: 'percent'}).format(number));
// Expected output: "100%"

console.log(new Intl.NumberFormat(undefined, { style: 'percent', maximumSignificantDigits: 5}).format(number));
// Expected output: "99.99%"
mpkorstanje commented 1 year ago

@mushahidq the problem that needs to be solved is that if there is 1 out of N failing tests for any positive non-zero value of N, the reported percentage should not be 100%. While you are suggesting the right solution, your example code doesn't accurately reflect that solution because you changed the input value from 0.999999999 to 0.9999 prior to rounding it.

If you do know the right solution, please do feel free to send a pull request. Otherwise you may have to find someone else first to coach you through this.

mushahidq commented 1 year ago

@mpkorstanje I apologise, I was testing using the same code segments for both cases hence the error. Someone had mentioned that rounding down to 2 decimal places might be considered so I was trying to provide an example of that where the number is round down before being passed to the formatter. Like for example using Math.floor(0.9999999999 * 10000) / 10000

AbdelHajou commented 1 year ago

I would suggest to just round down instead of up, to the nearest full number. I’d imagine displaying 99% of tests passed when it’s actually 99,6% wouldn’t matter as much as displaying 100%