apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
5.29k stars 973 forks source link

Enable codecov #6067

Closed 2010YOUY01 closed 2 weeks ago

2010YOUY01 commented 1 year ago

Which issue does this PR close?

Issue https://github.com/apache/arrow-datafusion/issues/3678 Prev PR that disabled codecov: https://github.com/apache/arrow-datafusion/pull/3679

Rationale for this change

cargo tarpaulin will consume lots of disk space, previously it failed due to this reason. I tried removing several large files on GitHub Action virtual machines, then It can run again. Problem: after that, two dataframe tests failed, they're disabled temporarily. (cargo tarpaulin will run tests to collect coverage stats, these tests will only fail under cargo tarpaulin, normal test runs are not affected) https://github.com/apache/arrow-datafusion/blob/10b0eff7dfbe195a724285bc166b20240e8ebccb/datafusion/core/src/dataframe.rs#L1978 https://github.com/apache/arrow-datafusion/blob/10b0eff7dfbe195a724285bc166b20240e8ebccb/datafusion/core/src/dataframe.rs#L2070 These two test cases are added after last time codecov CI is disabled, I don't know why they might fail under cargo tarpaulin Failure messages:

  failures:

---- dataframe::tests::partition_aware_union stdout ----
thread 'dataframe::tests::partition_aware_union' panicked at 'assertion failed: `(left == right)`
  left: `2`,
 right: `1`', datafusion/core/src/dataframe.rs:2008:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- dataframe::tests::verify_join_output_partitioning stdout ----
thread 'dataframe::tests::verify_join_output_partitioning' panicked at 'assertion failed: `(left == right)`
  left: `UnknownPartitioning(1)`,
 right: `Hash([Column { name: "c1", index: 0 }, Column { name: "c2", index: 1 }], 1)`', datafusion/core/src/dataframe.rs:2112:21

failures:
    dataframe::tests::partition_aware_union
    dataframe::tests::verify_join_output_partitioning

test result: FAILED. 1475 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 139.30s

Apr 19 21:59:34.770  INFO cargo_tarpaulin::process_handling::linux: Launching test
Apr 19 21:59:34.776  INFO cargo_tarpaulin::process_handling: running /home/runner/work/arrow-datafusion/arrow-datafusion/target/debug/deps/simplification-ba4cc1d732bcee01

Failing CI run : https://github.com/2010YOUY01/arrow-datafusion/actions/runs/4750006102 Can be reproduced with CI config in this commit https://github.com/apache/arrow-datafusion/commit/75c7fb77094d7e4ce907db343b77f268f075356f

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

codecov-commenter commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@10b0eff). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head cdd5f24 differs from pull request most recent head ae31a7b. Consider uploading reports for the commit ae31a7b to get more accurate results

@@           Coverage Diff           @@
##             main    #6067   +/-   ##
=======================================
  Coverage        ?   77.56%           
=======================================
  Files           ?      419           
  Lines           ?    96984           
  Branches        ?        0           
=======================================
  Hits            ?    75228           
  Misses          ?    21756           
  Partials        ?        0           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jackwener commented 1 year ago

Just in my opinion, I don't feel the need to add codedev for Datafusion for now.

2010YOUY01 commented 1 year ago

Just in my opinion, I don't feel the need to add codedev for Datafusion for now.

I followed tarpaulin repo's issues to get this fixed. This tool seems not very stable, people encounter different problems, and even the author can't tell why exactly. So another issue is if it is got fixed now and it's still likely to break in the future and cause CI problems again :( Maybe we should leave it commented and If people want to see coverage they can run it themself.

alamb commented 1 year ago

The coverage report from this PR is here https://app.codecov.io/gh/apache/arrow-datafusion/pull/6067 but contains no information on what was not covered (so like what do we get from this site other than a webpage with a number).

When I look at the coverage report from arrow2, it seems to be quite useful (and have sources, etc): https://app.codecov.io/gh/jorgecarleitao/arrow2?search=&displayType=tree

Maybe we can follow the model there https://github.com/jorgecarleitao/arrow2/blob/main/.github/workflows/coverage.yml 🤔

2010YOUY01 commented 1 year ago

Detailed information is normally displayed when switching to commit instead of pull requests. https://app.codecov.io/gh/apache/arrow-datafusion/tree/enable-codecov I guess for PRs' reports, Codecov requires the latest commit of the main branch to already have a code coverage report in order to use it as a base for comparison. That might be the reason for not showing a detailed report in the first PR.🤔 And thank you for the pointer! I will check if the tool they used to generate Codecov reports is more stable than the current one.

alamb commented 1 year ago

That might be the reason for not showing a detailed report in the first PR.🤔

It is possible.

However, if you look at codecov reports from sqlparser (which I think is configured the same as this), such as the one on https://github.com/sqlparser-rs/sqlparser-rs/pull/817 https://coveralls.io/builds/57556726

it has the same lack of source issue

github-actions[bot] commented 3 weeks ago

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.