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

Re-tuning jacoco coverage for a higher quality "you need to write more tests". #578

Merged

Conversation

npxcomplete
Copy link

I had a PR blocked on a coverage limit and decided the coverage report needed improving. First, all generated classes should be excluded from the report. Second, the block is applied to branch based coverage. Line based coverage shall no longer block changes. Why? A simple example: In a nested function call 'foo(bar(5))' if code is refactored to 'int x = bar(5); foo(x);' We've altered the line count and therefore the coverage ratio without any change to the code complexity or even to the AST. Branch coverage by contrast is a much closer approximation of 'has this code changed in complexity or behavior'. Thirdly, the scope of coverage is changed from PACKAGE to BUNDLE. PACKAGE makes sense for some libraries that limit interaction between package scopes. Since this repository contains deployable units (ie. lambda functions equivalent to executables) we choose to evaluate the coverage in totality. Why? A simple example is if some method is extracted across package boundaries the safety and correctness of the bundle hasn't changed but the coverage may throw a hissy fit. Overall these changes will produce a higher value signal when coverage checks do fail.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…t needed improving. First, all generated classes should be excluded from the report. Second, the block is applied to branch based coverage. Line based coverage shall no longer block changes. Why? A simple example: In a nested function call 'foo(bar(5))' if code is refactored to 'int x = bar(5); foo(x);' We've altered the line count and therefore the coverage ratio without any change to the code complexity or even to the AST. Branch coverage by contrast is a much closer approximation of 'has this code changed in complexity or behavior'. Thirdly, the scope of coverage is changed from PACKAGE to BUNDLE. PACKAGE makes sense for some libraries that limit interaction between package scopes. Since this repository contains deployable units (ie. lambda functions equivalent to executables) we choose to evaluate the coverage in totality. Why? A simple example is if some method is extracted across package boundaries the safety and correctness of the bundle hasn't changed but the coverage may throw a hissy fit. Overall these changes will produce a higher value signal when coverage checks do fail.
@npxcomplete npxcomplete marked this pull request as ready for review November 8, 2024 02:53
@npxcomplete npxcomplete added this pull request to the merge queue Nov 8, 2024
Merged via the queue into aws-cloudformation:master with commit 80cb0bd Nov 8, 2024
1 check passed
@npxcomplete npxcomplete deleted the re-tuning-coverage-checks branch November 8, 2024 03:58
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