Tests: tidy up address.py and segwit_addr.py #19253
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
|
I ran I did fail I reviewed all the changes, and it all looks good to me, except for a small detail, please see question inline. |
decentclock
reviewed
Jun 21, 2020
jonatack
reviewed
Jul 1, 2020
There was a problem hiding this comment.
ACK modulo #19253 (comment)
The pep8, docstring and imports tidy-ups can probably be one clean-up commit.
b450389 to
ecb74db
Compare
I don't think it's related. I'm able to run that test successfully on this branch. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
This was referenced Jul 1, 2020
|
rebased |
ecb74db to
7edcdcd
Compare
jonatack
reviewed
Sep 3, 2020
maflcko
reviewed
Sep 3, 2020
These functions can be exported to other modules, so be explicit that they're encoding and decoding segwit addresses
This optional parameter is never used, so remove it.
No need to import twice from util.py
It's almost impossible to read bytes literals in code, so replace them with the hex string literal and then convert them to a bytes object using bytes.fromhex().
7edcdcd to
825fcae
Compare
|
re-ACK 825fcae per Good catch, Marco, on the p2wpkh address test. |
|
ACK 825fcae |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 4, 2020
825fcae [tests] Replace bytes literals with hex literals (John Newbery) 64eca45 [tests] Fix pep8 style violations in address.py (John Newbery) b230f8b [tests] Correct docstring for address.py (John Newbery) ea70e6a [tests] Tidy up imports in address.py (John Newbery) 7f639df [tests] Remove unused optional verify_checksum parameter (John Newbery) 011e784 [tests] Rename segwit encode and decode functions (John Newbery) e455713 [tests] Move bech32 unit tests to test framework (John Newbery) Pull request description: Lots of small fixes: - moving unit tests to test_framework implementation files - renaming functions to be clearer - removing multiple imports - removing unreadable byte literals from the code - fixing pep8 violations - correcting out-of-date docstring ACKs for top commit: jonatack: re-ACK 825fcae per `git range-diff a0a422c 7edcdcd 825fcae` and verified `wallet_address_types.py` and `wallet_basic.py --descriptors` (the failure on one travis job) are green locally. MarcoFalke: ACK 825fcae fanquake: ACK 825fcae - looks ok to me. Tree-SHA512: aea509c27c1bcb94bef11205b6a79836c39c62249672815efc9822f411bc2e2336ceb3d72b3b861c3f4054a08e16edb28c6edd3aa5eff72eec1d60ea6ca82dc4
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 3, 2021
Summary: Commit description: > This optional parameter is never used, so remove it. PR description: > Lots of small fixes: > > moving unit tests to test_framework implementation files > renaming functions to be clearer > removing multiple imports > removing unreadable byte literals from the code > fixing pep8 violations > correcting out-of-date docstring Most of the PR is related to segwit or code style, so only 2 out of 7 commits are relevant. This is a backport of [[bitcoin/bitcoin#19253 | core#19253]] [1/2] bitcoin/bitcoin@7f639df Test Plan: `ninja check-functional` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Subscribers: majcosta Differential Revision: https://reviews.bitcoinabc.org/D10417
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 3, 2021
Summary: It's almost impossible to read bytes literals in code, so replace them with the hex string literal and then convert them to a bytes object using bytes.fromhex(). This is a backport of [[bitcoin/bitcoin#19253 | core#19253]] [2/2] bitcoin/bitcoin@825fcae Test Plan: `ninja check-functional` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D10418
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Lots of small fixes: