Improve getchaintxstats test coverage #12083

pull promag wants to merge 2 commits into bitcoin:master from promag:2018-01-getchaintxstats changing 2 files +28 −20

promag commented at 1:05 AM on January 4, 2018: member

No description provided.
fanquake added the label RPC/REST/ZMQ on Jan 4, 2018
fanquake added the label Tests on Jan 4, 2018
promag force-pushed on Jan 4, 2018
promag force-pushed on Jan 4, 2018
promag force-pushed on Jan 5, 2018

in test/functional/blockchain.py:116 in 5b95f96871 outdated

 111 | +        # Test `getchaintxstats` invalid `blockhash`
 112 | +        assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0)
 113 | +        assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0')
 114 | +        blockhash = self.nodes[0].generate(1)[0];
 115 | +        self.nodes[0].invalidateblock(blockhash)
 116 | +        assert_raises_rpc_error(-8, "Block is not in main chain", self.nodes[0].getchaintxstats, blockhash=blockhash)

ajtowns commented at 4:43 AM on January 11, 2018:

Wouldn't it be better to do:

blockhash = self.nodes[0].getblockhash(200)
self.nodes[0].invalidateblock(blockhash)
assert_raises_rpc_error()
self.nodes[0].reconsiderblock(blockhash)

and drop the invalidateblock in _test_gettxoutsetinfo()?

promag commented at 3:15 PM on January 15, 2018:

Done, I'll squash once it's green.

ajtowns approved
ajtowns commented at 4:44 AM on January 11, 2018: contributor

ACK 5b95f96871a33aada918c54c4eff263cb3c55fec -- refactoring looks like it shouldn't change the logic; extra test cases look good.
promag commented at 3:16 PM on January 15, 2018: member

@ajtowns Thanks for the review.

refactoring looks like it shouldn't change the logic

If this is a concern for others, I can take that commit to a new PR.
ajtowns cross-referenced this on Jan 16, 2018 from issue [tests] Rename functional tests by ajtowns
ryanofsky commented at 10:20 PM on February 5, 2018: contributor

utACK 590aa8f1b56cf1a8d82bff41a513cf5f3f530e4a
promag force-pushed on Feb 15, 2018
rpc: Refactor blockhash parse in getchaintxstats 501b43921c
qa: Improve getchaintxstats functional test 57e6786203
promag force-pushed on Feb 15, 2018
promag commented at 9:45 PM on February 15, 2018: member

Rebased to fix conflict of test file test/functional/rpc_blockchain.py rename.

Best reviewed with ?w=1.
laanwj merged this on Feb 26, 2018
laanwj closed this on Feb 26, 2018
laanwj referenced this in commit 228b086b9a on Feb 26, 2018
PastaPastaPasta referenced this in commit 68d820399d on Jun 10, 2020
PastaPastaPasta referenced this in commit b24d5eceaf on Jun 12, 2020
PastaPastaPasta referenced this in commit 2e2ca7ad64 on Jun 13, 2020
PastaPastaPasta referenced this in commit f6a53453cb on Jun 14, 2020
PastaPastaPasta referenced this in commit 169db16be5 on Jun 14, 2020
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11774 [tests] Rename functional tests