test: refactor: replace unnecessary `BytesIO` uses #27389

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202204-test-nuke_bytesio changing 3 files +9 −15

theStack commented at 12:21 PM on April 1, 2023: contributor

Rather than needing to create intermediate stream variables, we can use helper functions like tx_from_hex instead or access the result directly, leading both to increased readability and less code.

test: refactor: replace unnecessary `BytesIO` uses

Rather than needing to create intermediate stream variables, we can use
helper functions like `tx_from_hex` instead or access the result
directly, leading both to increased readability and less code.

f842ed9a40

DrahtBot commented at 12:21 PM on April 1, 2023: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK stickies-v, brunoerg, aureleoules

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:
- #26415 (rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block by andrewtoth)
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.
DrahtBot added the label Tests on Apr 1, 2023
in test/functional/interface_rest.py:161 in f842ed9a40
```
 157 | @@ -160,12 +158,11 @@ def run_test(self):
 158 |          bin_request = b'\x01\x02'
 159 |          for txid, n in [spending, spent]:
 160 |              bin_request += bytes.fromhex(txid)
 161 | -            bin_request += pack("i", n)
 162 | +            bin_request += n.to_bytes(4, 'little')
```
stickies-v commented at 11:01 AM on April 3, 2023:

nit: I suppose technically this is behaviour change, since we're going from default endianness to little endianness? But I suspect previously the test would have failed on big endian platforms, and being explicit is a strict improvement. Just flagging in case it does lead to weird behaviour somewhere.
stickies-v approved
stickies-v commented at 11:01 AM on April 3, 2023: contributor

ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0

Easier to read, less duplication.
brunoerg approved
brunoerg commented at 5:13 PM on April 3, 2023: contributor

crACK f842ed9a40c0db656b86f85e84dd4978865cc0a0
aureleoules approved
aureleoules commented at 5:58 PM on April 3, 2023: member

ACK f842ed9a40c0db656b86f85e84dd4978865cc0a0 - It seems that these are the only instances that can be changed and it simplifies test code.
fanquake merged this on Apr 4, 2023
fanquake closed this on Apr 4, 2023
theStack deleted the branch on Apr 4, 2023
sidhujag referenced this in commit ec54ff4665 on Apr 4, 2023
bitcoin locked this on Apr 3, 2024

Type	Reviewers
ACK	stickies-v, brunoerg, aureleoules

Contributors

Labels

test: refactor: replace unnecessary `BytesIO` uses #27389

Reviews

Conflicts