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

Code coverage change is not accurate on PRs #703

Open
jonnyandrew opened this issue Jun 1, 2023 · 4 comments
Open

Code coverage change is not accurate on PRs #703

jonnyandrew opened this issue Jun 1, 2023 · 4 comments

Comments

@jonnyandrew
Copy link
Contributor

Codecov shows an improvement in coverage despite no code changes.

Originally noted by @alunturner in #701 (comment)

@jonnyandrew
Copy link
Contributor Author

jonnyandrew commented Nov 20, 2023

Codecov docs mention that this could cause the problem

Reasons for unexpected changes
Missing coverage reports or failed builds.

So I think this happens because we're not running all the tests on PRs (Android and iOS are only run when the label is applied) but we are running all of them on the main branch, hence there is nearly always a mismatch and the coverage is reported to have changed.

@jmartinesp @langleyd do you have any background on the current system of adding Android/iOS labels? Did we hit some usage limits in the past? I'm wondering if we could try running all the tests on all PRs.

@jmartinesp
Copy link
Contributor

jmartinesp commented Nov 20, 2023

I'm not sure, to be honest. It might have been to make Rust/JS only PRs faster to merge, since the Android job takes ~15min per run and for iOS it's something between 20-25min. Maybe we could enable merge queues for this so when merging every job will be run, and thus have proper coverage reports?

@jonnyandrew
Copy link
Contributor Author

Ooh, I just found out about 'carryforward flags' in Codecov which might do what we need!

Carryforward Flags are designed for projects that do not upload total coverage for every commit (e.g., monorepos with multiple applications/languages, iterative/partial/delta testing setups, etc).

If I understood correctly, we could carry forward the base branch's Android and iOS coverage so that PRs do not need to report them.

@jonnyandrew
Copy link
Contributor Author

Maybe we could enable merge queues for this so when merging every job will be run, and thus have proper coverage reports?

Not to say we should not also use merge queues to ensure all the tests run before merging. That could still be nice to have as a safety net. Rust changes, for example, should run all the tests but it's easy to forget to add the labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants