Re-Add CodeCov

[blaginin]

Which issue does this PR close?

Related to #15155 (comment)

Rationale for this change

We have quite a lot of code that isn't covered in tests. And it's actually easy to miss covering some case when you submit a big PR

What changes are included in this PR?

Now, when a new PR introduces untested code, a warning message will be posted. Example: here.

Coverage data is aggregated from:

• linux-test
• linux-test-datafusion-cli
• sqllogictest-postgres
• macos-aarch64
• test-datafusion-pyarrow

Excluded from coverage:

• datafusion-examples — not technically tests.
• verify-benchmark-results — IMO we shouldn't rely on them for functionality tests, those should be covered on the unit test level
• doctests — because they requite nightly rust and I couldn't (yet) make it work in the CI (although will try more)

There are ways to make checks even more annoying (e.g. putting ❌ in CI checks). I want to do this later, but first, I want to make sure we're measuring coverage properly and others are okay with it. If that’s the case, we’ll tighten CI checks in future PRs.

Absolute coverage example: https://app.codecov.io/gh/blaginin/datafusion/pull/4/tree

Are there any user-facing changes?

No.

[blaginin]

To make this work, we'll need to add CODECOV_TOKEN and add the app to the repo (may already be there since we used it before)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Omega359]

I got a lot of pushback on code coverage when I brought it up, fyi. Here is a report I had done with a manual run.

[blaginin]

Thank you! The feedback I saw in that discussion:

• You asked if sqllogictests contribute to the coverage. Yes, here's the example (you can check sqllogictest-postgres)

• @timsaucer and @tustvold highlighted that coverage itself is useless and that 95% coverage doesn't mean much. Definitely agree! That's why I disabled project-based code coverage status - the only one I care about is the diff in the PR code. i.e., if you submit a PR and some of your code isn't covered, it should help the reviewer to understand the current state and not overlook tests.

• @mbutrovich raised a point about the license. In the DF repo itself, we only use the Codecov CLI to upload coverage. This isn't included in the DataFusion distribution itself and is under the Apache 2.0 license, so it should be fine?

[alamb]

Basically my opinion here is that the few times I tried to review the codecov reports, I found them useless. Maybe it has gotten better since they were originally

When I tried to look at the report on what lines were/were not covered it the diffs I remember looking at did not make any sense. Also I remember codecov takes a long time and thus was delaying PR CI cycles

if we are confident that

1. This data is actually correct
2. Reviewers will use the data to improve reviews,

Then I think it would be a good idea

[2010YOUY01]

Basically my opinion here is that the few times I tried to review the codecov reports, I found them useless. Maybe it has gotten better since they were originally

When I tried to look at the report on what lines were/were not covered it the diffs I remember looking at did not make any sense. Also I remember codecov takes a long time and thus was delaying PR CI cycles

if we are confident that

1. This data is actually correct
2. Reviewers will use the data to improve reviews,

Then I think it would be a good idea

I'm interested to see it when reviewing, but if it slows down CI, we can let it get triggered manually instead of running by default.

[alamb]

Starting with manual triggering sounds like a good idea to me

[blaginin]

Thank you for your feedback! I think we can do it this way:

• We make sure it doesn't affect test CI speed (I'll do that in this PR)
• We merge this PR
• We test for a few weeks to see if the community starts using Codecov reports and roll back if not (I'll create an issue to track this and assign it to myself)

[alamb]

Thank you for your feedback! I think we can do it this way:

Sounds like a good plan to me. Thank you

[alamb]

I am trying to clean up the review queue and it wasn't clear to me what the plan for this PR is, so marking it as draft