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

Enable support for code coverage #504

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Briancbn
Copy link
Member

@Briancbn Briancbn commented Apr 28, 2020

Feature description

This PR aims to achieve what is described in #500 including
generating code coverage report for the following builder and packages

  • colcon ament-python
  • colcon ament-cmake
  • colcon catkin-cmake
  • colcon catkin-python
  • catkin_tools,catkin_make,catkin_make_isolated catkin-cmake
  • catkin_tools catkin-python

code coverage report tools support

  • codecov.io
  • coveralls.io

CI

  • Travis CI
  • Gitlab CI
  • Github Action
  • Bitbucket

additionally,

  • test cases

How to use

First, please register in codecov.io or coveralls.io using your respective git account. Then you need to enable inside your codecov.io and coveralls.io account to accept reports from specific repository.

Configure CODE_COVERAGE variable to the name of the web reporting tool to enable code coverage generation and uploading report to the respective web reporting tool website,
You can apply to all of your jobs by something like below, Or define CODE_COVERAGE per job.

In Travis CI

  • Codecov
    env:
      global:
       - CODE_COVERAGE="codecov.io"
      # - CODECOV_TOKEN="..." # For private repo
  • Coveralls
    env:
      global:
       - CODE_COVERAGE="coveralls.io"
       # - COVERALLS_REPO_TOKEN=".." # For private repo

In Gitlab CI

  • Codecov
    before_script:
      - apk add --update bash coreutils tar curl
      - git clone --quiet --depth 1 https://github.com/ros-industrial/industrial_ci .industrial_ci -b master
    ...
    variables:
      CODE_COVERAGE: "codecov.io"
      CODECOV_TOKEN: "..."
  • Coveralls
    before_script:
      - apk add --update bash coreutils tar
      - apk add --update python3 py3-pip python3-dev
      - git clone --quiet --depth 1 https://github.com/ros-industrial/industrial_ci .industrial_ci -b master
    ...
    variables:
      CODE_COVERGAE: "coveralls.io"
      COVERALLS_REPO_TOKEN: "..."

In Github Action

  • Codecov (references)
    name: CI
    on: [push, pull_request]
    jobs:
      industrial_ci:
        strategy:
          matrix:
            env:
              - {ROS_DISTRO: eloquent, ROS_REPO: main}
        env:
          CODE_COVERAGE: codecov.io
        runs-on: ubuntu-latest
        steps:
          - uses: actions/checkout@v1
          - uses: 'ros-industrial/industrial_ci@master'
            env: ${{matrix.env}}
          - uses: codecov/codecov-action@v1
            # token: ${{ secrets.CODECOV_TOKEN }} # For private repo
    
  • Coveralls.io
    ...
    jobs:
      industrial_ci:
        strategy:
          matrix:
          ...
        env:
          CODE_COVERGAE: coveralls.io
          COVERALLS_REPO_TOKEN: "..."
        runs-on: ubuntu-latest
        steps:
          ... 

Known Issues

  • Gitlab CI Coveralls: Cannot identify branches or Open file for coverage reference
  • ROS1 Python Coveralls: Cannot ignore __init__.py file in the build/ directory
  • ROS1 Python: Cannot generate python coverage report with catkin_make and catkin_make_isolated

Updates

July 2: Add more detailed instructions for Gitlab CI and Github Action
July 2: Add Known Issues
July 8: Add codecov and coveralls setup instructions
July 8: Ignore python reports on files inside build/ and devel/

industrial_ci/src/util.sh Outdated Show resolved Hide resolved
@Briancbn Briancbn force-pushed the pr-coverage branch 2 times, most recently from 3f5153e to 1c6bfce Compare June 16, 2020 09:12
@Briancbn
Copy link
Member Author

Briancbn commented Jun 17, 2020

The following tests are conducted on packml_ros2, which is a ROS2 meta package with

  • 1 ament_python package using pytest (launch_testing not supported atm)
  • 5 ament_cmake package using gtest and gmock (only 3 had source and header files)
  • Tested on dashing and eloquent, both main and testing pool

Badges are redirected to builds and reports

There is a difference in how coverage.py is calculated between coveralls.io and codecov.io, I compared the coverage line by line there is no difference.

Coverage Tool Travis Gitlab Github Action
Codecov.io Build Statuscodecov pipeline statuscodecov CIcodecov Cannot identify branch & Cannot open lineview
Coveralls.io Build StatusCoverage Status pipeline statusCoverage Status Cannot identify branch & Cannot open lineview CICoverage Status

@Briancbn Briancbn changed the title WIP: Enable support for code coverage Enable support for code coverage Jun 17, 2020
@Briancbn
Copy link
Member Author

Briancbn commented Jun 17, 2020

The following tests are conducted on RVIP, which has

  • 4 catkin cmake packages
  • ROS 1 melodic
  • catkin_tools, catkin_make, catkin_make_isolated and colcon

Builds
codecov

@Briancbn
Copy link
Member Author

Briancbn commented Jun 17, 2020

Hi @ipa-mdl or anyone with write access
This PR is ready for review!! :):)

Test cases: Since the packages that I tested on are both not fully merged yet, I would like some recommendation regarding the packages to do wrap_test on ideally is

  • 1 ROS2 meta package with multiple ament_cmake and ament_python packages inside. e.g. geometry2
  • 1 ROS1 meta package with multiple cpp and python packages inside.

There are still some more TODOs, I will address in another PR

  • launch_testing and rostest validation (mainly for python)
  • Cannot identify branch & cannot open lineview in coverage report for some CI
  • Documentation (setup and configuration)

industrial_ci/src/coverage/coveralls_docker_env.sh Outdated Show resolved Hide resolved
industrial_ci/src/tests/source_tests.sh Outdated Show resolved Hide resolved
industrial_ci/src/tests/source_tests.sh Outdated Show resolved Hide resolved
@tylerjw
Copy link
Contributor

tylerjw commented Jun 20, 2020

This is really nice. Thank you for doing this. It has been on a long list of things I've wanted to do and your implementation covers more use cases and more features that I would have.

@tylerjw
Copy link
Contributor

tylerjw commented Jun 20, 2020

If you have extra bandwidth on this one more nice feature would be coverage percentage output on local runs (run_ci and rerun_ci). If not, that's not really that important, as I could easily create an AFTER_ script to do that.

@Briancbn
Copy link
Member Author

Briancbn commented Jun 21, 2020

Thank you @tylerjw!! At the moment, a display like the following will still be printed out when CODE_COVERAGE is set to something else, e.g. true or term (that is not coveralls.io or codecov.io). I don't think you will have problem getting these output using run_ci and rerun_ci.

                                               |Lines      |Functions|Branches  
Filename                                       |Rate    Num|Rate  Num|Rate   Num
================================================================================
[/root/target_ws/src/packml_ros2/]
packml_plugin/include/...ugin/packml_plugin.hpp| 0.0%     1| 0.0%   2|    -    0
packml_plugin/include/...ugin/packml_widget.hpp| 0.0%     1| 0.0%   2|    -    0
packml_plugin/src/packml_plugin.cpp            | 0.0%     9| 0.0%   4|    -    0
packml_plugin/src/packml_widget.cpp            | 0.0%   245| 0.0%  16|    -    0
packml_ros/include/packml_ros/packml_ros.hpp   |97.9%   187|75.0%   8|    -    0
packml_ros/src/packml_ros_node.cpp             | 0.0%    11| 0.0%   3|    -    0
packml_sm/include/packml_sm/common.hpp         | 100%     2| 100%   1|    -    0
packml_sm/include/packml_sm/events.hpp         | 100%    30| 100%  18|    -    0
packml_sm/include/packml_sm/state.hpp          |98.4%    64|94.1%  34|    -    0
packml_sm/include/packml_sm/state_machine.hpp  | 100%     8|77.8%   9|    -    0
packml_sm/include/packml_sm/transitions.hpp    | 100%    24| 100%  18|    -    0
packml_sm/src/state.cpp                        | 100%    39| 100%   7|    -    0
packml_sm/src/state_machine.cpp                |97.1%   240|96.8%  31|    -    0
packml_sm/src/transitions.cpp                  | 100%    33| 100%   8|    -    0
================================================================================
                                         Total:|68.8%   894|78.9% 161|    -    0
debconf: delaying package configuration, since apt-utils is not installed
Selecting previously unselected package python3-wheel.
(Reading database ... 61805 files and directories currently installed.)
Preparing to unpack .../python3-wheel_0.30.0-0.2_all.deb ...
Unpacking python3-wheel (0.30.0-0.2) ...
Setting up python3-wheel (0.30.0-0.2) ...
Name                                                           Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------------------------------
src/packml_ros2/packml_plc/packml_plc/__init__.py                  0      0      0      0   100%
src/packml_ros2/packml_plc/packml_plc/packml_plc_listener.py     155      3     38     18    89%
src/packml_ros2/packml_plc/packml_plc/packml_plc_sender.py       100      9     26      2    91%
------------------------------------------------------------------------------------------------
TOTAL                                                            255     12     64     20    90%

Unfortunately, the output are not combined into one, coveralls comes with merging capability but unfortunately cannot display final .json file in terminal.

Instead of lcov in the current implementation, I will try to use gcovr to do cpp coverage report generation, ignoring, merging and terminal display before uploading to the reporting tools, which will introduce a better and more parameterized ignore and coverage percentage acceptance setting.

Update: I opened a ticket colcon/colcon-core#359 regarding this to make a combined coverage, but I think what we have now is good for most packages and implementation.

Copy link
Member

@mathias-luedtke mathias-luedtke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all: very impressive!

There are some bigger and smaller issues, which need to be addressed before we can merge this.
For now I'd like to focus on the biggest..

This PR contains two parts: report generation and report uploading.
The generation part (including the first half of upload_coverage_report) looks file so far, but the upload is hard to maintain this way..

Instead of uploading everything during the build, it would be easier to do it outside of Docker.
The upload from within the build might even create problems with local testing.
Please try to move the upload handling out of the build.
Some CI services might support this out of the box anyway, including the handling of credentials.

@Briancbn Briancbn force-pushed the pr-coverage branch 2 times, most recently from f4849ff to b703795 Compare June 24, 2020 10:12
@Briancbn
Copy link
Member Author

Briancbn commented Jun 24, 2020

Thanks @ipa-mdl

Instead of uploading everything during the build, it would be easier to do it outside of Docker.
The upload from within the build might even create problems with local testing.
Please try to move the upload handling out of the build.

So now I am exposing the report files onto $TARGET_REPO_PATH/.ici_coverage_report/ and process them outside of the docker

# mount coverage report folder
if [ "$CODE_COVERAGE" ]; then
run_opts+=(-v "$TARGET_REPO_PATH/.ici_coverage_report:/root/.ici_coverage_report" -e "COVERAGE_REPORT_PATH=/root/.ici_coverage_report")
fi

The output files are respectively

  • For coveralls.io
    • coverage.json to use by python-coveralls for upload
  • For codecov and anything else
    • coverage.xml (Cobertura XML file containing coverage report for python packages)
    • coverage.info (LCOV file containing coverage report for cpp packages)

It seems to me, to upload report, the only way currently is to put them at the end of the entrypoint.
For example for Travis is like the following. I am still testing the rest (Should be more or less the same).
Please let me know if there is a better way than this.

industrial_ci/travis.sh

Lines 45 to 47 in 5b0bbc7

if [ -n "$CODE_COVERAGE" ]; then
bash "$DIR_THIS"/industrial_ci/src/upload_coverage_report.sh
fi

Some CI services might support this out of the box anyway, including the handling of credentials.

CODE_COVERAGE can also be set to anything else like true or term to only expose the report files and allow users to decide how to deal with the coverage report, e.g. using existing coverage functionality from Github Action Apps or Gitlab artifact.

Lastly, thanks to @rarrais 's work on ros_coverage. I have added additional improvement to support ROS1 python2 and python3 packages with unittests or nosetests on catkin_tools and colcon, by setting CATKIN_WITH_COVERAGE env variable to 1. (Note: This doesn't work on catkin_make and catkin_make_isolated)

@Briancbn Briancbn requested a review from mathias-luedtke July 2, 2020 02:10
agutenkunst added a commit to agutenkunst/pilz_robots that referenced this pull request Jul 7, 2020
@agutenkunst
Copy link
Contributor

agutenkunst commented Jul 7, 2020

Update: Now working see https://codecov.io/gh/agutenkunst/minimal_test_package/tree/b362893734dfa87fc738b51fbd0f0fcf17b695e9/src

Hi @Briancbn nice work! Looking forward to this!

I setup https://github.com/agutenkunst/minimal_test_package in order to check this (using ROS1 gtest + nosetest)

It failed with

==> Uploading reports
    url: https://codecov.io
    query: branch=master&commit=768ad75259b493084cf6d31e7d01c93467af291d&build=1.1&build_url=&name=&tag=&slug=agutenkunst%2Fminimal_test_package&service=travis&flags=&pr=false&job=358408309
->  Pinging Codecov
https://codecov.io/upload/v4?package=bash-20200629-ffaf297&token=secret&branch=master&commit=768ad75259b493084cf6d31e7d01c93467af291d&build=1.1&build_url=&name=&tag=&slug=agutenkunst%2Fminimal_test_package&service=travis&flags=&pr=false&job=358408309
->  Uploading to
curl: (3) <url> malformed
    X> Failed to upload
    -> Sleeping for 30s and trying again...
/dev/fd/63: line 1668: [: 0+1: integer expression expected
==> Uploading to Codecov
curl: (3) <url> malformed
curl: (3) <url> malformed
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:--  0:02:06 --:--:--     0curl: (7) Failed to connect to codecov.io port 443: Connection timed out
    -> Sleeping for 30s and trying again...
/dev/fd/63: line 1721: [: 0+1: integer expression expected
    X> Failed to upload coverage reports
The command "bash <(curl -s https://codecov.io/bash) -Z" exited with 1.

see https://travis-ci.com/github/agutenkunst/minimal_test_package/builds/174644770

Could very well be my mistake. Any ideas?

@agutenkunst
Copy link
Contributor

Ok coveralls.io fails with

requests.exceptions.SSLError: [Errno 1] _ssl.c:510: error:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert handshake failure

see https://travis-ci.com/github/agutenkunst/minimal_test_package/builds/174654480

Did you experience somethink like this before?

@agutenkunst
Copy link
Contributor

ROS1 Python Coveralls: Cannot ignore init.py file in the build/ directory

Could you provide your insights on this?

@Briancbn
Copy link
Member Author

Briancbn commented Jul 8, 2020

Thanks so much @agutenkunst for taking the time to test this PR:)

see https://travis-ci.com/github/agutenkunst/minimal_test_package/builds/174654480

Did you experience somethink like this before?

This is due to not enabling coveralls.io for your package.
See https://docs.travis-ci.com/user/coveralls/#1-add-your-repository-to-coveralls
I will also do a pip upgrade first seeing so many warnings in your recent build

Update: Now working see https://codecov.io/gh/agutenkunst/minimal_test_package/tree/b362893734dfa87fc738b51fbd0f0fcf17b695e9/src

Great! How did you resolve the problem?

ROS1 Python Coveralls: Cannot ignore init.py file in the build/ directory

Could you provide your insights on this?

So catkin_python_setup() will generate a __init__.py file inside your build directory and will be counted as a coverage file (missed 1 line by default as well!!!). Now I find it also affects codecov.io under ROS1 colcon builder.


Update:
I have address this in the latest commit (also rebased to latest to avoid shellcheck error).

@agutenkunst
Copy link
Contributor

So currently


codecov.io works (https://codecov.io/gh/agutenkunst/minimal_test_package/tree/b362893734dfa87fc738b51fbd0f0fcf17b695e9/src) (seemed to fail just once and worked on restarting the job, but since I clone your fork by branch name and not commit it is hard to recap)


coveralls.io
Still fails with

requests.exceptions.SSLError: [Errno 1] _ssl.c:510: error:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert handshake failure

even after I enabled the project on coveralls.io

see https://travis-ci.com/github/agutenkunst/minimal_test_package/builds/174654480 for full output.

@Briancbn
Copy link
Member Author

Briancbn commented Jul 9, 2020

Sorry about misleading earlier, I have found out that this is due to the Travis linux distro is trusty, if you remove that line, the build will succeed under xenial or if you use bionic, here is my log. It is strange that trusty's urlib3 will cause the SSLInsecurePlatform warning. Even the sudo apt update added in #545 is giving error in your log, which might have the previous issue #542 with Foxy as well

The xenial images by default also only provide python2, which might be a problem in the future. However, I am also reluctant to put in extra python3 installation script inside the install tag.


Update: https://urllib3.readthedocs.io/en/latest/user-guide.html#ssl-py2 This is the solution to that warning. I will see whether adding it in will break anything.

agutenkunst added a commit to agutenkunst/minimal_test_package that referenced this pull request Jul 9, 2020
@Briancbn
Copy link
Member Author

Briancbn commented Dec 16, 2020

I might be better to take out the upload steps completely and just delegate it to the outer job of the CI service.

I have moved the upload script to somewhere after docker_commit. I think this is be better than ci_main.sh and outside of ci_main.sh shell.
Things left to do.

  • Update Documentation
  • Mock test cases
  • Test in Travis CI (updates are tested in Github Actions and Gitlab CI. I ran out of credits for Travis)

@tylerjw
Copy link
Contributor

tylerjw commented Mar 21, 2021

Is there any way I can help with this. I think this is one of the last things we need before we can use industrial_ci for moveit on github actions.

@mathias-luedtke mathias-luedtke mentioned this pull request Mar 22, 2021
@mathias-luedtke
Copy link
Member

mathias-luedtke commented Mar 22, 2021

Is there any way I can help with this.

@tylerjw: For now it would be great, if you could test it and give some feedback.

@Briancbn: Please rebase your PR to master. I will review it again afterwards.

@tylerjw
Copy link
Contributor

tylerjw commented Mar 22, 2021

I tried it and wasn't able to get it to work for me. However I think that might be because I'm using BASEDIR now and that feature was merged later (and is probably part of the conflicts).

One thing I was wondering is does this PR do the filtering out of test files? How about if someone gives a repo file for the TARGET_WORKSPACE, will it filter the results to just the repo being tested. Here is the script I wrote that I'm using for now to prepare my coverage report until I can use this PR:

#!/bin/bash
pushd $BASEDIR

BLUE='\033[0;34m'
NOCOLOR='\033[0m'

echo -e "${BLUE}Capture coverage info${NOCOLOR}"
lcov --capture --directory target_ws --output-file coverage.info

echo -e "${BLUE}Extract repository files${NOCOLOR}"
lcov --extract coverage.info "$BASEDIR/target_ws/src/$TARGET_REPO_NAME/*" --output-file coverage.info

echo -e "${BLUE}Filter out test files${NOCOLOR}"
lcov --remove coverage.info '*/test/*' --output-file coverage.info

echo -e "${BLUE}Output coverage data for debugging${NOCOLOR}" 
lcov --list coverage.info

popd

@Briancbn
Copy link
Member Author

Is there any way I can help with this. I think this is one of the last things we need before we can use industrial_ci for moveit on github actions.

Hi @tylerjw, sorry I will only be able to follow up on this after March ends,..qaq. I can add you as collaborator if you would like to contribute to this PR?

One thing I was wondering is does this PR do the filtering out of test files?

It does filter out the test files for both python and C++
https://github.com/Briancbn/industrial_ci/blob/00063eeb6cfb70c879af48b1298846336e04bd6e/industrial_ci/src/coverage.sh#L38-L40

will it filter the results to just the repo being tested.

No, it doesn't. In fact, if you don't collect the initial zero report, files that hasn't been tested (no report) will not be included in the coverage -- which technically is an overestimation of the coverage data.

  # Capture initial coverage info
  lcov --capture --initial \
       --directory build \
       --output-file initial_coverage.info | grep -ve "^Processing"

I have another branch to address non-conventional ignore of both relative and absolute paths. Briancbn#1 (comment) will try to include it after I have some more time.

Briancbn and others added 10 commits December 8, 2021 04:51
Signed-off-by: Chen Bainian <[email protected]>

Make python coverage only available to colcon in ROS2

Signed-off-by: Chen Bainian <[email protected]>

Revert mercurial setup changes

Signed-off-by: Chen Bainian <[email protected]>

Fix curl not found in ROS1

Signed-off-by: Chen Bainian <[email protected]>

temp

Signed-off-by: Chen Bainian <[email protected]>

Add lcov and find report command

Signed-off-by: Chen Bainian <[email protected]>

Add in coveralls.io option

Signed-off-by: Chen Bainian <[email protected]>

Fix .coverage not found

Signed-off-by: Chen Bainian <[email protected]>

Fix coveralls not found

Signed-off-by: Chen Bainian <[email protected]>

Fix wrong .coverage path

Signed-off-by: Chen Bainian <[email protected]>

Use coveralls-merge

Signed-off-by: Chen Bainian <[email protected]>

Remove psutils installation

Signed-off-by: Chen Bainian <[email protected]>

Use coveragepy combine report

Signed-off-by: Chen Bainian <[email protected]>

Use coveragerc for files ignore

Signed-off-by: Chen Bainian <[email protected]>

Use coveralls specific environment variables

Signed-off-by: Chen Bainian <[email protected]>

Avoid exit for pure c++ or python package

Signed-off-by: Chen Bainian <[email protected]>

Avoid coveragepy report error

Signed-off-by: Chen Bainian <[email protected]>

Use target repo name

Signed-off-by: Chen Bainian <[email protected]>

Force Debug build during coverage build

Signed-off-by: Chen Bainian <[email protected]>

Expose .ici_coverage_report for coverage reports

Signed-off-by: Chen Bainian <[email protected]>

Run cd in subshell and expose coverage report files

Signed-off-by: Chen Bainian <[email protected]>

Some sed magic to remove identifiable absolute path

Signed-off-by: Chen Bainian <[email protected]>

Add support for ROS1 python unittest, Credit: @rarrais

Signed-off-by: Chen Bainian <[email protected]>

Move coveralls report merging inside, expose only merged json file

Signed-off-by: Chen Bainian <[email protected]>

Use CODECOV env variable directly in function

Signed-off-by: Chen Bainian <[email protected]>

Sample travis report upload script

Signed-off-by: Chen Bainian <[email protected]>

Fix typo on sample travis report upload script

Signed-off-by: Chen Bainian <[email protected]>

Remove redundant files

Signed-off-by: Chen Bainian <[email protected]>

Display coveragepy installation elegantly

Signed-off-by: Chen Bainian <[email protected]>

Add Gitlab support

Signed-off-by: Chen Bainian <[email protected]>
Fix some wrong versions of python

Signed-off-by: Chen Bainian <[email protected]>

Add Github Action coverage support

Signed-off-by: Chen Bainian <[email protected]>

Upgrade pip first

Signed-off-by: Chen Bainian <[email protected]>

Python report ignore everything from build and devel

Signed-off-by: Chen Bainian <[email protected]>

Wrong! Ignore devel instead

Signed-off-by: Chen Bainian <[email protected]>

Python ignore everything from build and devel

Signed-off-by: Chen Bainian <[email protected]>

Use include pattern instead of omit

Signed-off-by: Chen Bainian <[email protected]>

Update author copyrights

Signed-off-by: Chen Bainian <[email protected]>

Move coverage helper function into coverage.sh

Signed-off-by: Chen Bainian <[email protected]>

Streamline coverage upload script

Signed-off-by: Chen Bainian <[email protected]>

Abstract python pip installation

install setup tool as well

Move code coverage inside docker script
Signed-off-by: Chen Bainian <[email protected]>
@InvincibleRMC
Copy link

This branch seems stale. Is there any way I can add the code coverage to my ros industrial Github action?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants