[tests] Change invalidblockrequest to use BitcoinTestFramework #11772

pull jnewbery wants to merge 3 commits into bitcoin:master from jnewbery:refactor_invalidblockrequest changing 2 files +54 −69

jnewbery commented at 4:38 PM on November 27, 2017: member
Builds on #11771. Please review that PR first

Next step in #10603.
- first commit tidies up invalidblockrequest.py
- second commit removes usage of ComparisonTestFramework
jnewbery cross-referenced this on Nov 27, 2017 from issue Retire the comparison test framework by jnewbery
jnewbery cross-referenced this on Nov 27, 2017 from issue [tests] Change feature_block.py to use BitcoinTestFramework by jnewbery
fanquake added the label Tests on Nov 27, 2017
jnewbery force-pushed on Nov 30, 2017
jnewbery commented at 4:24 PM on December 2, 2017: member

This requires rebase, but I'll leave it as it is until #11771 is reviewed/merged.
jnewbery cross-referenced this on Dec 2, 2017 from issue [tests] Change feature_csv_activation.py to use BitcoinTestFramework by jnewbery
jnewbery cross-referenced this on Dec 2, 2017 from issue I accidentally [deliberately] killed it [the ComparisonTestFramework] by jnewbery
jnewbery force-pushed on Dec 14, 2017

in test/functional/invalidblockrequest.py:56 in 62bb3a0892 outdated

  79 | -            self.block_time += 1
  80 | -            test.blocks_and_transactions.append([block, True])
  81 | -            height += 1
  82 | -        yield test
  83 | +        best_block = node.getblock(node.getbestblockhash())
  84 | +        tip = int("0x" + node.getbestblockhash(), 0)

ryanofsky commented at 8:09 PM on December 20, 2017:

In commit "Change invalidblockrequest to use BitcoinTestFramework"

Maybe avoid 0x prefix here and above by calling int(hash, 16)

ryanofsky commented at 8:16 PM on December 20, 2017: contributor
Reviewed last two commits only. Other commits are part of #11771 (and out of date).
- utACK 62bb3a0892558cf8813a22e1bf7860598dde3441 [tests] Change invalidblockrequest to use BitcoinTestFramework
- utACK 1d26a167c73b3e94509b3d39fa5875fdb9601eb1 [tests] Fix flake8 warnings in invalidblockrequest
ajtowns cross-referenced this on Jan 16, 2018 from issue [tests] Rename functional tests by ajtowns
jnewbery cross-referenced this on Feb 13, 2018 from issue [tests] Change invalidtxrequest to use BitcoinTestFramework by jnewbery
jnewbery commented at 1:59 PM on February 13, 2018: member
This requires rebase now that #11771 has been merged.

Review comments from @conscott to address in this PR:
- unused update to block_time variable can be removed (https://github.com/bitcoin/bitcoin/pull/11771#discussion_r167710791)
- unused update to height variable can be removed (https://github.com/bitcoin/bitcoin/pull/11771#pullrequestreview-95970049)
- send_txs_and_test() is never called with sucess=True. Either remove that variable or add a successful call to send_txs_and_test() to exercise code path (https://github.com/bitcoin/bitcoin/pull/11771#pullrequestreview-95979452)

[test] Fix nits leftover from 11771

Remove unused variable reassignments in p2p_invalid_tx.py and call
send_txs_and_test() with valid transaction.

54b8c580b7

[tests] Fix flake8 warnings in invalidblockrequest 2b7064eda7

[tests] Change invalidblockrequest to use BitcoinTestFramework

[tests] update tests from changes to mininode in #11771 - added by @conscott

[tests] trivial update to hex conversion for readability - added by @conscott

e97b113b04

jnewbery force-pushed on Feb 14, 2018
jnewbery commented at 12:05 AM on February 14, 2018: member

Thanks to @conscott for rebasing this and fixing up the remaining issues from #11771 and Russ's review comment.
jamesob commented at 10:57 PM on February 14, 2018: member

:+1:

utACK https://github.com/bitcoin/bitcoin/pull/11772/commits/e97b113b049fa1ca7152373d6ea911d56bf7ef4e
laanwj assigned MarcoFalke on Mar 5, 2018
MarcoFalke commented at 8:56 PM on March 13, 2018: member

utACK e97b113b049fa1ca7152373d6ea911d56bf7ef4e
MarcoFalke merged this on Mar 13, 2018
MarcoFalke closed this on Mar 13, 2018
MarcoFalke referenced this in commit 0630974647 on Mar 13, 2018
jnewbery commented at 11:01 PM on March 18, 2018: member

Not an issue with your code, but eq() is not implemented for CTransaction, so the argument to assert is always true, since none of the tx objects are the same reference (they were copied).

Agree - this assert: https://github.com/bitcoin/bitcoin/blob/00d1680498c5550e7db1f359202d3433a092fafd/test/functional/p2p_invalid_block.py#L83 should be removed in a future PR.
jnewbery deleted the branch on Mar 18, 2018
MarcoFalke referenced this in commit 9a2db3b3d5 on Apr 5, 2018
HashUnlimited cross-referenced this on Aug 2, 2018 from issue I accidentally [deliberately] killed it [the ComparisonTestFramework] by HashUnlimited
codablock referenced this in commit 21040f773f on Sep 30, 2019
codablock referenced this in commit 33c77c4039 on Sep 30, 2019
codablock cross-referenced this on Sep 30, 2019 from issue Backport orphan TX tests and dependent PRs by codablock
codablock referenced this in commit e720d73770 on Oct 2, 2019
codablock referenced this in commit d40f2e5825 on Oct 2, 2019
UdjinM6 referenced this in commit beed5cf1f2 on Oct 3, 2019
codablock referenced this in commit 3e72a09c16 on Oct 3, 2019
barrystyle referenced this in commit 2b7f14e055 on Jan 22, 2020
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#10603 Retire the comparison test framework #11771 [tests] Change invalidtxrequest to use BitcoinTestFramework #11773 [tests] Change feature_block.py to use BitcoinTestFramework #11774 [tests] Rename functional tests #11817 [tests] Change feature_csv_activation.py to use BitcoinTestFramework #11818 I accidentally [deliberately] killed it [the ComparisonTestFramework]