ARROW-4189: [Rust] Added coverage report. #7797
Conversation
|
Here is an example of the current report: https://codecov.io/gh/jorgecarleitao/arrow/tree/rust_coverage/rust |
|
This is sub-optimal: the build currently takes 19m, and this PR makes the build take 37m. This is mostly due to the compilation, which I think we should be doing within docker. I will try a couple of things. |
|
coverage report will be nice! |
|
I investigated how to add this to the docker image. It is possible, but we need to run the image using Note that this argument introduces a couple of attack vectors to the docker orchestration - I am not even sure if travis accepts it. Regardless, let me know your thoughts on this @sunchao . For reference, the error that I get (locally and on the CI) when this argument is not passed is shown in these logs. |
|
@sunchao , I moved the coverage to run on the docker image and added reporting to codecov to github's workflow. Since coverage reporting requires a different compilation strategy, it recompiles the project and thus the CI takes longer. One way to address this is to make all our tests run through |
|
Thanks @jorgecarleitao ! IMO the compilation time is important so let's see of #7799 goes. I'll take a look later today. |
|
@jorgecarleitao So I saw that #7799 was merged. What does that mean for this PR now? |
|
@andygrove , unfortunately do not benefit from caching here, as this needs to be built from scratch. I am not very happy with the build time (+10m of build time). IMO it is great to have these reports, but I do not think we should block the build for more 10m for this, specially since we are not imposing any min coverage. Do we have any place to put nightly stuff? This could maybe fit there. My thinking is that increasing coverage is a great "starter" issues, and having a link to the nightly coverage report could create an incentive for people to pick a file and start increasing coverage on it. Another option is to run this instead of the tests (since this runs the lib tests anyway). |
jorgecarleitao
changed the title
|
@sunchao @andygrove, based on @wesm hint on ARROW-5972, I made this PR be to place rust coverage on nightly builds instead of every PR/build. I do not know how to test if this works, though. I mostly got inspiration by (aka copied) code around to my purposes. |
|
@kszucs could you take a look and see if there is a good way to test this? Otherwise maybe we can merge and test? |
|
@github-actions crossbow submit rust-coverage |
|
Revision: 46dc7f2 Submitted crossbow builds: ursa-labs/crossbow @ actions-530
|
|
Seems like the template has errors. You can take a look at the builds under the crossbow repository's actions tab. |
|
@github-actions crossbow submit rust-coverage |
|
@kszucs , thanks a lot. Can we try again (I tried, but I likely do not have authority to ask the bot stuff). EDIT: nevermind, it also answers to me. |
|
Revision: c58d803 Submitted crossbow builds: ursa-labs/crossbow @ actions-533
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: f198553 Submitted crossbow builds: ursa-labs/crossbow @ actions-534
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: aa19914 Submitted crossbow builds: ursa-labs/crossbow @ actions-535
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: 26319d2 Submitted crossbow builds: ursa-labs/crossbow @ actions-536
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: 48310b5 Submitted crossbow builds: ursa-labs/crossbow @ actions-537
|
|
The current status is that this is only running the @github-actions crossbow submit rust-coverage |
|
@github-actions crossbow submit rust-coverage |
|
Revision: 038fa8f Submitted crossbow builds: ursa-labs/crossbow @ actions-538
|
|
@github-actions crossbow submit rust-coverage |
1 similar comment
|
@github-actions crossbow submit rust-coverage |
|
Revision: d2d7aa9 Submitted crossbow builds: ursa-labs/crossbow @ actions-539
|
|
Revision: d2d7aa9 Submitted crossbow builds: ursa-labs/crossbow @ actions-540
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: b26d19d Submitted crossbow builds: ursa-labs/crossbow @ actions-542
|
|
@github-actions crossbow submit rust-coverage |
|
Revision: 2ba754d Submitted crossbow builds: ursa-labs/crossbow @ actions-543
|
|
@nevi-me , @kszucs , @emkornfield @sunchao @andygrove , I think that this is now ready to review. The report is here. The report takes 15m to make, and also runs all tests. We could replace running tests by this, but I am not sure we want to run tests with different compilation flags, and thus nightly seems more appropriate. |
|
@jorgecarleitao thanks for setting this up! I'm afraid the codecov script picks up the CI environment (branch commit sha etc.) belonging to crossbow, so it will make the reports less browsable/usable. We can also schedule nightly builds on apache/arrow directly, we already have nightly builds configured for Python and C++. It may be better to have a similar |
|
@kszucs , I agree that the browsability should be fixed. Either works for me: we can try to fix the paths or move the build to apache/arrow. I am not familiar with the CI in arrow, so I will abstain from a decision: let me know what you prefer. |
|
I thought that codecov looks for the CI specific environment variables, but if placing a |
|
@github-actions crossbow submit rust-coverage |
|
Revision: 92f59f1 Submitted crossbow builds: ursa-labs/crossbow @ actions-544
|
|
Ok, @kszucs I think that this is ready for a re-review. Report is here: https://codecov.io/github/apache/arrow/commit/604065c5188af00f640051d909e27f37db4eb43b I removed all code in tasks, and created a new workflow on github for this, like you initially suggested, as I was unable to make it work on the nightly. |
|
Thanks @jorgecarleitao, it looks good to me. Just one nit, could you please add a coverage report link to the rust README to improve its visibilty? |
|
Thanks @kszucs , good ideas. Done. |
|
@jorgecarleitao could you rebase on top of the master? I'm unable to push to your fork, but the build failures should be resolved after a rebase. |
This uses tarpaulin
Closes apache#7797 from jorgecarleitao/rust_coverage Authored-by: Jorge C. Leitao <jorgecarleitao@gmail.com> Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
Closes apache#7797 from jorgecarleitao/rust_coverage Authored-by: Jorge C. Leitao <jorgecarleitao@gmail.com> Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
No description provided.