Changeset View
Standalone View
test/functional/test_framework/test_framework.py
Show All 40 Lines | class TestStatus(Enum): | ||||
FAILED = 2 | FAILED = 2 | ||||
SKIPPED = 3 | SKIPPED = 3 | ||||
TEST_EXIT_PASSED = 0 | TEST_EXIT_PASSED = 0 | ||||
TEST_EXIT_FAILED = 1 | TEST_EXIT_FAILED = 1 | ||||
TEST_EXIT_SKIPPED = 77 | TEST_EXIT_SKIPPED = 77 | ||||
# Timestamp is 01.01.2019 | |||||
TIMESTAMP_IN_THE_PAST = "1546300800" | |||||
class BitcoinTestFramework(): | class BitcoinTestFramework(): | ||||
"""Base class for a bitcoin test script. | """Base class for a bitcoin test script. | ||||
Individual bitcoin test scripts should subclass this class and override the set_test_params() and run_test() methods. | Individual bitcoin test scripts should subclass this class and override the set_test_params() and run_test() methods. | ||||
Individual tests can also override the following methods to customize the test setup: | Individual tests can also override the following methods to customize the test setup: | ||||
Show All 36 Lines | def main(self): | ||||
parser.add_argument("--coveragedir", dest="coveragedir", | parser.add_argument("--coveragedir", dest="coveragedir", | ||||
help="Write tested RPC commands into this directory") | help="Write tested RPC commands into this directory") | ||||
parser.add_argument("--configfile", dest="configfile", | parser.add_argument("--configfile", dest="configfile", | ||||
help="Location of the test framework config file") | help="Location of the test framework config file") | ||||
parser.add_argument("--pdbonfailure", dest="pdbonfailure", default=False, action="store_true", | parser.add_argument("--pdbonfailure", dest="pdbonfailure", default=False, action="store_true", | ||||
help="Attach a python debugger if test fails") | help="Attach a python debugger if test fails") | ||||
parser.add_argument("--usecli", dest="usecli", default=False, action="store_true", | parser.add_argument("--usecli", dest="usecli", default=False, action="store_true", | ||||
help="use bitcoin-cli instead of RPC for all commands") | help="use bitcoin-cli instead of RPC for all commands") | ||||
parser.add_argument("--with-greatwallactivation", dest="greatwallactivation", default=False, action="store_true", | |||||
help="Activate great wall update on timestamp {}".format(TIMESTAMP_IN_THE_PAST)) | |||||
self.add_options(parser) | self.add_options(parser) | ||||
self.options = parser.parse_args() | self.options = parser.parse_args() | ||||
self.set_test_params() | self.set_test_params() | ||||
assert hasattr( | assert hasattr( | ||||
self, "num_nodes"), "Test must set self.num_nodes in set_test_params()" | self, "num_nodes"), "Test must set self.num_nodes in set_test_params()" | ||||
PortSeed.n = self.options.port_seed | PortSeed.n = self.options.port_seed | ||||
▲ Show 20 Lines • Show All 132 Lines • ▼ Show 20 Lines | def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None): | ||||
extra_args = [[]] * num_nodes | extra_args = [[]] * num_nodes | ||||
if binary is None: | if binary is None: | ||||
binary = [None] * num_nodes | binary = [None] * num_nodes | ||||
assert_equal(len(extra_args), num_nodes) | assert_equal(len(extra_args), num_nodes) | ||||
assert_equal(len(binary), num_nodes) | assert_equal(len(binary), num_nodes) | ||||
for i in range(num_nodes): | for i in range(num_nodes): | ||||
self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, rpc_port=rpc_port(i), p2p_port=p2p_port(i), | self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, rpc_port=rpc_port(i), p2p_port=p2p_port(i), | ||||
timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli)) | timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli)) | ||||
if(self.options.greatwallactivation): | |||||
deadalnix: No need for parenthesis. | |||||
self.nodes[-1].args.append( | |||||
"-greatwallactivationtime={}".format(TIMESTAMP_IN_THE_PAST)) | |||||
deadalnixUnsubmitted Not Done Inline ActionsThis seems like it would override the value for any test that want to set it explicitly. deadalnix: This seems like it would override the value for any test that want to set it explicitly. | |||||
FabienAuthorUnsubmitted Done Inline ActionsThe node has 2 attributes, args and extra_args. The tests can set the latter one. Fabien: The node has 2 attributes, `args` and `extra_args`. The tests can set the latter one.
When the… | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsI think Amaury is pointing out that by altering the args after creating the node, setting -greatwallactivationtime overrides values that are set explicitly in tests. This is probably not the intent, as the test should pass in the state it was set at. However, I see a problem with this approach no matter how you slice it:
Given the above, I think #1 is the right way to go, as tests that need to be updated can be easily grep'd for. Our main concern is what breaks on flagday. jasonbcox: I think Amaury is pointing out that by altering the args after creating the node, setting `… | |||||
jasonbcoxUnsubmitted Not Done Inline ActionsAh I see. Taking a look at the implementation of TestNode, #1 is precisely what you have done. self.args is set as part of TestNode's init and the self.args + self.extra_args do not get appended until the node has started with a call to start(). In that case, this implementation is relying on behavior internal to TestNode. There are two much clearer ways to deal with this:
jasonbcox: Ah I see. Taking a look at the implementation of TestNode, #1 is precisely what you have done. | |||||
FabienAuthorUnsubmitted Done Inline Actions
So far the best approach imo is to refactor the TestNode to add accessors for self.args, maybe renaming it to clarify its intent. This would make the changes no longer rely on TestNode internal behavior. Fabien: # Prepending `self.extra_args` is likely to fail, because the `self.extra_args` that are set… | |||||
def start_node(self, i, *args, **kwargs): | def start_node(self, i, *args, **kwargs): | ||||
"""Start a bitcoind""" | """Start a bitcoind""" | ||||
node = self.nodes[i] | node = self.nodes[i] | ||||
node.start(*args, **kwargs) | node.start(*args, **kwargs) | ||||
node.wait_for_rpc_connection() | node.wait_for_rpc_connection() | ||||
▲ Show 20 Lines • Show All 162 Lines • ▼ Show 20 Lines | def _initialize_chain(self): | ||||
shutil.rmtree(os.path.join( | shutil.rmtree(os.path.join( | ||||
self.options.cachedir, "node" + str(i))) | self.options.cachedir, "node" + str(i))) | ||||
# Create cache directories, run bitcoinds: | # Create cache directories, run bitcoinds: | ||||
for i in range(MAX_NODES): | for i in range(MAX_NODES): | ||||
datadir = initialize_datadir(self.options.cachedir, i) | datadir = initialize_datadir(self.options.cachedir, i) | ||||
args = [os.getenv("BITCOIND", "bitcoind"), "-server", | args = [os.getenv("BITCOIND", "bitcoind"), "-server", | ||||
"-keypool=1", "-datadir=" + datadir, "-discover=0"] | "-keypool=1", "-datadir=" + datadir, "-discover=0"] | ||||
if(self.options.greatwallactivation): | |||||
deadalnixUnsubmitted Not Done Inline Actionsdito deadalnix: dito | |||||
args.append( | |||||
"-greatwallactivationtime={}".format(TIMESTAMP_IN_THE_PAST)) | |||||
deadalnixUnsubmitted Not Done Inline ActionsI'm not sure we want to cache command line arguments. Or if so, then there needs to be some cache invalidation logic. Thinking about this more, it seems like this is the proper approach: add a facility to add elements to the config file used by the node. deadalnix: I'm not sure we want to cache command line arguments. Or if so, then there needs to be some… | |||||
FabienAuthorUnsubmitted Done Inline ActionsThe arguments caching thing is already built in TestNode. There is no facility to access it, so the args attribute is accessed directly when needed (here and in feature_conf_args.py to remove the -datadir argument). This can be easity refactored if needed. Fabien: The arguments caching thing is already built in TestNode. There is no facility to access it, so… | |||||
if i > 0: | if i > 0: | ||||
args.append("-connect=127.0.0.1:" + str(p2p_port(0))) | args.append("-connect=127.0.0.1:" + str(p2p_port(0))) | ||||
self.nodes.append(TestNode(i, self.options.cachedir, extra_args=[], host=None, rpc_port=rpc_port(i), p2p_port=p2p_port(i), | self.nodes.append(TestNode(i, self.options.cachedir, extra_args=[], host=None, rpc_port=rpc_port(i), p2p_port=p2p_port(i), | ||||
timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) | timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) | ||||
self.nodes[i].args = args | self.nodes[i].args = args | ||||
self.start_node(i) | self.start_node(i) | ||||
# Wait for RPC connections to be ready | # Wait for RPC connections to be ready | ||||
▲ Show 20 Lines • Show All 82 Lines • Show Last 20 Lines |
No need for parenthesis.