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

Short-term fix to make coverage CI check pass #316

Closed
echeran opened this issue Oct 9, 2020 · 8 comments · Fixed by #331
Closed

Short-term fix to make coverage CI check pass #316

echeran opened this issue Oct 9, 2020 · 8 comments · Fixed by #331
Assignees
Labels
C-test-infra Component: Integration test infrastructure T-bug Type: Bad behavior, security, privacy
Milestone

Comments

@echeran
Copy link
Contributor

echeran commented Oct 9, 2020

In order to immediately unblock coverage checks, we should remove caching from the coverage checks in CI. (ex: #312)

The longer-term solution is represented separately in #315.

@echeran echeran added the C-test-infra Component: Integration test infrastructure label Oct 9, 2020
@echeran echeran self-assigned this Oct 9, 2020
@zbraniecki
Copy link
Member

that has been merged now.

@echeran echeran changed the title Remove caching from coverage CI check in short-term Short-term fix to make coverage CI check pass Oct 13, 2020
@echeran
Copy link
Contributor Author

echeran commented Oct 13, 2020

The PR to remove caching from the CI coverage checks has been merged. Even though we thought that it would fix the immediate problem, we're currently not seeing the CI coverage checks pass. I want to keep this issue open until we get that working.

@echeran echeran reopened this Oct 13, 2020
@filmil
Copy link
Contributor

filmil commented Oct 13, 2020

Is there a way to run the CI checks locally? That seems to me as a necessary step to get to a place where this can be debugged effectively.

@echeran
Copy link
Contributor Author

echeran commented Oct 13, 2020

There is nothing official. And there wasn't anything open-source worth trying, at least when I started GH Actions stuff in the summer.. It looks like, in that relatively short time, the leading community tool for this has fixed an important issue recently that makes it worth trying out.

I agree, I don't understand why an important framework wouldn't have a local runner, for exactly that reason.

@zbraniecki
Copy link
Member

I got a repro! Manually followed https://github.com/mozilla/grcov#usage until cargo +nightly test reproduced it.

I'm going to try to fix it or switch us to tarpaulin.

@zbraniecki zbraniecki self-assigned this Oct 13, 2020
@zbraniecki zbraniecki added this to the ICU4X 0.1 milestone Oct 13, 2020
@zbraniecki
Copy link
Member

Ok, so the issue is described by Alex in rust-lang/cargo#5423

I have two ideas how to fix it:

  • Try to run coverage without panic=abort. Not pretty but if it gives us a reasonable coverage, we'll ride it for now and I'll report an issue to grcov.
  • Switch to tarpaulin. I have an old PR Switch code coverage to Tarpaulin #184 and I verified that it doesn't require panic=abort.

@zbraniecki
Copy link
Member

fixed

@zbraniecki
Copy link
Member

Reported upstream as mozilla/grcov#494

@sffc sffc added the T-bug Type: Bad behavior, security, privacy label Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-infra Component: Integration test infrastructure T-bug Type: Bad behavior, security, privacy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants