-
Notifications
You must be signed in to change notification settings - Fork 299
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
[CI] Add coveragepy and remove ignore: test #1668
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1668 +/- ##
==========================================
+ Coverage 84.59% 86.73% +2.13%
==========================================
Files 115 115
Lines 10537 10539 +2
Branches 972 972
==========================================
+ Hits 8914 9141 +227
+ Misses 1295 1052 -243
- Partials 328 346 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
|
6566686
to
bf4c4e3
Compare
This pull request is in conflict. Could you fix it @christophfroehlich? |
This reverts commit a795f9f.
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
0d9ff6a
to
7fa5b1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alles gut
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Recently we worked a lot on the spawner scripts, maybe we should add coverage data also for them.
The only problem is that we don't test them with pytest but with system calls from C++ files. This needs to manually invoke coverage.py with the python modules by the
std::system
calls instead of usingros2 run
Most of the necessary changes are ros-controls/ros2_control_ci#115
Recently, the codecov setting to ignore the test folders did not work and they got included (which increased our coverage numbers). I removed the ignore line, and intentional add the tests to the coverage where there are good arguments for it anyways.