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 coverage #829

Merged
merged 1 commit into from
Jun 28, 2022
Merged

enable coverage #829

merged 1 commit into from
Jun 28, 2022

Conversation

Emilgardis
Copy link
Member

@Emilgardis Emilgardis commented Jun 21, 2022

Implements coverage via lcov artifacts and uploads them to https://app.codecov.io/gh/cross-rs/cross

.github/actions/cargo-llvm-cov/action.yml Outdated Show resolved Hide resolved
.github/actions/setup-rust/action.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Emilgardis Emilgardis force-pushed the feature/cov branch 7 times, most recently from 7e99832 to 502e1ac Compare June 22, 2022 01:48
@Emilgardis
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jun 22, 2022
@bors
Copy link
Contributor

bors bot commented Jun 22, 2022

try

Build succeeded:

@Emilgardis Emilgardis force-pushed the feature/cov branch 2 times, most recently from ae39f85 to ed2c004 Compare June 22, 2022 09:00
@Emilgardis Emilgardis marked this pull request as ready for review June 22, 2022 09:00
@Emilgardis Emilgardis requested a review from a team as a code owner June 22, 2022 09:00
@Emilgardis Emilgardis force-pushed the feature/cov branch 2 times, most recently from 8974a42 to b175434 Compare June 22, 2022 14:44
find ${artifacts} -name "lcov.*.info" -exec ./codecov -F $(echo {} | sed -n 's/lcov\.\(.*\)\.info/\1/p') \
${pr:+-P ${pr}} -f {} --sha ${sha_rev} -n $(echo {} | sed -n 's/lcov\.\(.*\)\.info/\1/p') \;
Copy link
Member Author

Choose a reason for hiding this comment

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

we may have to specify parent sha here, not sure if it really works when you have a staging/trying branch.

been trying to find prior art for codecov with bors integration but not able to find any good examples, only ones I find are my own previous tries :D

CHANGELOG.md Outdated Show resolved Hide resolved
@Emilgardis Emilgardis force-pushed the feature/cov branch 4 times, most recently from 2f4d6ac to 2a3fcc4 Compare June 23, 2022 23:12
Comment on lines +368 to +369
find ${artifacts} -name "lcov.*.info" -exec ./codecov -F $(echo {} | sed -n 's/lcov\.\(.*\)\.info/\1/p') \
${pr:+-P ${pr}} -f {} --sha ${sha_rev} -n $(echo {} | sed -n 's/lcov\.\(.*\)\.info/\1/p') \;
Copy link
Member Author

Choose a reason for hiding this comment

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

I want this to never be able to fail, but on fail echo ::error title=Coverage upload failed::

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know how to do that well though

@Alexhuszagh
Copy link
Contributor

Fix the changelog and r=me whenever.

@Emilgardis Emilgardis added the no changelog A valid PR without changelog (no-changelog) label Jun 28, 2022
@Alexhuszagh
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Jun 28, 2022
829: enable coverage r=Alexhuszagh a=Emilgardis

Implements coverage via lcov artifacts and uploads them to https://app.codecov.io/gh/cross-rs/cross

Co-authored-by: Emil Gardström <[email protected]>
Copy link
Contributor

@Alexhuszagh Alexhuszagh left a comment

Choose a reason for hiding this comment

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

Looking at the merge attempt, we'll likely need to export if the target supports LLVM code coverage. It may be worth just doing it on x86_64-unknown-linux-gnu.

id: cov
uses: ./.github/actions/cargo-llvm-cov
if: steps.prepare-meta.outputs.has-image
with:
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably change to:

if: steps.prepare-meta.outputs.has-image && matrix.target == 'x86_64-unknown-linux-gnu'

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need (or want) coverage on the test binaries, fixed this by clearing RUSTFLAGS, maybe it should be more specific though (i.e only remove instrument-coverage but not sure how to do that well)

@bors
Copy link
Contributor

bors bot commented Jun 28, 2022

Build failed:

@Emilgardis
Copy link
Member Author

bors try --target mips64-unknown-linux-muslabi64

bors bot added a commit that referenced this pull request Jun 28, 2022
@bors
Copy link
Contributor

bors bot commented Jun 28, 2022

try

Build succeeded:

@Alexhuszagh
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 28, 2022

Build succeeded:

@bors bors bot merged commit 00fe87b into cross-rs:main Jun 28, 2022
@Emilgardis Emilgardis added this to the v0.2.3 milestone Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog A valid PR without changelog (no-changelog)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants