test: Add generate* calls to test framework #22741

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2108-testSync01 changing 14 files +99 −84

MarcoFalke commented at 7:59 AM on August 19, 2021: member

This is needed for #22567.

By making the calls to generate* member function of the test framework, it paves the way to make it easier to implicitly call the sync_all member function.
fanquake added the label Tests on Aug 19, 2021
DrahtBot commented at 8:32 AM on August 19, 2021: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #22543 (test: Use MiniWallet in mempool_limit.py by ShubhamPalriwala)
- #21862 (test: Set regtest.BIP65Height = 111 to speed up tests by MarcoFalke)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
in test/functional/test_framework/util.py:448 in fa3bb4f705 outdated
```
 444 | @@ -445,10 +445,10 @@ def find_output(node, txid, amount, *, blockhash=None):
 445 |  
 446 |  # Helper to create at least "count" utxos
 447 |  # Pass in a fee that is sufficient for relay and mining new transactions.
 448 | -def create_confirmed_utxos(fee, node, count):
 449 | +def create_confirmed_utxos(test_framework, fee, node, count):
```
jnewbery commented at 11:36 AM on August 19, 2021:

It feels slightly strange passing the TestFramework object into these functions in util.py.

Hopefully longer term, create_confirmed_utxos() will be replaced by a miniwallet function, and mine_large_block() and generate_to_height() can be moved to the single files that use those functions.

MarcoFalke commented at 12:13 PM on October 6, 2021:

generate_to_height is removed, but the others remain for now
jnewbery commented at 1:07 PM on August 19, 2021: member

My intention when creating TestNode was that it would be a batteries included programmatic API to the node. It's used by the test framework, but can easily be reused for manual testing, stress testing, etc, since it has no dependencies on the framework. I've personally used it to control a mainnet node with a few tweaks.

This branch introduces a dependency from MiniWallet on TestFramework. I think that's a shame, since it'd be nice if MiniWallet could be used independently from the test framework for things like manual testing, like TestNode can.

I've made a slight modification to this branch here: https://github.com/jnewbery/bitcoin/tree/pr22741.1 which doesn't add that dependency. Let me know what you think.
MarcoFalke force-pushed on Aug 19, 2021
test: Add generate* calls to test framework faf7e92804
in test/functional/test_framework/test_framework.py:624 in 45817a7239 outdated
```
 618 | @@ -619,6 +619,26 @@ def join_network(self):
 619 |          self.connect_nodes(1, 2)
 620 |          self.sync_all()
 621 |  
 622 | +    def generate(self, generator, nblocks, **kwargs):
 623 | +        """
 624 | +        Genarate nblocks blocks.
```
MarcoFalke commented at 1:31 PM on August 19, 2021:

in commit 45817a7239cd5c0ef9126fbdf33bc72283eafa3a: Typo.

Also the signature can now be generalized to *args, **kwargs?

jnewbery commented at 1:41 PM on August 19, 2021:

no ragrets
MarcoFalke commented at 1:31 PM on August 19, 2021: member

Thanks @jnewbery. I've pushed your branch and left a nit comment.
MarcoFalke force-pushed on Aug 19, 2021
in test/functional/feature_rbf.py:28 in fa855ae77f outdated
```
  24 | @@ -25,7 +25,7 @@
  25 |  MAX_REPLACEMENT_LIMIT = 100
  26 |  
  27 |  
  28 | -def make_utxo(node, amount, confirmed=True, scriptPubKey=DUMMY_P2WPKH_SCRIPT):
  29 | +def make_utxo(test_framework, node, amount, confirmed=True, scriptPubKey=DUMMY_P2WPKH_SCRIPT):
```
jnewbery commented at 1:46 PM on August 19, 2021:

May as well make this a method on ReplaceByFeeTest now that it's taking a test_framework as its first argument.

MarcoFalke commented at 2:16 PM on August 19, 2021:

Leaving this as a follow-up along with #22741 (review) to minimize the diff in this pull.

jnewbery commented at 3:10 PM on August 19, 2021:

makes sense!

MarcoFalke commented at 5:07 PM on August 19, 2021:

Actually the diff won't increase with the options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space, so I went ahead to push the changes.

Also, I've reduced the other changes in this commit, because they can be done by scripted-diffs later on.
jnewbery commented at 1:48 PM on August 19, 2021: member

utACK fa855ae77f759e1db50c742bc6f3ef8e230c92dd

Use generate* from TestFramework

The changes in feature_rbf can be reviewed with
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

fab2e23b57

MarcoFalke force-pushed on Aug 19, 2021
jnewbery commented at 5:28 PM on August 19, 2021: member

utACK fab2e23b579c44f1b4bed4d813bbe5a21eaa22c8
fanquake merged this on Aug 24, 2021
fanquake closed this on Aug 24, 2021
MarcoFalke deleted the branch on Aug 24, 2021
sidhujag referenced this in commit 412a8ed6f9 on Aug 24, 2021
Vasundhara383 referenced this in commit 811d14ff1d on Apr 6, 2022
Vasundhara383 referenced this in commit 8c8c1b7458 on Apr 6, 2022
DrahtBot locked this on Oct 30, 2022

Contributors

Labels

Linked (view graph)

#22567 test: Implicitly sync after generate* to preempt races and intermittent test failures

test: Add generate* calls to test framework #22741

Conflicts