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

[CI] Use wf from ros2_control_ci for coverage build #188

Merged

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Feb 18, 2024

Reusable workflow was added with ros-controls/ros2_control_ci#12

Together with #181 this closes #111

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.10%. Comparing base (759d954) to head (8daccf7).

Additional details and impacted files
@@             Coverage Diff              @@
##           ros2-master     #188   +/-   ##
============================================
  Coverage        50.10%   50.10%           
============================================
  Files               10       10           
  Lines              497      497           
  Branches           166      167    +1     
============================================
  Hits               249      249           
  Misses             217      217           
  Partials            31       31           
Flag Coverage Δ
unittests 50.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Seems fine the failing lottery seems unrelated.

@christophfroehlich
Copy link
Contributor Author

Seems fine the failing lottery seems unrelated.

I'm not sure about this token now, the coverage workflow fails mentioning a missing codecov token, and it didn't get updated since the first commit in this PR :/

@fmauch
Copy link

fmauch commented Feb 21, 2024

Seems fine the failing lottery seems unrelated.

I'm not sure about this token now, the coverage workflow fails mentioning a missing codecov token, and it didn't get updated since the first commit in this PR :/

hm, you're right. The docs seem to be pretty clear about tokens, btw. Maybe we use v3 in those repos where it works without a token?

I am wondering why our CI job isn't failing. This one (in my own repo) does.

@christophfroehlich
Copy link
Contributor Author

Seems fine the failing lottery seems unrelated.

I'm not sure about this token now, the coverage workflow fails mentioning a missing codecov token, and it didn't get updated since the first commit in this PR :/

hm, you're right. The docs seem to be pretty clear about tokens, btw. Maybe we use v3 in those repos where it works without a token?

I'm not sure why it doesn't work from my fork then (or why it worked only once to be precise). I'll open this PR in another repo and see what happens there. Maybe one from a fork and one from a branch of the repo itself.

No, we use v4
https://github.com/ros-controls/ros2_control/blob/c4affe4bbfbb12b947acfc7b268f529724e0aae8/.github/workflows/ci-coverage-build.yml#L45-L49

but we have the error there as well:
https://github.com/ros-controls/ros2_control/actions/runs/7964042229/job/21740799081

I'll have a look how to properly pass the secret, but it didn't work out when I tried it the first time.

I am wondering why our CI job isn't failing. This one (in my own repo) does.

Thanks for fixing this.

This reverts commit 60dfa5f.
@christophfroehlich
Copy link
Contributor Author

Something is wrong with the lcov/codecov paths, it now includes the header from generate_parameter_library
image

@christophfroehlich christophfroehlich marked this pull request as draft February 27, 2024 19:17
@christophfroehlich
Copy link
Contributor Author

Seems to be fixed with ros-controls/ros2_control_ci#20, waiting for the merge
image

@christophfroehlich christophfroehlich marked this pull request as ready for review February 28, 2024 19:12
@bmagyar bmagyar merged commit c78c686 into ros-controls:ros2-master Mar 4, 2024
19 checks passed
@christophfroehlich christophfroehlich deleted the ci_coverage_build branch April 4, 2024 19:31
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.

Merge CI job descriptions into same file
4 participants