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

Improve code coverage #101

Closed
1 of 2 tasks
nahueespinosa opened this issue Feb 9, 2023 · 4 comments
Closed
1 of 2 tasks

Improve code coverage #101

nahueespinosa opened this issue Feb 9, 2023 · 4 comments
Assignees
Labels
cpp Related to C++ code enhancement New feature or request

Comments

@nahueespinosa
Copy link
Member

nahueespinosa commented Feb 9, 2023

Description

In the latest version of beluga (fb8294d at the time of this post), the code coverage report shows:

Summary: Line Rate = 59% (861 / 1467), Branch Rate = 16% (915 / 5548)

We should start working towards improving these metrics for existing code. In particular, beluga_amcl does not have any tests for the ROS 2 node implementation.

Definition of done

  • Line rate coverage greater than 90%.
  • Branch rate coverage greater than 90%.

Additional considerations

Reference: https://testing.googleblog.com/2020/08/code-coverage-best-practices.html

Focusing on getting the number as close as possible to 100% leads to a false sense of security. It could also be wasteful, burning machine cycles and creating technical debt from low-value tests that now need to be maintained. Bad code being pushed to production due to missing tests could happen either because (a) your tests did not cover a specific path of code, a test gap that is easy to identify with code coverage analysis, or (b) because your tests did not cover a specific edge case in an area that did have code coverage, which is difficult or impossible to catch with code coverage analysis. Code coverage does not guarantee that the covered lines or branches have been tested correctly, it just guarantees that they have been executed by a test. Be mindful of copy/pasting tests just for the sake of increasing coverage, or adding tests with little actual value, to comply with the number.

@nahueespinosa nahueespinosa added enhancement New feature or request cpp Related to C++ code labels Feb 9, 2023
@glpuga
Copy link
Collaborator

glpuga commented Feb 9, 2023

Do we know which parts are the most under-tested? different code sections may have different reasons for that.

JIC templates may also add subtle distortions in coverage measurement, an so do inline functions:

@nahueespinosa
Copy link
Member Author

nahueespinosa commented Feb 9, 2023

Do we know which parts are the most under-tested?

For beluga, we get:

  Reading tracefile /__w/beluga/beluga/ros_ws/build/beluga/coverage.info
  Summary coverage rate:
    lines......: 82.8% (731 of 883 lines)
    functions..: 85.4% (169 of 198 functions)
    branches...: 21.5% (782 of 3632 branches)

And for beluga_amcl:

  Reading tracefile /__w/beluga/beluga/ros_ws/build/beluga_amcl/coverage.info
  Summary coverage rate:
    lines......: 23.1% (130 of 563 lines)
    functions..: 34.1% (15 of 44 functions)
    branches...: 6.9% (133 of 1916 branches)

So, most of the untested code is in beluga_amcl.

JIC templates may also add subtle distortions in coverage measurement, an so do inline functions:

That's useful to know, thanks!

@olmerg
Copy link
Collaborator

olmerg commented Feb 10, 2023

We can make an integration test like is make in nav2, which will be similar to the beluga example, but with a node which validate that

  • set of the initial position
  • publication of estimated pose
  • broadcast publication
  • particles publication
  • likelihood_field_pub_

@ivanpauno
Copy link
Collaborator

Created a new ticket for system level tests: #127

@nahueespinosa nahueespinosa self-assigned this May 20, 2023
nahueespinosa added a commit that referenced this issue May 25, 2023
Related to #101, this adds tests for the AMCL node up to 95% line
coverage. It also fixes a couple of issues with lifecycle management and
removes the read_only property for parameters that can change at
runtime.

Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
nahueespinosa added a commit that referenced this issue May 31, 2023
Related to #101.

GCC records branch information for each line where a scope exit due to
some thrown exception is possible. This sets too high a standard for us
to meet. C++ templates may also add subtle distortions in coverage
measurement, an so do inline functions.

This patch keeps only line coverage data collection while disabling
branch and function coverage.

Signed-off-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp Related to C++ code enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants