test: create default wallet in extended tests #19971

1. 
   Sjors commented at 10:41 am on September 18, 2020: member

   This was omitted from #15454
2. fanquake added the label Tests on Sep 18, 2020
3. 
   jnewbery commented at 11:00 am on September 18, 2020: member

   Can these be fixed by adding the -wallet= command line argument, like the other tests in #15454?
4. 
   promag commented at 11:11 am on September 18, 2020: member

   @jnewbery if we ditch automatic wallet creation then we could already avoid that approach.
5. Sjors force-pushed on Sep 18, 2020
6. 
   Sjors commented at 12:12 pm on September 18, 2020: member

   @jnewbery done
7. 
   jnewbery commented at 1:18 pm on September 18, 2020: member

   utACK 165b8fd561

   Thanks for the quick fix!

8. in test/functional/feature_dbcrash.py:79 in 165b8fd561 outdated

   74@@ -75,7 +75,8 @@ def skip_test_if_missing_module(self):
   75     def setup_network(self):
   76         self.add_nodes(self.num_nodes, extra_args=self.extra_args)
   77         self.start_nodes()
   78-        self.import_deterministic_coinbase_privkeys()
   79+        for n in self.nodes:
   80+            self.import_deterministic_coinbase_privkeys()
   

   ---

   ---

   

   ryanofsky commented at 2:49 pm on September 18, 2020:

   In commit “test: create default wallet in extended tests” (165b8fd5617e4bd4d5f99586306a66f084d95d13)

   Could you add a comment here? It’s unclear why you’d want to call the same function n times.

   ---

   

   Sjors commented at 3:53 pm on September 18, 2020:

   That’s a leftover bug from my refactor.
9. ryanofsky approved
10. 
    ryanofsky commented at 2:50 pm on September 18, 2020: member

    Code review ACK 165b8fd5617e4bd4d5f99586306a66f084d95d13
11. 
    ryanofsky commented at 3:03 pm on September 18, 2020: member

    Previous version 9df44e0b97ba74b4df01e600b4c19e9d75310498 also looked good, but John’s suggestion to use -wallet="" is good because it provides more consistency with other tests so they be cleaned up easier later. There are a few ways tests could conceivably create default wallets:

    1. Not pass any -wallet arguments or call createwallet RPC, just rely on bitcoind default wallet creation (only possible before #15454)
    2. Pass ["-wallet="] argument and rely on test framework to strip out the argument and turn it into a createwallet RPC call behind the scenes
    3. Just call createwallet RPC directly from the test
    4. Give test framework a wants_default_wallet or similar test setup option, so tests have a more direct way of saying whether they want a default wallet or not and we can get rid of the “-wallet=” interception stuff.

    #15454 moved us from (1) to (2). Originally this PR used approach (3) for extended tests, now it uses (2). Probably we want to end up doing (4)

12. test: create default wallet in extended tests

    This was omitted from #15454

    a5f5374b43
13. Sjors force-pushed on Sep 18, 2020
14. 
    Sjors commented at 3:55 pm on September 18, 2020: member

    Third time’s a charm.
15. ryanofsky approved
16. 
    ryanofsky commented at 3:59 pm on September 18, 2020: member

    Code review ACK a5f5374b43275ce4529c3a44c4b44e478d35e079. Just reverted a leftover diff since last review
17. 
    glozow commented at 11:16 pm on September 18, 2020: member

    utACK https://github.com/bitcoin/bitcoin/pull/19971/commits/a5f5374b43275ce4529c3a44c4b44e478d35e079
18. fanquake merged this on Sep 19, 2020
19. fanquake closed this on Sep 19, 2020

    ---

20. Sjors deleted the branch on Sep 19, 2020
21. Fabcien referenced this in commit 407828747f on Oct 7, 2021
22. DrahtBot locked this on Feb 15, 2022

Contributors
Sjors jnewbery promag ryanofsky glozow

Labels
Tests