fixed coverage report inclusion #505
Merged
castler merged 1 commit intoJun 10, 2026
Merged
Conversation
AAmbuj
requested review from
LittleHuba,
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
AAmbuj
force-pushed
the
amsh_fix_coverage_instrumentation
branch
6 times, most recently
from
406cb1a to
2c3b3a3
Compare
castler
requested changes
Jun 10, 2026
| - name: Run Unit Test with Coverage for C++ | ||
| id: run-coverage | ||
| run: | | ||
| bazel coverage //... --build_tests_only |
Contributor
There was a problem hiding this comment.
We do not want to filter by unit tests only.
| size = "small", | ||
| timeout = "short", | ||
| tags = tags + ["manual"], | ||
| tags = tags + ["unit"], |
Contributor
There was a problem hiding this comment.
This change (the whole commit) has the side effect, that we will only execute _linux and _qnx, which is not waht we want. Thus we should drop that commit. I merged for this problem already a solution.
| ":unix_domain_test", | ||
| "@score_communication//score/message_passing/log:log_test", | ||
| "@score_communication//score/message_passing/non_allocating_future:non_allocating_future_test", | ||
| ":client_connection_test_linux", |
Contributor
There was a problem hiding this comment.
Since we drop previus commit we can drop this also.
|
|
||
| # With this instrumentation filter for our two main components, we ensure that `bazel coverage //...` is yielding the correct results | ||
| coverage --instrumentation_filter="^//score/message_passing[/:],^//score/mw/com[/:],-//score/mw/com/performance_benchmarks[/:],-//score/mw/.*/test[/:]". | ||
| coverage --instrument_test_targets |
Contributor
There was a problem hiding this comment.
Why do we want to instrument the test targets? If we don't do that, you do not have to filter it later.
| # With this instrumentation filter for our two main components, we ensure that `bazel coverage //...` is yielding the correct results | ||
| coverage --instrumentation_filter="^//score/message_passing[/:],^//score/mw/com[/:],-//score/mw/com/performance_benchmarks[/:],-//score/mw/.*/test[/:]". | ||
| coverage --instrument_test_targets | ||
| coverage --nobuild_tests_only |
Contributor
There was a problem hiding this comment.
We want to build everything,
| --ignore-errors category,inconsistent | ||
| --filter line,branch,range,region,branch_region \ | ||
| --rc no_exception_branch=1 \ | ||
| --ignore-errors category,inconsistent,mismatch |
Contributor
There was a problem hiding this comment.
I see no reason to change that?
| --branch-coverage \ | ||
| --ignore-errors category,inconsistent | ||
| --filter line,branch,range,region,branch_region \ | ||
| --rc no_exception_branch=1 \ |
| --function-coverage \ | ||
| --branch-coverage \ | ||
| --ignore-errors category,inconsistent | ||
| --filter line,branch,range,region,branch_region \ |
| id: run-coverage | ||
| run: | | ||
| bazel coverage //... --build_tests_only | ||
| bazel coverage //... --test_tag_filters=unit |
Contributor
There was a problem hiding this comment.
We do not want to change this.
|
|
||
| ```bash | ||
| bazel coverage //... | ||
| bazel coverage //... --test_tag_filters=unit |
AAmbuj
force-pushed
the
amsh_fix_coverage_instrumentation
branch
from
2c3b3a3 to
71e621e
Compare
castler
approved these changes
Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-More stable coverage in CI
-Reduced false uncovered lines from non-executable content
-Clearer branch coverage signal
-Better alignment with the reference coverage