Add build option for coverage

[jpsamper2009]

Description

• In order to get coverage output that can be parsed by tools like gcov, it is necessary to build with special flags: -fprofile-arcs and -ftest-coverage

• This currently means that one must add the following arguments to colcon build:

  --ament-cmake-args -DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS}  -fprofile-arcs -ftest-coverage " -DCMAKE_C_FLAGS="${CMAKE_C_FLAGS}  -fprofile-arcs -ftest-coverage "
  

• For example, see https://ci.ros2.org/view/nightly/job/nightly_linux_coverage/210/consoleFull

• While this is manageable in a CI server, it is burdensome when trying to generate coverage locally because of the length of the argument and the subtleties of escaping the arguments for CMake.

• This issue proposes adding a flag (e.g., --with-coverage-flags) to colcon build which wraps the long arguments above.

• Naturally, for other supported compilers (clang?), the with-coverage-flags option could also handle the specific compile flags.

Motivation

• Making it easier for developers to generate coverage locally

Expected behavior

colcon build --with-coverage --ament-cmake-args <other args>

works like

colcon build --ament-cmake-args -DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS}  -fprofile-arcs -ftest-coverage " -DCMAKE_C_FLAGS="${CMAKE_C_FLAGS}  -fprofile-arcs -ftest-coverage " <other args>

• Optionally, it could be useful to also include -DCOVERAGE_RUN=1 such that developers can adjust test behavior during coverage test runs.
  • For example, any timing-sensitive tests will tend to fail during coverage runs because of the extra CPU cycles, so the threshold for timeouts may need to be adjusted: e.g.,

#ifdef COVERAGE_RUN
    timeout = 20
#else
    timeout = 5
#endif

Future work

• There could also be an extension to colcon test_results to digest coverage results: e.g.,

colcon build --with-coverage
colcon test
colcon test_results --output-coverage

[dirk-thomas]

There are certainly many different cases where simplifying the command line arguments would be beneficial. I am not sure what the right approach for that would be.

I don't think adding those to colcon-core is a good approach since there is an infinite amount of different ones which could be useful and the "core" tries to be as small as possible - e.g. it doesn't know about CMake. colcon-cmake could be a package where this feature is added but I am not even convinced that adding a command line option to "abbreviate" the verbose version is the right way to go.

I would image this more like a "profile" / "config" feature. Somewhere there is a set of preconfigured options like "coverage" which defines to what existing command line options this should expand to. Maybe something like the --meta option or along those line. I haven't used it for such a case yet but it might be a starting point.

[jpsamper2009]

@dirk-thomas I think something like the --meta feature may be a clean way to implement this feature. A generic solution could be to allow aliasing of command line parameters? e.g.,

People could define an alias in the root of their workspace:

{
    "build": {
        "symlink-install": true
        "arg-alias": {
            "with-coverage" : "--ament-cmake-args -DCMAKE_CXX_FLAGS=\"${CMAKE_CXX_FLAGS}  -fprofile-arcs -ftest-coverage \" -DCMAKE_C_FLAGS=\"${CMAKE_C_FLAGS}  -fprofile-arcs -ftest-coverage\" "
        }
    }
}

So

colcon build --with-coverage

would expand to the long command

The challenges I see are that:

1. It would require some modifications so that if a non-existent argument is passed to colcon, colcon will look in the .meta file for aliases before exiting with an error.
2. It would require some changes to the argument parser to include these aliases in colcon --help
3. Arguments like --ament-cmake-args would need to become additive, so that one could use:

colcon build --with-coverage --ament-cmake-args <more-args>

4. Escaping characters (like quotes) could be a pain

[calvertdw]

Everyone's workflow for this kind of stuff is different but I handle this with a GitHub repo with pages of commonly used console commands. You can edit them directly on GitHub with the built-in editor, and when you want to use them, there is a convenient "Copy line" button for pasting into terminals.
Edit: Removing my image to reduce clutter in this thread since it is getting referenced frequently.

[dirk-thomas]

While this could be realized as a "verb alias" I do see a couple of downsides / limitations to it:

• Making the argparse infrastructure aware of these requires first parsing some arguments, then using those to find additional configuration files declaring aliases, and then add those to the argparse structure (in order to show up in --help etc.). This approach sounds rather complicated and especially runtime-complex to me (e.g. considering completion needs to do the same).

• The alias approach limits each "configuration snippet" to be used separately. Thinking about what kind of "configuration snippets" could be useful would include things like:

  • coverage
  • memory sanitizer
  • debug / release (with different optimization options as well as directories)
  • arguments for cross compilation

That being said I would rather like to see these kind of "configuration snippets" as mixins. So it should be able to choose multiple (assuming they don't collide / contradict each other) at the same time. So e.g. "memory sanitizer" together with "debug" or "coverage" together with "release" (not really good examples but I guess it clarifies the idea).

Those "mixins" could be contributed in a similar way as the .meta files in the colcon-metadata-repository and then easily shared and used by all users interested in them.

[jpsamper2009]

@dirk-thomas If I understand correctly, what you are proposing would look something like:

1. There exists a project similar to colcon-metadata-repository, let's call it colcon/colcon-mixin-repository

2. Register the mixins from the repository:

colcon mixins add my-mixins https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml

3. Update mixins:

colcon mixins update my-mixins

4. They are now available in colcon build:

$ colcon build --help
...
Mixin arguments:
  --with-coverage    Equivalent to --ament-cmake-args _DCMAKE_...
  --debug-build      Equivalent to --cmake-args -DCMAKE_BUILD_TYPE=Debug
  --compile-aarch64  Equivalent to --cmake-force-configure, --cmake-args ...
...

And the .mixin file could be something like:

{
    "build": {
        "with-coverage": [ "--ament-cmake-args -DCMAKE_...."],
        "debug-build": ["--cmake-args -DCMAKE_BUILD_TYPE=Debug"],
        "compile-aarch64": [
            "--cmake-force-configure",
            "--cmake-args ..."
        ]
    }
}

[dirk-thomas]

If I understand correctly, what you are proposing would look something like:

Exactly - thanks for the more concrete summary 👍

4. They are now available in colcon build:

I would probably lean toward colcon build --mixin with-coverage debug-build (so making the mixin names parameters of an actual --mixin argparse option. That would give you all the freedom for naming the mixins without the chance of colliding with other options in the future.

And the .mixin file could be something like:

The value of each mixin should probably follow the structure in the existing meta / yaml files:

{
    "build": {
        "with-coverage": {
            "ament-cmake-args": ["-DCMAKE_...."],
        }
        "debug-build": {
            "cmake-args": ["-DCMAKE_BUILD_TYPE=Debug"],
        }
        "compile-aarch64": {
            "cmake-force-configure": true,
            "cmake-args": [...],
        }
    }
}

I am just not sure yet how all these aspects should play together in the bigger picture: metadata, defaults, mixins, profiles (like in catkin-tools), etc. 🤔

[dirk-thomas]

Please see the new package colcon-mixin and the corresponding data repository colcon-mixin-repository which has a few PRs with some proposed mixins.

Any feedback on this is appreciated.

[jpsamper2009]

@dirk-thomas Thanks! I've created this PR which can close this issue