assumevalid.py was merged as part of PR 9484, but was not added to the test_runner, so is not run even as part of the extended tests.
This commit adds assumevalid to the list of tests in test_runner. It also makes the code in assumevalid considerably clearer.
assumevalid was merged as part of PR 9484, but was not added to the
test_runner, so is not run even as part of the extended tests.
This commit adds assumevalid to the list of tests in test_runner. It
also clarifies the code in assumevalid considerably.
utACK. We need to get in the habit of testing the tests by breaking the code, -- I do this sometimes, but clearly not enough.
Concept ACK. I was wondering if it makes sense to put the rpc tests in a separate folder and then run a check in test_runner, that all files in the folder are mentioned at least once in the test_runner file.
@MarcoFalke - I've already got a branch that does almost that (in my version I added an extra list NON_TESTS to test_runner and then check that all files in the functional directory appear in one of the test lists or NON_TESTS.
I think your version is tidier. I'm happy to rework my branch or review a PR if you want to implement it. Whichever you prefer.
utACK 717ad131f6b665b2e7cf55db689da9616e727d78
I was wondering if it makes sense to put the rpc tests in a separate folder
Not sure about this. I agree that a way to identify 'orphan' tests would be great, however having scripts in multiple folders is unfortunately kind of annoying with Python and its automatic directory hierarchy for packages. Also it means having to think/search where the test is when manually running it. I think I prefer one directory with all the executable scripts.
The test is failing intermittently on master, it seems:
2017-03-27 14:09:57.475000 TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 150, in main
self.run_test()
File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/assumevalid.py", line 197, in run_test
assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202)
File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/util.py", line 476, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(1051 == 2202)
200 | + # Send all blocks to node1. All blocks will be accepted. 201 | for i in range(2202): 202 | node1.send_message(msg_block(self.blocks[i])) 203 | - node1.sync_with_ping() # make sure the most recent block is synced 204 | + node1.sync_with_ping() # make sure the most recent block is synced 205 | assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202)
using assert_blockchain_height should prevent it to fails intermittently. @jnewbery thoughts ?
@NicolasDorier you may be right. I think that sync_with_ping() should be enough to make sure the node is sync'ed to the tip (p2p messages are processed sequentially and single threaded by bitcoind, so it shouldn't send a pong back until it's worked through all the block messages).
Sadly there aren't quite enough logs to see what's going on here. I'm going to leave this as it is for a few more nightly runs to see if we can get a better set of diags. This is in the extended test suite so it won't cause PR or merge travis builds to fail.
If I can't get to the bottom of why this is failing, I'll try with your change, but I'd like to understand what the problem is before trying your fix.
sigh. sync_with_ping() doesn't assert that the node has sync'ed. It's up to the caller to assert on the return value. Sadly, none of the test cases that call sync_with_ping() actually assert on the return value. Proposed fix here: https://github.com/bitcoin/bitcoin/pull/10114