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

instagibbs commented at 4:09 PM on January 23, 2019: member

The fewer magic numbers the better.

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

instagibbs renamed this:
~~remove some magic mining constants in functional tests~~
[QA] remove some magic mining constants in functional tests
on Jan 23, 2019

instagibbs force-pushed on Jan 23, 2019

fanquake added the label Tests on Jan 23, 2019

in test/functional/mining_basic.py:169 in 75adf596a7 outdated

 163 | @@ -164,9 +164,9 @@ def assert_submitblock(block, result_str_1, result_str_2=None):
 164 |          assert_submitblock(bad_block, 'prev-blk-not-found', 'prev-blk-not-found')
 165 |  
 166 |          self.log.info('submitheader tests')
 167 | -        assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * 80))
 168 | -        assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * 78))
 169 | -        assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata='ff' * 80))
 170 | +        assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * BLOCK_HEADER_SIZE))
 171 | +        assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * (BLOCK_HEADER_SIZE-2)))
 172 | +        assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata=super(CBlock, bad_block).serialize().hex()))

practicalswift commented at 9:00 AM on January 24, 2019:

I'm not sure I follow here: which constructor is it that takes CBlock and bad_block as parameters?

instagibbs commented at 2:44 PM on January 24, 2019:

The base class is CBlockHeader, so we're accessing its serialization function.

Empact commented at 12:33 AM on January 25, 2019:

Maybe rename bad_block above to bad_hashPrevBlock_block or similar, and invoke the CBlockHeader constructor directly?

AkioNak commented at 7:41 AM on February 18, 2019:

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'

in test/functional/interface_rest.py:216 in 75adf596a7 outdated

 212 | @@ -211,28 +213,30 @@ def run_test(self):
 213 |          self.test_rest_request('/block/{}'.format(bb_hash))
 214 |          self.nodes[0].reconsiderblock(bb_hash)
 215 |  
 216 | +        BLOCK_HEADER_SIZE = len(CBlockHeader().serialize())

laanwj commented at 1:03 PM on January 24, 2019:

As this constant is used in multiple places, wouldn't it be an idea to define it in a shared place?

instagibbs commented at 2:45 PM on January 24, 2019:

Done, defined right after CBlockHeader is defined

instagibbs commented at 2:49 PM on January 24, 2019:

(and added an assert right after for the invariant of 80)

instagibbs force-pushed on Jan 24, 2019

remove some magic mining constants in functional tests 1e7f741745

submitheader: more directly test missing prev block header b651ef7e1c

instagibbs force-pushed on Jan 24, 2019

MarcoFalke commented at 11:11 PM on January 24, 2019: member

utACK b651ef7e1c39a820089695b29d14a07d910a385a

Empact commented at 12:33 AM on January 25, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/15238/commits/b651ef7e1c39a820089695b29d14a07d910a385a

MarcoFalke referenced this in commit 029d28a7aa on Feb 12, 2019

MarcoFalke merged this on Feb 12, 2019

MarcoFalke closed this on Feb 12, 2019

in test/functional/mining_basic.py:18 in b651ef7e1c

  14 | @@ -15,6 +15,7 @@
  15 |  from test_framework.messages import (
  16 |      CBlock,
  17 |      CBlockHeader,
  18 | +    BLOCK_HEADER_SIZE

MarcoFalke commented at 9:58 PM on February 12, 2019:

style-nit: Missing trailing , :eyes:

Sjors commented at 9:01 AM on February 20, 2019: member

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

Fixed in #15439

deadalnix referenced this in commit 5abfa6f26e on Oct 2, 2020

vijaydasmp referenced this in commit 239cba178c on Sep 12, 2021

vijaydasmp referenced this in commit 4884dbe534 on Sep 12, 2021

vijaydasmp referenced this in commit 590a5c999a on Sep 13, 2021

vijaydasmp referenced this in commit a6e089e7e2 on Sep 13, 2021

DrahtBot locked this on Dec 16, 2021