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.

Sign up for GitHub

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

Jump to bottom

fix: report line coverage instead of function coverage #171

Merged
merged 1 commit into from
Oct 23, 2024
Merged

fix: report line coverage instead of function coverage #171

merged 1 commit into from
Oct 23, 2024

Conversation

roypat
Copy link
Collaborator

@roypat roypat commented Oct 23, 2024

It seems that the raw output of llvm-cov changed at some point, and we ended upcomparing functoin coverage instead of line coverage. Fix this by instead using the new json output, which will hopefully prevent such goofs in the future.

Fixes #170
Signed-off-by: Patrick Roy roypat@amazon.co.uk

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@roypat
fix: report line coverage instead of function coverage
de58af0 
It seems that the raw output of llvm-cov changed at some point, and we
ended upcomparing functoin coverage instead of line coverage. Fix this
by instead using the new json output, which will hopefully prevent such
goofs in the future.

Fixes rust-vmm#170
Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
@roypat
Copy link
Collaborator Author

roypat commented Oct 23, 2024

Maybe we could also talk about getting rid about these coverage files altogether and instead using codecov.io at some point? We did that at Firecracker a year ago or so

@stefano-garzarella
Copy link
Member

Maybe we could also talk about getting rid about these coverage files altogether and instead using codecov.io at some point? We did that at Firecracker a year ago or so

Yeah, can you open an issue, so if someone has free cycle can look at it.

@roypat
Copy link
Collaborator Author

roypat commented Oct 23, 2024

Maybe we could also talk about getting rid about these coverage files altogether and instead using codecov.io at some point? We did that at Firecracker a year ago or so

Yeah, can you open an issue, so if someone has free cycle can look at it.

I've opened rust-vmm/community#183 :)

@roypat roypat merged commit 1150c47 into rust-vmm:main Oct 23, 2024
3 checks passed
@stefano-garzarella stefano-garzarella mentioned this pull request Oct 25, 2024
Merged
stefano-garzarella pushed a commit to rust-vmm/vhost that referenced this pull request Oct 25, 2024
@dependabot @stefano-garzarella
build(deps): bump rust-vmm-ci from 209c04e to 1150c47
ef12554 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `209c04e` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@209c04e...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
stefano-garzarella pushed a commit to rust-vmm/vhost that referenced this pull request Oct 25, 2024
@dependabot @stefano-garzarella
build(deps): bump rust-vmm-ci from 209c04e to 1150c47
a573574 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `209c04e` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@209c04e...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
stefano-garzarella pushed a commit to rust-vmm/vhost-device that referenced this pull request Oct 28, 2024
@dependabot @stefano-garzarella
build(deps): bump rust-vmm-ci from 2122417 to 1150c47
88a4147 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `2122417` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@2122417...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
stefano-garzarella pushed a commit to rust-vmm/vhost-device that referenced this pull request Oct 28, 2024
@dependabot @stefano-garzarella
build(deps): bump rust-vmm-ci from 2122417 to 1150c47
fe02fcd 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `2122417` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@2122417...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
@stefano-garzarella stefano-garzarella mentioned this pull request Oct 29, 2024
Merged
stefano-garzarella pushed a commit to rust-vmm/vm-virtio that referenced this pull request Oct 29, 2024
@dependabot @stefano-garzarella
build(deps): bump rust-vmm-ci from 209c04e to 1150c47
a517012 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `209c04e` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@209c04e...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
epilys pushed a commit to rust-vmm/vm-virtio that referenced this pull request Oct 29, 2024
@dependabot @epilys
build(deps): bump rust-vmm-ci from 209c04e to 1150c47
adbf691 
Bumps [rust-vmm-ci](https://github.com/rust-vmm/rust-vmm-ci) from `209c04e` to `1150c47`.
- [Commits](rust-vmm/rust-vmm-ci@209c04e...1150c47)

---
updated-dependencies:
- dependency-name: rust-vmm-ci
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

[SG] Updated coverage score since we fixed its calculation, see
rust-vmm/rust-vmm-ci#171

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShadowCurse ShadowCurse ShadowCurse approved these changes

@stefano-garzarella stefano-garzarella stefano-garzarella approved these changes

@andreeaflorescu andreeaflorescu Awaiting requested review from andreeaflorescu andreeaflorescu is a code owner

@lauralt lauralt Awaiting requested review from lauralt lauralt is a code owner

@sameo sameo Awaiting requested review from sameo sameo is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is using "function coverage" metric instead of "line coverage" intended?
3 participants
@roypat @stefano-garzarella @ShadowCurse