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

Add coveralls badge to README #53

Merged
merged 1 commit into from
Mar 19, 2023
Merged

Add coveralls badge to README #53

merged 1 commit into from
Mar 19, 2023

Conversation

cljoly
Copy link
Owner

@cljoly cljoly commented Mar 18, 2023

No description provided.

@coveralls
Copy link

coveralls commented Mar 18, 2023

Coverage Status

coverage: 88.589%. remained the same
when pulling 8069481 on add-coveralls-badge
into f3c45cd on master.

@cljoly
Copy link
Owner Author

cljoly commented Mar 18, 2023

I’m a bit disappointed by the whole coverage thing: it does not include doc-tests (would need the nightly compiler and it’s unstable taiki-e/cargo-llvm-cov#2) nor the integration tests (because they are in a separate binary I suspect).

In its current state, it’s a bit misleading. I’m thinking of showing coverage only for the code that’s not behind features and to move back the integration tests that don’t need features in the standard directory.

@cljoly
Copy link
Owner Author

cljoly commented Mar 18, 2023

cc @czocher

@czocher
Copy link
Collaborator

czocher commented Mar 18, 2023

It does not run the doctests, but from what I see it does run the integration tests:

Running tests/integration_test.rs (target/llvm-cov-target/debug/deps/integration_test-98686e68c83a8c03)

running 1 test

We can use the nigthly unstable feature for the doctest support since it does not influence the project, just the CI, which we do not guarantee anything about. What do you think @cljoly?

@cljoly
Copy link
Owner Author

cljoly commented Mar 18, 2023

You are right I looked too quickly, integration tests are run indeed. And we can give a try to the nightly CI builds, if it turn out to be too unstable (i.e. if it fails on valid code too often), we can always go back to stable.

@cljoly
Copy link
Owner Author

cljoly commented Mar 18, 2023

Hum, so with nightly / the doctest option, coveralls does not get the sources: https://coveralls.io/builds/57998356/source?filename=rusqlite_migration%2Fsrc%2Fasynch.rs.

image

That’s annoying. I guess if I have to chose, I prefer to have a slightly lower coverage but an easy access to uncovered lines for contributors who may not want to go through all the trouble of installing cargo-llvm-cov and running it locally.

@cljoly
Copy link
Owner Author

cljoly commented Mar 18, 2023

Maybe codecov.io would work better on this, but they seem very confused by the fact that I renamed this repo years ago.

@cljoly
Copy link
Owner Author

cljoly commented Mar 19, 2023

Yeah, the online report on coverage for each file line comes back once I removed the doc-test/nightly stuff.

image

@cljoly cljoly force-pushed the add-coveralls-badge branch 3 times, most recently from 2f57b5d to 0ddf90a Compare March 19, 2023 00:23
@czocher
Copy link
Collaborator

czocher commented Mar 19, 2023

@cljoly what's the story with tarpaulin and doctests? I noticed the issue they created for them seems solved: xd009642/tarpaulin#13

If it supports them we can migrate easily.

@cljoly
Copy link
Owner Author

cljoly commented Mar 19, 2023

Yes, that’s a good point, perhaps it won’t be too slow and it’ll work better with the doc-tests. Could you please try to migrate to tarpaulin, if you have time?

@czocher
Copy link
Collaborator

czocher commented Mar 19, 2023

@cljoly sure, I'll implement it later today.

@cljoly
Copy link
Owner Author

cljoly commented Mar 19, 2023

Great thank you.

@czocher
Copy link
Collaborator

czocher commented Mar 19, 2023

@cljoly see #57.

@cljoly cljoly merged commit be0834c into master Mar 19, 2023
@cljoly cljoly deleted the add-coveralls-badge branch March 19, 2023 17:08
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