Improve feature coverage in CI test suite #147
Conversation
|
I'm hesitant about the |
|
No worries, I'll pull that change out of the PR and re-spin. Thanks for looking. |
The feature "serde-std" gets tested along with "std" during the combos of two loop, no need to run it explicitly.
Currently when we run the test suite we always enable the "std" feature. There are tests that can be run without "std", we should run these without "std" to better test our no-std code.
Currently we always run tests with "std" enabled. It is currently not possible to run the tests without an allocator but there is no reason that we should not be able to run allocation free tests without an allocator, doing so is better because it tests our claim that the lib can be used without an allocator.
tcharding
force-pushed
the
feature-gating
branch
4 times, most recently
from
e7f5513 to
746f39e
Compare
In order to assist users of the library understanding which features should be enabled for what functionality add documentation to the optional dependencies.
tcharding
force-pushed
the
feature-gating
branch
from
746f39e to
af992d7
Compare
| @@ -41,6 +42,9 @@ if [ "$DO_FEATURE_MATRIX" = true ]; then | |||
| done | |||
| done | |||
|
|
|||
| # Other combos | |||
| # TODO: Add this test once we bump MSRV past 1.29 | |||
| # cargo test --all --no-default-features --features="std,schemars" | |||
There was a problem hiding this comment.
In af992d7:
Did you intend to comment out this line? From your commit message it sounds like you do intend to test std and schemars.
We definitely need schemars to be tested in CI somehow. Historically we have had breakage from this dependency that went undetected by CI. (This is why, incidentally, I worte all my own independent CI scripts, and also why schemars is not a dependency in any other rust-bitcoin crate.)
There was a problem hiding this comment.
Shit, I've been a bit sloppy lately fixing commit messages when re-basing. I'm still struggling big time with Rust 1.29, I have some shell functions to build with 1.29 but I often forget to use them because they are not well tied in with my editor ... so I'm constantly re-basing and getting punished by CI. You might have noticed all the force pushes in my PRs :) I'm being lazy, we are so close to bumping to 1.41
We have extended_tests which test bitcoin_hashes with serde, schemars, and std enabled. The schemars feature doesn't build on its own at the moment cargo check --no-default-features --features=schemars fails. I didn't investigate why I just added this comment to remind us.
I added this commented out line with the MSRV comment since I've seen that done in a few places and have started doing it. Now as I write this message I think it would be better to add all these to the edition 2018 tracking issue instead of leaving it in the code. Oh that's right, the tracking issue is not on this repo its on rust-bitcoin.
Will fix the commit log.
There was a problem hiding this comment.
Ok, good observation that we'll be bumping to 1.41.0 soon, and at that point we can put schemars into the normal CI pipeline.
We get build errors when building with Rust 1.29 for `core2` and `schemars`. Add todos to remind us to test theses once we bump MSRV. Note the `schemars` cannot be added to the test matrix because it must be built with `std`, we must add a specific test for this combination. Example build error: ``` error: unable to get packages from source Caused by: failed to parse manifest at `/home/tobin/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/core2-0.3.3/Cargo.toml` Caused by: editions are unstable Caused by: feature `edition` is required ``` Add a TODO to `test.sh` to remind us to add `core2` to the features matrix once we bump MSRV. Add a commented out schemars/std test and a note to uncomment test once we bump MSRV past 1.29
tcharding
force-pushed
the
feature-gating
branch
from
af992d7 to
5dc3fd4
Compare
Improve feature testing by adding all the features to the test matrix and enabling testing with
--no-default-features.