[tests] Add getmininginfo test #11150

mess110 commented at 10:37 PM on August 25, 2017: contributor

Add getmininginfo functional test in mining.py

in test/functional/rpc_getmininginfo.py:11 in 3ca4b6bc5b outdated

   6 | +
   7 | +from test_framework.test_framework import BitcoinTestFramework
   8 | +from test_framework.util import assert_equal
   9 | +from decimal import Decimal
  10 | +
  11 | +

promag commented at 10:46 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:18 in 3ca4b6bc5b outdated

  13 | +    def __init__(self):
  14 | +        super().__init__()
  15 | +        self.num_nodes = 1
  16 | +        self.setup_clean_chain = False
  17 | +
  18 | +

promag commented at 10:46 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:26 in 3ca4b6bc5b outdated

  21 | +
  22 | +        self._test_returns_all_the_keys()
  23 | +        self._test_values()
  24 | +        self._test_blocks_key_is_equal_to_getblockcount()
  25 | +
  26 | +

promag commented at 10:47 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:39 in 3ca4b6bc5b outdated

  38 | +        ]
  39 | +
  40 | +        for key in required_keys:
  41 | +            assert(key in self.mining_info)
  42 | +
  43 | +

promag commented at 10:47 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:60 in 3ca4b6bc5b outdated

  55 | +        assert_equal(self.mining_info['difficulty'], difficulty)
  56 | +        assert_equal(self.mining_info['errors'], errors)
  57 | +        assert_equal(self.mining_info['networkhashps'], networkhashps)
  58 | +        assert_equal(self.mining_info['pooledtx'], 0)
  59 | +
  60 | +

promag commented at 10:47 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:57 in 3ca4b6bc5b outdated

  60 | +
  61 | +    def _test_blocks_key_is_equal_to_getblockcount(self):
  62 | +        assert_equal(self.nodes[0].getblockcount(), 200)
  63 | +        assert_equal(self.mining_info['blocks'], self.nodes[0].getblockcount())
  64 | +
  65 | +

promag commented at 10:47 PM on August 25, 2017:

Remove newline.

in test/functional/rpc_getmininginfo.py:20 in 3ca4b6bc5b outdated

  15 | +        self.num_nodes = 1
  16 | +        self.setup_clean_chain = False
  17 | +
  18 | +
  19 | +    def run_test(self):
  20 | +        self.mining_info = self.nodes[0].getmininginfo()

promag commented at 10:48 PM on August 25, 2017:

Don't create self.mining_info, pass to functions below?

in test/functional/rpc_getmininginfo.py:27 in 3ca4b6bc5b outdated

  22 | +        self._test_returns_all_the_keys()
  23 | +        self._test_values()
  24 | +        self._test_blocks_key_is_equal_to_getblockcount()
  25 | +
  26 | +
  27 | +    def _test_returns_all_the_keys(self):

promag commented at 10:49 PM on August 25, 2017:

There are few with _ prefix, remove?

in test/functional/rpc_getmininginfo.py:45 in 3ca4b6bc5b outdated

  40 | +        for key in required_keys:
  41 | +            assert(key in self.mining_info)
  42 | +
  43 | +
  44 | +    def _test_values(self):
  45 | +        difficulty = Decimal('4.656542373906925E-10')

promag commented at 10:52 PM on August 25, 2017:

Inline below?

in test/functional/rpc_getmininginfo.py:46 in 3ca4b6bc5b outdated

  41 | +            assert(key in self.mining_info)
  42 | +
  43 | +
  44 | +    def _test_values(self):
  45 | +        difficulty = Decimal('4.656542373906925E-10')
  46 | +        networkhashps = Decimal('0.003333333333333334')

promag commented at 10:52 PM on August 25, 2017:

Same a above?

fanquake added the label Tests on Aug 25, 2017

mess110 commented at 11:06 PM on August 25, 2017: contributor

@promag thanks for taking a look. Obviously won't leave 2 new lines in the future :smile:

in test/functional/rpc_getmininginfo.py:9 in 554046320e outdated

   0 | @@ -0,0 +1,59 @@
   1 | +#!/usr/bin/env python3
   2 | +# Copyright (c) 2014-2017 The Bitcoin Core developers
   3 | +# Distributed under the MIT software license, see the accompanying
   4 | +# file COPYING or http://www.opensource.org/licenses/mit-license.php.
   5 | +"""Test the getmininginfo API."""
   6 | +
   7 | +from test_framework.test_framework import BitcoinTestFramework
   8 | +from test_framework.util import assert_equal
   9 | +from decimal import Decimal

jnewbery commented at 3:11 PM on August 28, 2017:

supernit: place imports in PEP-8 ordering (std library first, then project imports)

in test/functional/rpc_getmininginfo.py:41 in 554046320e outdated

  36 | +
  37 | +        for key in required_keys:
  38 | +            assert(key in mining_info)
  39 | +
  40 | +    def test_values(self, mining_info):
  41 | +        errors = 'This is a pre-release test build - use at your own risk -' \

jnewbery commented at 3:12 PM on August 28, 2017:

The This is a pre-release test build - use at your own risk error is only presented on non-release branches, so this assert will fail on all release branches. Just omit it.

jnewbery commented at 3:18 PM on August 28, 2017: member

Thanks for this @mess110 . As an alternative to making this an individual test case (which adds a few seconds overhead to running the test suite), you could add this as a sub-test at the top of the mining.py test script.

Whether or not to add a new test script or extend an old one is a judgement call. In this case, I think the getmininginfo RPC can be run at the start of mining.py without any side-effects so it's a good candidate for extending the existing test.

Add getmininginfo functional test 4f2905b76b

in test/functional/rpc_getmininginfo.py:38 in 554046320e outdated

  33 | +            'networkhashps',
  34 | +            'pooledtx',
  35 | +        ]
  36 | +
  37 | +        for key in required_keys:
  38 | +            assert(key in mining_info)

MarcoFalke commented at 3:25 PM on August 28, 2017:

No need to assert for keys in our tests. Python has you covered by throwing a KeyError in case you access a key that does not exist.

mess110 renamed this:
~~[tests] Add getmininginfo functional tests~~
[tests] Add getmininginfo test
on Aug 28, 2017

mess110 commented at 5:14 PM on August 28, 2017: contributor

@jnewbery thanks. I initially started in mining.py but I decided not to put it there because it would be "cleaner" to have its own test. In this particular case, I agree, the extra time to setup the test is not worth it @MarcoFalke thanks for the review and hint

jnewbery commented at 9:47 PM on August 28, 2017: member

Looks good to me. We could test the actual values returned by getmininginfo more thoroughly (ie are the values for size, weight, hashps, etc correct under different circumstances) in a different PR. Testing the interface correctness in this PR is better than no testing at all.

Tested ACK 4f2905b76b502a9de235dbe739e5fe504ef5bc22

in test/functional/mining.py:14 in 4f2905b76b

  10 |  
  11 | -from binascii import b2a_hex
  12 |  import copy
  13 | +from binascii import b2a_hex
  14 | +from decimal import Decimal
  15 |

promag commented at 10:36 PM on August 28, 2017:

Nit, remove empty lines?

promag commented at 10:41 PM on August 28, 2017:

Ignore as per PEP-8.

promag commented at 10:40 PM on August 28, 2017: member

utACK 4f2905b.

MarcoFalke merged this on Aug 29, 2017

MarcoFalke closed this on Aug 29, 2017

MarcoFalke referenced this in commit a90e6d2bff on Aug 29, 2017

mess110 deleted the branch on Aug 29, 2017

MarcoFalke referenced this in commit 847c75ec64 on Oct 3, 2017

PastaPastaPasta referenced this in commit cd6753377a on Sep 19, 2019

PastaPastaPasta referenced this in commit 8578c9ee52 on Sep 23, 2019

PastaPastaPasta referenced this in commit 8d8aba8540 on Sep 24, 2019

codablock referenced this in commit 37250c02e2 on Sep 24, 2019

barrystyle referenced this in commit 8785d7b377 on Jan 22, 2020

MarcoFalke locked this on Sep 8, 2021