100% test coverage #918
Labels
good first issue
Good for newcomers
Comments
anderseknert
added a commit
that referenced
this issue
Sep 8, 2024
Like with previous PR aiming to increase our test coverage (#1058), doing so this time revealed a few places where code was unused, or could be rewritten to be better optimized, removed, or made easier to read. One find that is yet to be addressed here is how we copy the whole `ast.found.refs` object into `ast.all_refs`, which surely comes with a cost that seems unnecessary, given we can use `ast.found.refs` directly instead. But since there are a few consumers, and they are not **exactly** identical (`all_refs` contains import refs too), I'll follow up on that in a later PR. I'm also adding the codecov badge to the README to make it easy to see when our coverage drops. But I'm not as convinced we should fail PRs when coverage isn't 100% as I was before, as it might make for a worse contributor experience. Fixes #918 Signed-off-by: Anders Eknert <[email protected]>
anderseknert
added a commit
that referenced
this issue
Sep 8, 2024
Like with previous PR aiming to increase our test coverage (#1058), doing so this time revealed a few places where code was unused, or could be rewritten to be better optimized, removed, or made easier to read. One find that is yet to be addressed here is how we copy the whole `ast.found.refs` object into `ast.all_refs`, which surely comes with a cost that seems unnecessary, given we can use `ast.found.refs` directly instead. But since there are a few consumers, and they are not **exactly** identical (`all_refs` contains import refs too), I'll follow up on that in a later PR. I'm also adding the codecov badge to the README to make it easy to see when our coverage drops. But I'm not as convinced we should fail PRs when coverage isn't 100% as I was before, as it might make for a worse contributor experience. Fixes #918 Signed-off-by: Anders Eknert <[email protected]>
anderseknert
added a commit
that referenced
this issue
Sep 8, 2024
Like with previous PR aiming to increase our test coverage (#1058), doing so this time revealed a few places where code was unused, or could be rewritten to be better optimized, removed, or made easier to read. One find that is yet to be addressed here is how we copy the whole `ast.found.refs` object into `ast.all_refs`, which surely comes with a cost that seems unnecessary, given we can use `ast.found.refs` directly instead. But since there are a few consumers, and they are not **exactly** identical (`all_refs` contains import refs too), I'll follow up on that in a later PR. I'm also adding the codecov badge to the README to make it easy to see when our coverage drops. But I'm not as convinced we should fail PRs when coverage isn't 100% as I was before, as it might make for a worse contributor experience. Fixes #918 Signed-off-by: Anders Eknert <[email protected]>
Merged
anderseknert
added a commit
that referenced
this issue
Sep 8, 2024
Like with previous PR aiming to increase our test coverage (#1058), doing so this time revealed a few places where code was unused, or could be rewritten to be better optimized, removed, or made easier to read. One find that is yet to be addressed here is how we copy the whole `ast.found.refs` object into `ast.all_refs`, which surely comes with a cost that seems unnecessary, given we can use `ast.found.refs` directly instead. But since there are a few consumers, and they are not **exactly** identical (`all_refs` contains import refs too), I'll follow up on that in a later PR. I'm also adding the codecov badge to the README to make it easy to see when our coverage drops. But I'm not as convinced we should fail PRs when coverage isn't 100% as I was before, as it might make for a worse contributor experience. Fixes #918 Signed-off-by: Anders Eknert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While this is mostly an expensive waste of time in most languages, it's fairly easy to achieve 100% test coverage in Rego, and without ever having to test "impossible" conditions. The fact that we've never measured test coverage in Regal and despite that have one above 98% is some evidence of that.
It would be a useful exercise to get that number to 100% to see if there is anything important we currently miss in our tests. Once we have 100% coverage, we should add a build step that enforces that going forward.
In order to see which lines aren't covered, just run
go run main.go test --coverage bundleand search the output for "not_covered".The text was updated successfully, but these errors were encountered: