Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot run multiple instances of TestWrapper #20

Closed
jachiang opened this issue Sep 13, 2019 · 3 comments
Closed

Cannot run multiple instances of TestWrapper #20

jachiang opened this issue Sep 13, 2019 · 3 comments
Assignees

Comments

@jachiang
Copy link
Contributor

jachiang commented Sep 13, 2019

Running setup() on multiple instances of TestWrapper results in a AssertionError:

>>> test = util.TestWrapper()
>>> test2 = util.TestWrapper()
>>> test.setup()
2019-09-13T18:08:04.389000Z TestFramework./var/folders/x6/74wk_20d3t90825bykj23y780000gn/T/bitcoin_func_test_0hnz1ze_ (INFO): Initializing test directory /var/folders/x6/74wk_20d3t90825bykj23y780000gn/T/bitcoin_func_test_0hnz1ze_
>>> test2.setup()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jamesc/Dropbox/repos/taproot/util.py", line 83, in setup
    super(TestWrapper,self).setup()
  File "/Users/jamesc/Dropbox/repos/test/bitcoin/test/functional/test_framework/test_framework.py", line 196, in setup
    self.network_thread = NetworkThread()
  File "/Users/jamesc/Dropbox/repos/test/bitcoin/test/functional/test_framework/mininode.py", line 469, in __init__
    assert not self.network_event_loop
AssertionError

This occurs because we are initialising a network thread twice. Initialising with a different thread for each TestWrapper object requires refactor of NetworkThread class in mininode.py.

This has been added as a todo to the TestFramework main method refactor PR.

@jnewbery
Copy link
Contributor

I think we don't want users to be able to create multiple TestWrapper instances (and it shouldn't be possible since #17).

@jachiang - is there a reason you want to have more than one instance of TestWrapper?

@jachiang
Copy link
Contributor Author

@jachiang - is there a reason you want to have more than one instance of TestWrapper?

@jnewbery - No reason for the workshop. It's more that I don't see any reason why it shouldn't be possible to run multiple instances of TestWrapper - If a required resource (e.g. port) is not available due to user misconfiguration, setup() should raise an exception. I think the initial design intention was to limit a single network thread per test run. It would seem cleaner to extend that to a single network thread per test instance.

I definitely think #17 is the best solution for the upcoming workshop. Perhaps for the master PR, it feels like cleaner code to allow for multiple instances. There may be some future use-case in running multiple test instances for benchmarking node cluster configurations?

@jnewbery
Copy link
Contributor

Closing this since it's not needed for the workshop. You can follow up with a PR in the bitcoind repo if you think it's important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants