Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: remove byte.hex() to keep compatibility #15439

Merged
merged 1 commit into from Feb 20, 2019

Conversation

Projects
None yet
5 participants
@AkioNak
Copy link
Contributor

commented Feb 19, 2019

Use test_framework.util.bytes_to_hex_str() instead of bytes.hex() that new in Python 3.5 to support minimum version of Python(test).

test/functional/test_framework/wallet_util.py is also reported to have '.hex()' in #15397,
but it does not matter because it calls CScript.hex() defined in wallet_util.py.

tests: remove byte.hex() to keep compatibility
Use test_framework.util.bytes_to_hex_str() instead of bytes.hex() that
new in Python 3.5, to support minimum version of Python(test).

@fanquake fanquake added the Tests label Feb 19, 2019

@fanquake fanquake requested a review from MarcoFalke Feb 19, 2019

@MarcoFalke MarcoFalke added this to the 0.18.0 milestone Feb 19, 2019

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Let's merge this after the branch off to 0.18

@laanwj

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Let's merge this after the branch off to 0.18

To be clear: you want to merge this to the 0.18 branch after there is a 0.18 branch?

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Yes, there is no reason to have this in the 0.19 brach

@laanwj

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

OK makes sense—let's ask @AkioNak to change the target branch when it's there

@Sjors

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

tACK 1a062b8. I'm OK with merging it into the 0.18 branch. Although it took me some time to find this after running into these test failures myself.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

Ok, it appears at least two devs are using pyenv and would be unable to run the tests at all for the next two weeks. Going to merge this now and then revert in #14954

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Feb 20, 2019

Merge bitcoin#15439: tests: remove byte.hex() to keep compatibility
1a062b8 tests: remove byte.hex() to keep compatibility (Akio Nakamura)

Pull request description:

  Use ```test_framework.util.bytes_to_hex_str()``` instead of ```bytes.hex()``` that new in Python 3.5 to support minimum version of Python(test).

  ```test/functional/test_framework/wallet_util.py``` is also reported to have '\.hex()' in bitcoin#15397,
  but it does not matter because it calls CScript.hex() defined in wallet_util.py.

Tree-SHA512: 1019212e965f0848d235fab4da11959dffa42e8adfcd41216c10795cfc63c804b5deb5a3317f25d29940b9dcf088ab76fe3fa80d2679dc19f5f482dc5bde3283

@MarcoFalke MarcoFalke merged commit 1a062b8 into bitcoin:master Feb 20, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@AkioNak AkioNak deleted the AkioNak:keep_compatiblity branch Feb 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.