Improve the test script and CI pipeline #405
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use the Bash `set -e` option to make the test script fail if a command fails. While we are at it add sanity check commands that call the compiler and `cargo`.
There is no obvious reason to run the fuzzer in a subprocess. Remove the subprocess from the fuzzing code. While we are at it change the code comment to state what it does instead of mentioning the CI vms.
There is no obvious reason to run the fuzzer in a subprocess. Remove the subprocess from the fuzzing code.
tcharding
force-pushed
the
05-11-test-script
branch
from
560ded4 to
06fba01
Compare
Member
Author
|
Removed last patch, I guess that is why we are using 1.58 for fuzzing and not stable? |
tcharding
changed the title
In an effort to improve test coverage without being inefficient add a new env variable `DO_FEATURE_MATRIX` to the test script. This is how we do it over in `rust-secp256k1`, it enables good test coverage while not having to run the whole matrix in every CI job. Add cargo calls for each example because the current way we run the examples does not work for those who configure the build output directory to something other than the default.
Be a good UNIX hacker and exit the test script explicitly with a success value.
As we do in `rust-secp256k1` use a string that includes 'Nightly' in name of the nightly test.
In the `Tests` CI job run the feature matrix tests. Also run these tests for 4 toolchains, stable, beta, nightly, and 1.41.1
Rename the integration test job to `IntTests` now that we do not run the full matrix of unit tests in this job. Simplify the yaml by removing the strategy section since we only use a single toolchain in this job.
tcharding
force-pushed
the
05-11-test-script
branch
from
06fba01 to
ed51323
Compare
tcharding
changed the title
sanket1729
approved these changes
May 12, 2022
| @@ -1,5 +1,7 @@ | |||
| #!/bin/sh -ex | |||
|
|
|||
| set -e | |||
Member
There was a problem hiding this comment.
This is a big bug! I wonder how it ever worked till now
Member
Author
There was a problem hiding this comment.
I guess we were getting some false positives in the CI runs. rust-bitcoin/contrib/test.sh doesn't have this either :)
| cargo run --example sign_multisig | ||
| cargo run --example verify_tx > /dev/null | ||
| cargo run --example psbt | ||
| cargo run --example xpub_descriptors |
Member
There was a problem hiding this comment.
Why manually list examples instead of run-parts? Is it because of the extra args to each example? We need to remember to add examples here every time.
Member
Author
There was a problem hiding this comment.
Its so the script can be run locally, but yes as you say there is a maintenance cost. Is it worth it, I don't know? I configure target-dir to be a global build cache so there is no target directory when I build so the examples don't run.
apoelstra
approved these changes
May 13, 2022
sanket1729
reviewed
May 15, 2022
| run-parts ./target/debug/examples | ||
| # Run all the examples | ||
| cargo build --examples | ||
| cargo run --example htlc --features=compiler |
Member
There was a problem hiding this comment.
nit: There is a white space at the end of this line.
Member
Author
There was a problem hiding this comment.
ooo, means I did not read the diff before pushing, bad Tobin. Thanks man, I'll fix it.
Member
There was a problem hiding this comment.
I read the diff and I missed this too, oops! Thanks Sanket.
heap-coder
added a commit
to heap-coder/rust-miniscript
that referenced
this pull request
Sep 27, 2025
…I pipeline
ed51323a0bb3257b09abfb5eb51ca21815b815af Rename and simplify integration test job (Tobin C. Harding)
27df8630e5c7f419d5f1a3d0d7b2095f3731ab82 Run feature matrix tests (Tobin C. Harding)
ca9988de6cb441c5b4293d78fc0b3b10920d5333 Add additional information to nightly job (Tobin C. Harding)
a5718f76a9885a1e2c0d51992527bc3ed115fb94 Explicitly exit test script with 0 (Tobin C. Harding)
3913a08dfb3d55fd7b0153f66bb8c6d36fca0f6f Improve test coverage (Tobin C. Harding)
04577224f72bf66e24de7c5d0d0137797a706ed3 Do not run formatter in subprocess (Tobin C. Harding)
b072a5f8122fdc0ed162fb61b4cb748b46286d24 Do not run fuzzer in subprocess (Tobin C. Harding)
62d5169f210907d1bfe11085f1601fde50ede304 Fail script if command fails (Tobin C. Harding)
Pull request description:
Make an effort to improve the test script, taking inspiration for the test script in `rust-secp256k1`. Since Bash can, at times, be a bit obscure, I made extra effort to do very simple discreet patches.
First part of the PR is fixes to the test script, second part is improvements to the CI pipeline.
ACKs for top commit:
sanket1729:
ACK ed51323a0bb3257b09abfb5eb51ca21815b815af
apoelstra:
ACK ed51323a0bb3257b09abfb5eb51ca21815b815af
Tree-SHA512: 9e150b1faebc0d5ae73f7779129c1e8ec5c48bb5e73380793266f02b73636427a45aa378db6cbab8f815f655fd3ead2fffab6000bdb92cf7c443de507c72d261
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make an effort to improve the test script, taking inspiration for the test script in
rust-secp256k1. Since Bash can, at times, be a bit obscure, I made extra effort to do very simple discreet patches.First part of the PR is fixes to the test script, second part is improvements to the CI pipeline.