librustzcash
librustzcash copied to clipboard
CI: Run tests with cargo-nextest
Codecov Report
Base: 50.42% // Head: 50.80% // Increases project coverage by +0.38% :tada:
Coverage data is based on head (
e778dc8) compared to base (67cb63a). Patch coverage: 49.24% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## non-consensus-changes-on-branchid-37519621 #512 +/- ##
==============================================================================
+ Coverage 50.42% 50.80% +0.38%
==============================================================================
Files 90 97 +7
Lines 8306 9522 +1216
==============================================================================
+ Hits 4188 4838 +650
- Misses 4118 4684 +566
| Impacted Files | Coverage Δ | |
|---|---|---|
| components/f4jumble/src/lib.rs | 77.77% <ø> (ø) |
|
| components/zcash_address/src/convert.rs | 4.25% <0.00%> (-2.00%) |
:arrow_down: |
| components/zcash_address/src/kind/unified.rs | 38.80% <ø> (+0.82%) |
:arrow_up: |
| zcash_client_backend/src/data_api.rs | 5.88% <ø> (ø) |
|
| zcash_client_backend/src/data_api/error.rs | 20.51% <0.00%> (-2.35%) |
:arrow_down: |
| zcash_client_backend/src/data_api/wallet.rs | 17.07% <0.00%> (-4.85%) |
:arrow_down: |
| zcash_client_backend/src/decrypt.rs | 0.00% <0.00%> (ø) |
|
| zcash_client_sqlite/src/error.rs | 23.52% <0.00%> (+2.10%) |
:arrow_up: |
| zcash_extensions/src/transparent/demo.rs | 49.71% <ø> (+4.31%) |
:arrow_up: |
| zcash_history/src/lib.rs | 0.00% <ø> (ø) |
|
| ... and 126 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Hmm, this actually ends up taking longer, because for some reason the sequence of compiles it triggers takes longer overall, plus we have several minutes compiling and installing cargo-nextest itself. So I'm putting this on the backburner until we can figure out build efficiencies.
BTW nextest is now available as pre-built binaries, if you're interested in using them: https://nexte.st/book/pre-built-binaries
(Below I ignore the 2m 50s for building the param-fetching binary, which is unchanged)
- Run tests and doc tests (7m 34s)
- 6m 06s building, 1m 28s testing.
- Run slow tests (1m 41s)
- 0.15s building, so basically all testing.
- Total build time: 6m 06s
- Total test time: 3m 09s
- Run tests and slow tests (8m 56s)
- 6m 06s building, 2m 50s testing.
- Run doc tests (3m 52s)
- 3m 39s building, 13s testing.
- Total build time: 9m 45s
- Total test time: 3m 03s
Switching to nextest saves us just over 3% during testing (6 seconds), but increases build times by almost 60% due to the need to recompile with doctests enabled. I'm a little surprised by this, as I saw a 25% reduction overall (building + testing) locally, but I suspect this is due to both faster build times locally (I have a Ryzen 9 5950X), and overcontention on the more resource-constrained CI builders (since our proving tests are internally running multithreaded code). In any case, it's not worth us merging this PR unless nextest has a way to build the tests with doc tests enabled (even if it can't run them due to limitations in stable Rust).
Speculation: is there any way we can do sophisticated / fine-grained build caching in CI, since that's the bottleneck?
I've experimented with a few cargo build caching github actions without much luck. I think I just don't understand gh CI well enough to understand how to do it well.
Maintainer of nextest here -- it's a bit surprising that doctests cause a rebuild because I don't see that on my own projects, it's worth digging into why that is happening. Wondering if there's something related to proc macros going on (not sure if you have any).
In any case, it's not worth us merging this PR unless nextest has a way to build the tests with doc tests enabled
Sadly the two are related -- cargo test --doc is just a really special snowflake.