Increase test coverage of C code [AP-949] #1377
Merged
Conversation
This file contains 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
woodfell
changed the title
woodfell
force-pushed
the
woodfell/try_to_increase_code_coverage
branch
2 times, most recently
from
f4eda7e to
a975a84
Compare
RReichert
approved these changes
Nov 21, 2023
generator/sbpg/test_structs.py
Outdated
There was a problem hiding this comment.
looks like the python sonarcloud checks need tweaking (out of scope for PR). its assuming this file is production code and needs test cases.
woodfell
force-pushed
the
woodfell/try_to_increase_code_coverage
branch
from
a975a84 to
9861721
Compare
jungleraptor
approved these changes
Nov 22, 2023
# Description @swift-nav/devinfra Companion PR for #1377 which introduces new test cases. Once merged this PR will increase test coverage of the C binding to near 100% # API compatibility Does this change introduce a API compatibility risk? <!-- Provide a short explanation why or why not --> ## API compatibility plan If the above is "Yes", please detail the compatibility (or migration) plan: <!-- Provide a short explanation plan here --> # JIRA Reference https://swift-nav.atlassian.net/browse/BOARD-XXXX
|
[libsbp-c] Kudos, SonarCloud Quality Gate passed!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
@swift-nav/devinfra
Increase the test coverage of C code by rewriting the template for the V4 C++ tests
This is a complete rewrite of the existing template which covers the following test cases:
*_encoded_len()sbp_state_t)sbp::Stateandsbp::MessageHandler<>)This comprehensive template covers 100% of generated V4 code. No attempt was made to increase coverage of the legacy API, although there is little code to test anyway.
This rewrite by itself increases code coverage from ~20% to ~55% just based on the existing test cases.
Coverage can be further increased by introducing new test cases for the 120 or so messages which are currently uncovered. This is done on a separate PR #1382 and brings coverage up to ~99% (generated locally with lcov, sonar cloud seems to have a different way of calculating it)
To keep things a little smaller this PR will not introduce the new test cases, they will be kept separate and merged in to this branch before going to master
To aid with reviewing the commits are broken up to logical steps. The first couple deal with some supporting changes such as altering the generate to pass through some extra meta information (the generated companion fields for variable length arrays were already present to some extent in the previous template, but the information provided was not sufficient to get complete test coverage)
Next there are some minor alterations to some existing test cases. These all follow the same pattern of only affecting messages which have variable length arrays or string. For variable length arrays the existing specification for the companion "count" fields is extended out with 2 extra bits of information, for encoded strings there is one new fields. Both of these are required in order to get 100% coverage in the new template.
Finally, all code is regenerated. The final commit makes up the vast majority of changes in this PR. I suggest concentrating on just a single test case along with the code template itself, once these are understood all the other generated files can be treated with confidence.
Sonar cloud is not reporting any code coverage for this PR because there are no changes to actual code in libsbp, only the test case source code which isn't included in the calculation. You can see the effect of this PR by following the sonar cloud link an noting the "estimated after merge" number.
For Reviewers
Suggested places to concentrate on:
generator/sbpg/targets/resources/c/test/v4/sbp_cpp_test.cc.j2- The main template which was rewritten for this PRspec/tests/yaml/swiftnav/sbp/logging/test_MsgFwd.yaml- for an example of how the meta information for variable length arrays has changedc/test/cpp/auto_check_sbp_logging_MsgFwd.cc- generated output of above test casespec/tests/yaml/swiftnav/sbp/settings/test_MsgSettingsReadByIndexResp.yaml- for an example of how the meta information for encoded strings has changedc/test/cpp/auto_check_sbp_settings_MsgSettingsReadByIndexDone.cc- generated output of above test caseAPI compatibility
Does this change introduce a API compatibility risk?
No
API compatibility plan
If the above is "Yes", please detail the compatibility (or migration) plan:
N/A
JIRA Reference
https://swift-nav.atlassian.net/browse/AP-949