Skip to content

Conversation

@scouten-adobe
Copy link

As a user of this crate, I'd like to know how well this code is covered by its own self tests.

I'm certainly open to alternatives to CodeCov, but I'd like to see something that shows active development of tests designed to assert the overall reliability of this code and show coverage of edge cases.

I've commented out two pieces that will require intervention from the SpruceID team:

  1. Support for WASM code coverage is commented out until WASM builds are viable. (See feat: Support wasm32-wasi #544.)
  2. Reliability of the upload service will improve once a CODE_COV token is provided. This has to be done by the repo owner.

@CLAassistant
Copy link

CLAassistant commented Aug 29, 2024

CLA assistant check
All committers have signed the CLA.

@sbihel
Copy link
Member

sbihel commented Aug 30, 2024

Before we look further into this, I'd like to ask what your experience with code coverage in Rust has been like? When I looked into it a few years ago, it seemed like the tools available then weren't able to accurately track coverage. Which means the resulting percentage wasn't very meaningful.

I personally only know one significant library that uses CodeCov, with openidconnect. And looking at the report, it seems a bit incoherent.

I'm asking because whilst I agree that this library could use with more tests and documentation, I don't want us to chase unreachable targets, or have a coverage number reflect badly on us when it's out of our control.

@scouten-adobe
Copy link
Author

@sbihel sorry I didn't see this sooner. I'll point you to two projects I maintain or co-maintain where we have used CodeCov to good effect:

One thing I've noted (you'll see it in XMP Toolkit, but not in c2pa-rs) is that it seems to help to have the #[test] files separated from the source files; otherwise the test code gets factored into the coverage reports and I consider that misleading.

I have done some preliminary experiments with WASM based coverage and expect to be able to give a positive report on that before long. (Ironically, part of that is blocked waiting on #544 or something similar to land.)

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.

3 participants