-
Notifications
You must be signed in to change notification settings - Fork 212
run coverage before doc-build so it doesn't delete the docs that were just built #1392
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
Conversation
… just built related issue: rust-lang/cargo#9447
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a good approach until the upstream bug is fixed. Thanks!
// we have to run coverage before the doc-build because currently it | ||
// deletes the doc-target folder. | ||
// https://github.com/rust-lang/cargo/issues/9447 | ||
let doc_coverage = match self.get_coverage(target, build, metadata, limits) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind adding a build test that coverage is generated? Maybe it could look at the /crate page and see if there's a % sign somewhere (see https://docs.rs/crate/tokio/1.5.0 for an example of what that looks like).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a test.
Instead of checking the HTML output I would propose we just check if the coverage is in the database, which is good enough to know if the build with coverage worked, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a better test :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jyn514 I added the assertion to the build-test
387234a
to
0cde41c
Compare
related issue: rust-lang/cargo#9447