test: add functional test for TestShell (matching doc example) #33546

1. 
   theStack commented at 0:31 am on October 6, 2025: contributor

   This PR adds a functional framework test for the TestShell class. The primary motivation for this is to avoid that the example instructions for the interactive Python shell in test-shell.md get outdated or broken without noticing, a problem we had already several times in the past (see #26520, #27906, #31415). Having a copy is still not perfect, as docs and functional test have to be kept in sync, but I don’t expect this to be a problem in practice, assuming the hint in the functional test comment is hopefully noticed if changes are made.

   Alternatively, the example instructions in test-shell.md could be removed with a hint to the functional test (I tend to prefer to keep the docs as-is though, showing the full REPL interaction).

   The first commit contain two small removal fix-ups in test-shell.md regarding the result of the createwallet RPC call and the mentioning of the noshutdown option that was removed earlier (see #31620). Could be backported to v30.

2. DrahtBot added the label Tests on Oct 6, 2025
3. 
   DrahtBot commented at 0:31 am on October 6, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33546.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	brunoerg

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. theStack force-pushed on Oct 6, 2025
5. 
   maflcko commented at 6:19 am on October 6, 2025: member

   Looks like the new test times out in CI?
6. theStack force-pushed on Oct 6, 2025
7. theStack force-pushed on Oct 6, 2025
8. 
   theStack commented at 8:15 am on October 6, 2025: contributor

   Force-pushed with a fix to make the test also work for non-wallet builds. With this, all CI jobs should pass now.

   Looks like the new test times out in CI?

   Hmm, seems like the test just hangs whenever the TestShell instance is not shut down at the end. Wrapped it in an try: ... finally: test.shutdown() block in order to prevent time-outs if any exception would be thrown there.

9. 
   brunoerg commented at 1:25 pm on October 9, 2025: contributor

   Concept ACK
10. doc: test: update TestShell example instructions/options

    The `createwallet` RPC doesn't return the empty passphrase
    warning anymore if no passphrase was passed explicitly.
    The `noshutdown` parameter key was removed in commit
    fa0dc09b9002f0bcae63af6af8d37fb3e0040ef4, so remove it from
    the table.

    53874f7934
11. test: add functional test for `TestShell` (matching doc example) 57f7c68821
12. in test/functional/feature_framework_testshell.py:41 in 9babfb9331 outdated

    36+            test.sync_blocks()
    37+            assert_equal(test.nodes[1].getblockchaininfo()["blocks"], 101)
    38+            assert_equal(test.nodes[0].getbalance(), Decimal('50.0'))
    39+            test.nodes[0].log.info("Successfully mined regtest chain!")
    40+    finally:
    41+        test.shutdown()
    

    ---

    ---

    

    brunoerg commented at 1:59 pm on October 9, 2025:

    After the shutdown, we could reset test and check if num_nodes has been reset. E.g.:

    0     finally:
    1         test.shutdown()
    2+        test.reset()
    3+        assert test.num_nodes is None
    

    ---

    

    theStack commented at 3:27 pm on October 9, 2025:

    Thanks, done.
13. theStack force-pushed on Oct 9, 2025
14. theStack force-pushed on Oct 9, 2025
15. brunoerg approved
16. 
    brunoerg commented at 4:43 pm on October 9, 2025: contributor

    ACK 57f7c68821d96cc096db624cb06b7a252d038300

Contributors
theStack DrahtBot maflcko brunoerg

Labels
Tests