bitcoin
test: Use test framework utils in functional tests
Replaces bare asserts with test framework utils across both the functional tests and the test framework itself.
Also adds the assert_not_equal, assert_less_than, and assert_less_than_or_equal util functions for greater readability.
Fixes #23119.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | BrandonOdiwuor |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #29007 (test: create deterministic addrman in the functional tests by stratospher)
- #28926 (rpc: add 'getnetmsgstats' RPC by willcl-ark)
- #28890 (rpc: Remove deprecated -rpcserialversion by maflcko)
- #28550 (Covenant tools softfork by jamesob)
- #28312 (test: fix
keys_to_multisig_script(P2MS) helper for n/k > 16 by theStack) - #28307 (rpc, wallet: fix incorrect segwit redeem script size limit by furszy)
- #21283 (Implement BIP 370 PSBTv2 by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Though the change is simple, it touches a ton of files. I can break it into multiple PRs if that makes sense.
@theStack Added you as reviewer/mentor, because the issue was opened by you
@osagie98: Can you squash the commits and rebase on master? Happy to review.
@theStack done (I hope, still getting used to the GitHub workflow). Let me know if you'll need anything else
Hi @theStack, any chance you'd be able to review this?
Yes, planning to (finally) review this within the next 1-2 days.
Hi @BrandonOdiwuor,
Sorry, I missed the notification of your messages. I missed a few asserts, so I went back to add them. I also initially chose not to change any assert x is True/False as I wasn't sure if it would be useful, but I decided now to change those as well.
Please take another look whenever you're free, thank you!
Closing for now due to inactivity. I think it is pretty clear that a change like this won't be going in in one go.
If someone wants to try this again, I'd recommend to try a smaller portion and see if reviewers like it. If not, then maybe it isn't worth changing?