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

[QA] remove some magic mining constants in functional tests #15238

Merged
merged 2 commits into from Feb 12, 2019

Conversation

Projects
None yet
8 participants
@instagibbs
Copy link
Member

commented Jan 23, 2019

The fewer magic numbers the better.

Also more directly tested a submitheader case of bad previous blockhash.

@instagibbs instagibbs changed the title remove some magic mining constants in functional tests [QA] remove some magic mining constants in functional tests Jan 23, 2019

@instagibbs instagibbs force-pushed the instagibbs:magic_miner branch to d4a2b9e Jan 23, 2019

@fanquake fanquake added the Tests label Jan 23, 2019

@instagibbs instagibbs force-pushed the instagibbs:magic_miner branch 2 times, most recently from 15b0e3c to d37b1c3 Jan 24, 2019

@instagibbs instagibbs force-pushed the instagibbs:magic_miner branch from d37b1c3 to b651ef7 Jan 24, 2019

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Jan 24, 2019

utACK b651ef7

@Empact

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

utACK b651ef7

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

Merge bitcoin#15238: [QA] remove some magic mining constants in funct…
…ional tests

b651ef7 submitheader: more directly test missing prev block header (Gregory Sanders)
1e7f741 remove some magic mining constants in functional tests (Gregory Sanders)

Pull request description:

  The fewer magic numbers the better.

  Also more directly tested a `submitheader` case of bad previous blockhash.

Tree-SHA512: 52b01a6aa199fa909eea4e9e84409a901933e545724e33149cc4132c82168199fd678809b6d94d95c9ff6ad02238a9552363620d13b8beaa5d4b67ade9ef425c

@MarcoFalke MarcoFalke merged commit b651ef7 into bitcoin:master Feb 12, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -15,6 +15,7 @@
from test_framework.messages import (
CBlock,
CBlockHeader,
BLOCK_HEADER_SIZE

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Feb 12, 2019

Member

style-nit: Missing trailing , 👀

assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata='ff' * 80))
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * BLOCK_HEADER_SIZE))
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * (BLOCK_HEADER_SIZE-2)))
assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata=super(CBlock, bad_block).serialize().hex()))

This comment has been minimized.

Copy link
@AkioNak

AkioNak Feb 18, 2019

Contributor

Currentry, minimum required version of Python (test) is 3.4.
https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md

But hex() seems to be introdued in Python 3.5.
https://docs.python.org/3.5/library/stdtypes.html?highlight=bytes%20hex#bytes.hex

I got following error when executing this test on Python3.4.

AttributeError: 'bytes' object has no attribute 'hex'
@Sjors

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

This test also fails for me locally on Python 3.4. If you install PyEnv it will catch this.

Fixed in #15439

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.