Skip to content

Commit

Permalink
Merge bitcoin#30714: test: fix TestShell initialization (late follo…
Browse files Browse the repository at this point in the history
…w-up for bitcoin#30463)

bd7ce05 test: fix `TestShell` initialization (late follow-up for bitcoin#30463) (Sebastian Falbesoner)

Pull request description:

  Creating a `TestShell` instance as stated in the [docs](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test-shell.md) currently fails on master:
  ```
  $ python3
  Python 3.10.13 (main, Mar 15 2024, 07:36:23) [Clang 16.0.6 ] on openbsd7
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import sys
  >>> sys.path.insert(0, "/home/thestack/bitcoin/test/functional")
  >>> from test_framework.test_shell import TestShell
  >>> test = TestShell().setup(num_nodes=2, setup_clean_chain=True)
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "/home/thestack/bitcoin/test/functional/test_framework/test_shell.py", line 70, in __new__
      TestShell.instance = TestShell.__TestShell()
  TypeError: BitcoinTestFramework.__init__() missing 1 required positional argument: 'test_file'
  ```
  Since bitcoin#30463, BitcoinTestFramework instances expect the path of the calling test at construction, in order to find shared data like the configuration (config.ini) and the cache. Note that in contrast to actual functional tests, we can't simply pass `__file__` here, as the test shell module sits within the `test_framework` subfolder, so we have to navigate up to the parent directory and append some dummy test file name.

  On the long-term we should probably add some TestShell instantation smoke-test to detect issues like this early. As I'm not too familiar with the CI I'm not sure what is a good way to achieve this (a functional test obviously can't be used, as that's already a BitcoinTestFramework test in itself), but happy to take suggestions.

ACKs for top commit:
  ismaelsadeeq:
    Tested ACK bd7ce05
  danielabrozzoni:
    tACK bd7ce05
  brunoerg:
    ACK bd7ce05

Tree-SHA512: c3a2365e2cda48a233ee724673c490787981354914f33e10eadbbad9c68e8403d84c5551229a611401e743886539de380ba4bfcb77032b6c85731e3bbe962dc1
  • Loading branch information
glozow committed Aug 29, 2024
2 parents 7349d57 + bd7ce05 commit 1e48238
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion test/functional/test_framework/test_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
# Copyright (c) 2019-2022 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
import pathlib

from test_framework.test_framework import BitcoinTestFramework


class TestShell:
"""Wrapper Class for BitcoinTestFramework.
Expand Down Expand Up @@ -67,7 +69,13 @@ def __new__(cls):
# This implementation enforces singleton pattern, and will return the
# previously initialized instance if available
if not TestShell.instance:
TestShell.instance = TestShell.__TestShell()
# BitcoinTestFramework instances are supposed to be constructed with the path
# of the calling test in order to find shared data like configuration and the
# cache. Since TestShell is meant for interactive use, there is no concrete
# test; passing a dummy name is fine though, as only the containing directory
# is relevant for successful initialization.
tests_directory = pathlib.Path(__file__).resolve().parent.parent
TestShell.instance = TestShell.__TestShell(tests_directory / "testshell_dummy.py")
TestShell.instance.running = False
return TestShell.instance

Expand Down

0 comments on commit 1e48238

Please sign in to comment.