Skip to content

fix: show a better message when coverage is really close to min coverage#1084

Merged
alestiago merged 2 commits intoVeryGoodOpenSource:mainfrom
Kirpal:fix/coverage-message
Jul 26, 2024
Merged

fix: show a better message when coverage is really close to min coverage#1084
alestiago merged 2 commits intoVeryGoodOpenSource:mainfrom
Kirpal:fix/coverage-message

Conversation

@Kirpal
Copy link
Contributor

@Kirpal Kirpal commented Jul 12, 2024

Status

READY

Description

On our project we had one line uncovered, which gave us a coverage of 99.995%. Our minimum coverage is 100%. The message given was Expected coverage >= 100.00% but actual is 100.00%, which isn't very helpful. This change will still use 2 decimal places unless the actual coverage is so close that it rounds to the expected amount, and in that case will increase to an appropriate number of decimal places.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@Kirpal Kirpal requested a review from a team as a code owner July 12, 2024 20:22
).called(1);
verify(
() => logger.err(
'Expected coverage >= 100.000000% but actual is 99.999995%.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still prefer the 100 to stay the same (100.00%), the actual is where the decimal point matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because there could be a case where someone has min coverage going to more than two decimal places, like 99.004. In that case, if the coverage was 99.003, then the message would be Expected coverage >= 99.00% but actual is 99.003%.

Obviously it's an edge case, but I don't think we should round either value if it could make the error messaging more confusing. Plus, in most cases the coverage won't be that close and it will display 100.00% anyway.

@alestiago alestiago merged commit 15bf549 into VeryGoodOpenSource:main Jul 26, 2024
matiasleyba pushed a commit to a-wallen/very_good_cli that referenced this pull request Jul 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants