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

Add test coverage #443

Closed
wants to merge 1 commit into from
Closed

Add test coverage #443

wants to merge 1 commit into from

Conversation

fatkodima
Copy link
Contributor

I need Test Coverage Id from owners to finish this.

@grzuy
Copy link
Collaborator

grzuy commented Dec 17, 2019

Hi @fatkodima,

I extremely value test suite quality. I really do. A lot. Which was the main driver for #269 and #293.

That said, I am not sure "Test Coverage" percentage indicators are very good at indicating high or low test suite quality. Also, I am worried it can lead people to over test with low-value/low-quality test cases just for the sake of getting a higher number. In general, I tend to agree with almost everything that Martin Fowler wrote in https://martinfowler.com/bliki/TestCoverage.html.

I can see the value of making it easy for anyone to run simplecov on demand so that someone can report an issue about any seriously untested code path, but not sure about promoting this indicator as a key metric for the quality of the project.

Thank you for the PR and your concern of improving the test suite :-)

@fatkodima
Copy link
Contributor Author

Hi, @grzuy
Yes, I'm mostly sharing your opinions. For me, only really low test coverage percentage is meaningful, as it indicates about very untested code. But high percentage is a bit meaningless due to the reasons mentioned in your comment.

This metric will only indicate that this project is enough tested and should not motivate people to improve test coverage by squeezing additional percent fractions. Feel free to close it if you have doubts.

Would you mind reviewing other PRs? :)

@grzuy grzuy closed this Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants