test: refactor: use consistent bytes <-> hex-string conversion in functional test framework #22619
Conversation
maflcko
left a comment
There was a problem hiding this comment.
Concept ACK, but I am wondering if the member method can be used
|
@MarcoFalke Yeah, it works. It also applies to the change made in I could just switch them to use |
ef40833 to
306dad7
Compare
|
Concept ACK |
|
Tested the changes on my end and works correctly. |
theStack
left a comment
There was a problem hiding this comment.
Concept ACK
There is one more instance where the hex() member method can be used, see below.
binascii is still used in Python files outside of the test framework, e.g. in contrib and test/util (I didn't think about those in the issue, my bad...). If you want, you can also tackle those in this PR, if not, a follow-up is also okay I guess.
Yeah, initially after running |
306dad7 to
5a1bef6
Compare
… hex-string conversion in functional test framework 5a1bef6 test: refactor: remove binascii from test_framework (Zero-1729) Pull request description: This PR continues the work started in PR #22593, regarding using the `bytes` built-in module. In this PR specifically, instances of `binascii`'s methods `hexlify`, `unhexlify`, and `a2b_hex` have been replaced with the build-in `bytes` module's `hex` and `fromhex` methods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework. Additionally, certain changes made are based on the following assumption: ``` bytes.hex(data) == binascii.hexlify(data).decode() bytes.hex(data).encode() == binascii.hexlify(data) ``` Ran the functional tests to ensure behaviour is still consistent and changes didn't break existing tests. closes #22605 ACKs for top commit: theStack: Code-review ACK 5a1bef6 🔢 Tree-SHA512: 8f28076cf0580a0d02a156f3e1e94c9badd3d41c3fbdfb2b87cd8a761dde2c94faa5f4c448d6747b1ccc9111c3ef1a1d7b42a11c806b241fa0410b7529e2445f
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
No idea what is going on with GitHub, this was merged 35 minutes ago: f4328eb |
…hod calls 021daed refactor: replace remaining binascii method calls (Zero-1729) Pull request description: This PR removes the remaining `binascii` method calls outside `test/functional` and `test_framework`, as pointed out here bitcoin/bitcoin#22619 (review). Follow-up to #22593 and #22619 Closes #22605 ACKs for top commit: josibake: re-ACK bitcoin/bitcoin@021daed theStack: re-ACK 021daed Tree-SHA512: 2ae9fee8917112c91a5406f219ca70f24cd8902b903db5a61fc2de85ad640d669a772f5c05970be0fcee6ef1cdd32fae2ca5d1ec6dc9798b43352c8160ddde6f
Summary: Instances of binascii's methods `hexlify`, `unhexlify`, and `a2b_hex` have been replaced with the build-in bytes module's `hex` and `fromhex` methods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework. Also update some code in a comment in serialize_tests.cpp that was using binascii. This code also needed additional changes to work with python3 (because in python 3 you cannot join `bytes` using a `str` separator). This is a backport of [[bitcoin/bitcoin#22619 | core#22619]] Test Plan: `ninja check-functional-extended` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D11137
This PR continues the work started in PR #22593, regarding using the
bytesbuilt-in module. In this PR specifically, instances ofbinascii's methodshexlify,unhexlify, anda2b_hexhave been replaced with the build-inbytesmodule'shexandfromhexmethods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework.Additionally, certain changes made are based on the following assumption:
Ran the functional tests to ensure behaviour is still consistent and changes didn't break existing tests.
closes #22605