[tests] Add getmininginfo test #11150

mess110 · 2017-08-25T22:37:50Z

Add getmininginfo functional test in mining.py

promag · 2017-08-25T22:46:05Z

test/functional/rpc_getmininginfo.py

+from test_framework.util import assert_equal
+from decimal import Decimal
+
+


Remove newline.

promag · 2017-08-25T22:46:34Z

test/functional/rpc_getmininginfo.py

+        self.num_nodes = 1
+        self.setup_clean_chain = False
+
+


Remove newline.

promag · 2017-08-25T22:47:35Z

test/functional/rpc_getmininginfo.py

+        self._test_values()
+        self._test_blocks_key_is_equal_to_getblockcount()
+
+


Remove newline.

promag · 2017-08-25T22:47:42Z

test/functional/rpc_getmininginfo.py

+        for key in required_keys:
+            assert(key in self.mining_info)
+
+


Remove newline.

promag · 2017-08-25T22:47:48Z

test/functional/rpc_getmininginfo.py

+        assert_equal(self.mining_info['networkhashps'], networkhashps)
+        assert_equal(self.mining_info['pooledtx'], 0)
+
+


Remove newline.

promag · 2017-08-25T22:47:52Z

test/functional/rpc_getmininginfo.py

+        assert_equal(self.nodes[0].getblockcount(), 200)
+        assert_equal(self.mining_info['blocks'], self.nodes[0].getblockcount())
+
+


Remove newline.

promag · 2017-08-25T22:48:35Z

test/functional/rpc_getmininginfo.py

+
+
+    def run_test(self):
+        self.mining_info = self.nodes[0].getmininginfo()


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

promag · 2017-08-25T22:49:46Z

test/functional/rpc_getmininginfo.py

+        self._test_blocks_key_is_equal_to_getblockcount()
+
+
+    def _test_returns_all_the_keys(self):


There are few with _ prefix, remove?

promag · 2017-08-25T22:52:34Z

test/functional/rpc_getmininginfo.py

+
+
+    def _test_values(self):
+        difficulty = Decimal('4.656542373906925E-10')


Inline below?

promag · 2017-08-25T22:52:40Z

test/functional/rpc_getmininginfo.py

+
+    def _test_values(self):
+        difficulty = Decimal('4.656542373906925E-10')
+        networkhashps = Decimal('0.003333333333333334')


Same a above?

mess110 · 2017-08-25T23:06:40Z

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

jnewbery

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.

jnewbery · 2017-08-28T15:11:21Z

test/functional/rpc_getmininginfo.py

+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+from decimal import Decimal


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

jnewbery · 2017-08-28T15:12:40Z

test/functional/rpc_getmininginfo.py

+            assert(key in mining_info)
+
+    def test_values(self, mining_info):
+        errors = 'This is a pre-release test build - use at your own risk -' \


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.

maflcko · 2017-08-28T15:25:45Z

test/functional/rpc_getmininginfo.py

+        ]
+
+        for key in required_keys:
+            assert(key in mining_info)


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 · 2017-08-28T17:14:34Z

@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 · 2017-08-28T21:47:08Z

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 4f2905b

promag

utACK 4f2905b.

promag · 2017-08-28T22:36:17Z

test/functional/mining.py

 import copy
+from binascii import b2a_hex
+from decimal import Decimal



Nit, remove empty lines?

Ignore as per PEP-8.

4f2905b Add getmininginfo functional test (Cristian Mircea Messel) Pull request description: Add `getmininginfo` functional test in `mining.py` Tree-SHA512: 12be9cfb37e9ac4c6625fc06051704c8a8dfd7271c2654f994c7659c8810e4b7a4335105ae159315308bcd45b65589bab1829bd134d2f4cabf74d63f2e5d22fe

Github-Pull: bitcoin#11150 Rebased-From: 4f2905b

4f2905b Add getmininginfo functional test (Cristian Mircea Messel) Pull request description: Add `getmininginfo` functional test in `mining.py` Tree-SHA512: 12be9cfb37e9ac4c6625fc06051704c8a8dfd7271c2654f994c7659c8810e4b7a4335105ae159315308bcd45b65589bab1829bd134d2f4cabf74d63f2e5d22fe

promag reviewed Aug 25, 2017

View reviewed changes

fanquake added the Tests label Aug 25, 2017

jnewbery reviewed Aug 28, 2017

View reviewed changes

maflcko reviewed Aug 28, 2017

View reviewed changes

Add getmininginfo functional test

4f2905b

mess110 changed the title ~~[tests] Add getmininginfo functional tests~~ [tests] Add getmininginfo test Aug 28, 2017

promag reviewed Aug 28, 2017

View reviewed changes

maflcko merged commit 4f2905b into bitcoin:master Aug 29, 2017

mess110 deleted the test_rpc_getmininginfo branch August 29, 2017 08:44

sipa mentioned this pull request Sep 11, 2017

Fix confusing blockmax{size,weight} options, dont default to throwing away money #11100

Merged

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Oct 3, 2017

Add getmininginfo functional test

847c75e

Github-Pull: bitcoin#11150 Rebased-From: 4f2905b

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

		from test_framework.util import assert_equal
		from decimal import Decimal

		self._test_values()
		self._test_blocks_key_is_equal_to_getblockcount()

		assert_equal(self.mining_info['networkhashps'], networkhashps)
		assert_equal(self.mining_info['pooledtx'], 0)

		assert_equal(self.nodes[0].getblockcount(), 200)
		assert_equal(self.mining_info['blocks'], self.nodes[0].getblockcount())



		def run_test(self):
		self.mining_info = self.nodes[0].getmininginfo()

		self._test_blocks_key_is_equal_to_getblockcount()


		def _test_returns_all_the_keys(self):



		def _test_values(self):
		difficulty = Decimal('4.656542373906925E-10')

[tests] Add getmininginfo test #11150

[tests] Add getmininginfo test #11150

Uh oh!

Conversation

mess110 commented Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mess110 commented Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnewbery Aug 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mess110 commented Aug 28, 2017

Uh oh!

jnewbery commented Aug 28, 2017

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mess110 commented Aug 25, 2017 •

edited

Loading

mess110 commented Aug 25, 2017 •

edited

Loading

jnewbery Aug 28, 2017 •

edited

Loading