Extra test cases [AP-949] #1382
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
force-pushed
the
woodfell/extra_test_cases
branch
from
17c20c9
to
c09e73e
Compare
woodfell
force-pushed
the
woodfell/extra_test_cases
branch
from
c09e73e
to
933e0c9
Compare
woodfell
changed the base branch from
master
to
woodfell/try_to_increase_code_coverage
|
[libsbp-c] Kudos, SonarCloud Quality Gate passed!
|
woodfell
requested review from
jungleraptor,
a team,
notoriaga and
pcrumley
as code owners
woodfell
merged commit 6b519c0
into
woodfell/try_to_increase_code_coverage
22 of 23 checks passed
woodfell
added a commit
that referenced
this pull request
Nov 22, 2023
# 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: - Behaviour of `*_encoded_len()` - Encoding payload directly to a user-provided buffer - Underflow errors and alternative uses - Decoding payload from a user provided buffer - Underflow errors and alternative uses - Sending and receiving complete frame through C API (`sbp_state_t`) - Sending and receiving complete frame through C++ API (`sbp::State` and `sbp::MessageHandler<>`) - Encoding/decoding payload through C++ API - Comparison functions and operators with all potential mismatches covered automatically - C++ type traits - All string functions generated as part of the API for each message which contains string fields 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 PR - `spec/tests/yaml/swiftnav/sbp/logging/test_MsgFwd.yaml` - for an example of how the meta information for variable length arrays has changed - `c/test/cpp/auto_check_sbp_logging_MsgFwd.cc` - generated output of above test case - `spec/tests/yaml/swiftnav/sbp/settings/test_MsgSettingsReadByIndexResp.yaml` - for an example of how the meta information for encoded strings has changed - `c/test/cpp/auto_check_sbp_settings_MsgSettingsReadByIndexDone.cc` - generated output of above test case # API 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
woodfell
added a commit
that referenced
this pull request
Nov 23, 2023
# Description @swift-nav/devinfra Not every CI stage ran on #1382 so when it merged to master a few things broke. These were the JS test stage and one of the rust linting stages. The fixes are minor, correcting a badly written test spec with a duplicate YAML key, creating exceptions for several test cases in the JS tests, and omitting generation of rust tests files which don't contain any cases # API 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/BOARD-XXXX
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
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?
API compatibility plan
If the above is "Yes", please detail the compatibility (or migration) plan:
JIRA Reference
https://swift-nav.atlassian.net/browse/BOARD-XXXX