test: Add test for getblockheader verboseness #19722

torhte commented at 6:54 PM on August 14, 2020: contributor

Improve test coverage by adding a test for getblockheader with verbose argument set to false.

torhte renamed this:
~~[test] Add test for getblockheader verboseness~~
test: Add test for getblockheader verboseness
on Aug 14, 2020

DrahtBot added the label Tests on Aug 14, 2020

in test/functional/rpc_blockchain.py:41 in 1b689f50da outdated

  37 | @@ -38,7 +38,7 @@
  38 |      TIME_GENESIS_BLOCK,
  39 |  )
  40 |  from test_framework.messages import (
  41 | -    msg_block,
  42 | +    msg_block, FromHex, CBlockHeader,

theStack commented at 11:56 AM on August 15, 2020:

For multi-line imports (the ones ending with ( in the first line), each import should be in a separate line and they should be sorted alphabetically (see e.g. from...import above in the same file).

torhte commented at 8:10 AM on August 17, 2020:

Thanks!

in test/functional/rpc_blockchain.py:284 in 1b689f50da outdated

 279 | @@ -280,6 +280,14 @@ def _test_getblockheader(self):
 280 |          assert isinstance(int(header['versionHex'], 16), int)
 281 |          assert isinstance(header['difficulty'], Decimal)
 282 |  
 283 | +        # Test with verbose=False, which should return the header as hex.
 284 | +        header_hex = node.getblockheader(blockhash = besthash, verbose = False)

theStack commented at 12:03 PM on August 15, 2020:

It is preferred to pass named arguments without any spaces between the =. See https://www.python.org/dev/peps/pep-0008/#other-recommendations (point 5) and the discussion https://stackoverflow.com/questions/8853063/pep-8-why-no-spaces-around-in-keyword-argument-or-a-default-parameter-value.

torhte commented at 8:10 AM on August 17, 2020:

Thanks!

theStack commented at 12:04 PM on August 15, 2020: contributor

Concept ACK Welcome as a new contributor torhte! 🎉

[test] Add test for getblockheader verboseness

Add test for getblockheader with verbose argument set to false.

5067c5acc3

torhte force-pushed on Aug 17, 2020

theStack approved

theStack commented at 9:03 AM on August 18, 2020: contributor

ACK https://github.com/bitcoin/bitcoin/pull/19722/commits/5067c5acc30c5cf87496c1bf8eb03712cc66b206

DrahtBot cross-referenced this on Aug 20, 2020 from issue test: Remove confusing mininode terminology by jnewbery

DrahtBot commented at 8:27 PM on August 20, 2020: contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19760 (test: Remove confusing mininode terminology by jnewbery)

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.

Coverage

Coverage	Change (pull 19722, 041785c8c5ec4bde09a15c4479b46130b6d95da7)	Reference (master, 0d9e14a6466cd1644cb659b7b79d40d8761034c0)
Lines	+0.0087 %	90.9042 %
Functions	-0.0128 %	85.8475 %
Branches	+0.0076 %	52.6444 %

<sup>Updated at: 2020-08-21T08:01:32.021751.</sup>

MarcoFalke merged this on Aug 21, 2020

MarcoFalke closed this on Aug 21, 2020

torhte deleted the branch on Aug 21, 2020

sidhujag referenced this in commit 2fc3016af4 on Aug 21, 2020

PastaPastaPasta referenced this in commit c8d7993066 on Jun 27, 2021

PastaPastaPasta referenced this in commit 2bdb648768 on Jun 28, 2021

PastaPastaPasta referenced this in commit b0d7642296 on Jun 29, 2021

PastaPastaPasta referenced this in commit eff9cbcafc on Jul 1, 2021

PastaPastaPasta referenced this in commit b422c4a4bc on Jul 1, 2021

PastaPastaPasta referenced this in commit bde1e87c5a on Jul 15, 2021

PastaPastaPasta referenced this in commit 1382c881df on Jul 16, 2021

Fabcien referenced this in commit c7152f710c on Sep 16, 2021

bitcoin locked this on Feb 15, 2022