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

Update focal to use gcc9 #50

Merged
merged 1 commit into from Apr 7, 2022
Merged

Update focal to use gcc9 #50

merged 1 commit into from Apr 7, 2022

Conversation

mjcarroll
Copy link
Contributor

Signed-off-by: Michael Carroll michael@openrobotics.org

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@chapulina
Copy link
Contributor

I believe the test run in gazebosim/gz-fuel-tools#230 didn't have coverage enabled, it would be nice to test that LCOV is working as expected with this change

@scpeters
Copy link
Contributor

scpeters commented Apr 6, 2022

@chapulina
Copy link
Contributor

Has anyone triggered a test run using this branch which has coverage enabled? I just wanna make sure we don't break the coverage jobs

@scpeters
Copy link
Contributor

scpeters commented Apr 7, 2022

Has anyone triggered a test run using this branch which has coverage enabled? I just wanna make sure we don't break the coverage jobs

trying it now with gazebosim/gz-math@1f6703c

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

The test run was successful!

@chapulina chapulina merged commit 3c6839d into focal Apr 7, 2022
@chapulina chapulina deleted the focal_gcc9 branch April 7, 2022 21:49
@azeey
Copy link
Contributor

azeey commented May 2, 2022

Coverage seems to be broken in some (if not all) our libraries. In sdformat PR #939, for example, the number of tracked files is now just 2:
image
whereas, about a month ago (eg gazebosim/sdformat#909), the number of files was 69:
image

I suspect removing the lcov related lines might have caused the issue, but I'm not 100% sure.

@scpeters
Copy link
Contributor

scpeters commented May 2, 2022

Has anyone triggered a test run using this branch which has coverage enabled? I just wanna make sure we don't break the coverage jobs

trying it now with ignitionrobotics/ign-math@1f6703c

it looks like the GitHub action didn't passed the status check but didn't generate a proper coverage report

@azeey
Copy link
Contributor

azeey commented May 2, 2022

it looks like the GitHub action didn't passed the status check but didn't generate a proper coverage report

https://app.codecov.io/gh/ignitionrobotics/ign-math/branch/scpeters%2Faction_focal_gcc9

That actually looks okay to me (https://codecov.io/gh/ignitionrobotics/ign-math/tree/1f6703cd08f2c4685ae1c0b3ecddb5df1a300841). The project totals show 70 files, which is 2 more than a random PR I picked from 2 moths ago.

@azeey
Copy link
Contributor

azeey commented May 3, 2022

Okay, this might actually be caused by the Checkout action due to actions/checkout#764. I think what's happening is that codecov tries to determine the project root using git (https://github.com/codecov/codecov-bash/blob/1817ab23d6b35ff6596e48d15db366b71f17ada3/codecov#L48), but fails due to how the repo is checkout as of actions/checkout#764. We can see this when comparing the console output of codecov for a failed and successful report generation:

Failed report generation (https://github.com/ignitionrobotics/sdformat/runs/6244787092?check_suite_focus=true)

  current dir:  /github/workspace/build
  project root: .

Successful report generation (https://github.com/ignitionrobotics/sdformat/runs/5565309483?check_suite_focus=true)

  current dir:  /github/workspace/build
  project root: /github/workspace

My suggestion would be to cd into the root and run codecov from there with a flag to tell it where to find the coverage.info file.

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.

None yet

4 participants