Adds a new functional test for testing various command line and configuration file argument interactions, that aren't specific enough to other functionality to be placed in other tests.
Currently this tests the error messages for non-existent datadir, which would have caught the bug fixed in #11829. It also tests that command line arguments override the ones in the config file.
I plan on working on a fix for #11819 / #1044 and then expanding this test with cases for that.
15 | + self.num_nodes = 1 16 | + 17 | + def run_test(self): 18 | + self.stop_node(0) 19 | + # Remove the -datadir argument so it doesn't override the config file 20 | + self.nodes[0].args = [arg for arg in self.nodes[0].args if arg.startswith("-datadir")]
Did you mean if **not** arg.startswith("-datadir")?
Doh, yes, oops :)
38 | + self.stop_node(0) 39 | + 40 | + # Ensure command line argument overrides datadir in conf 41 | + os.mkdir(new_data_dir_2) 42 | + self.start_node(0, ['-datadir='+new_data_dir_2, '-wallet=w2']) 43 | + assert(os.path.isfile(os.path.join(new_data_dir_2, 'wallets', 'w2')))
femto-nit: missing space after assert, redundant round braces.
14 | + self.num_nodes = 1 15 | + 16 | + def run_test(self): 17 | + self.stop_node(0) 18 | + # Remove the -datadir argument so it doesn't override the config file 19 | + self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")]
Note that the configuration is written to the bitcoin.conf in the datadir that you are removing from the args. I suspect this will cause issues.
Hint: git grep 'def initialize_datadir'
~Yeah I've been trying to figure a way around this, if -datadir is specified on the command line it will override the conf file, but if it isn't then it can't find the conf file anyway. Still trying to think if this is possible or not.~ EDIT: Fixed
20 | + 21 | + default_data_dir = os.path.join(self.options.tmpdir, 'node0') 22 | + new_data_dir = os.path.join(default_data_dir, 'newdatadir') 23 | + new_data_dir_2 = os.path.join(default_data_dir, 'newdatadir2') 24 | + 25 | + # Check that using -datadir argument on non-existant directory fails
spelling existent
16 | + def run_test(self): 17 | + self.stop_node(0) 18 | + # Remove the -datadir argument so it doesn't override the config file 19 | + self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")] 20 | + 21 | + default_data_dir = os.path.join(self.options.tmpdir, 'node0')
Maybe call get_datadir_path(self.options.tmpdir, 0) to avoid hardcoding this.
utACK 7dc6a4253ea1c101b6b0591e4c9a84b9015d5891
Addressed @ryanofsky comments and squashed fixup commit, thanks :)
22 | + default_data_dir = get_datadir_path(self.options.tmpdir, 0) 23 | + new_data_dir = os.path.join(default_data_dir, 'newdatadir') 24 | + new_data_dir_2 = os.path.join(default_data_dir, 'newdatadir2') 25 | + 26 | + # Check that using -datadir argument on non-existent directory fails 27 | + self.nodes[0].datadir = new_data_dir
Remove?
This is a shame. I don't think individual tests should be reaching into a TestNode object to change the datadir, since that's used by the test framework in various places.
@jnewbery You propose to discard the "old" TestNode and create a fresh one whenever the datadir is changed?
No. The problem here is that it's useful to have a static datadir that files like debug.log and stderr are written to, since that helps with debugging failing scripts.
I agree that it's not a clean solution but it's the only thing I could come up with to make it work, how do you propose otherwise to test the datadir argument?
I think you're right. It's not ideal, but it's as good a way as any to do this.
It might be nice to have a log message at the start of the test warning that not all the node logs will be in the standard location:
self.log.warning("This test uses non-default data directories. Not all node logs will be in the default location.")
but probably not worth a separate PR to add that at this point.
39 | + self.stop_node(0) 40 | + assert os.path.isfile(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) 41 | + 42 | + # Ensure command line argument overrides datadir in conf 43 | + os.mkdir(new_data_dir_2) 44 | + self.nodes[0].datadir = new_data_dir_2
Remove?
Can't remove either of those, they are used by the test runner to connect to the RPC and monitor the output of debug.log and stuff I believe, otherwise it can't find it. It's set separately from any command line arguments to bitcoind
Maybe check if datadir is specified in extra_args in TestNode.start?
utACK be9a13c.
Thank you so much for adding tests for this! utACK be9a13c8a0f149ac219934b515399cca60bf2123
Post-merge:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
utACK be9a13c8a0f149ac219934b515399cca60bf2123
-----BEGIN PGP SIGNATURE-----
iQIcBAEBCgAGBQJaOu+pAAoJENLqSFDnUoslsOcQAIiaYN2oS9TNACNr4wnYBcXW
CY6i4q5Eyfr8CaENaeZNvDoYm92DH+dysVvVXjJoQetxTL+kiEU6xj1MdV24RCcd
IWgqAroaLIGDFbe5DL7J9YOGwUWsO56NtSLKB5vZsaX8HocP/kNh2jvXTaq6Hq8E
tvUev82IWTQQfit15yiO6ETsSiJadN7WPst0tfrj+gqIs67Acrg8hyP2VQ5Q594v
8O7nt0IFUr8TWmYPf/8JrCN3odiEAwva+KSGuYYayQT4bEIBVUxy49D0vfRQMdad
paTVdw7u53lFbcZKyIDzHFPKnU6FDV+wIqtCgtGPWNhFq3p6s4GCxlKHo37WbVdi
cjsm4tC4v7UN0Mor9WACc79gkkooWD32ZqN9iOtkshG6aTJt0kIwLrzOVm60uR6D
HkGj3yLREBYUZsvF4wSngCrdWAreHU9RePRTo8vKfPVTf7sr5LLoe+iBiSlvaviI
5gUUNMxD+UG5nfotBQSNBV7I1WQ48t+wO5WjIW7SKUfVnlGk0FT6C1eNI6JDpW9i
Ln2Mq82pwO16aZKAn+NxXxyQe9huqs1xNNC/TtBqMLbM2Wf96MjhPjTQJTujJBrH
90LAJ35nHawrrxC4RZL5ipkJUIu8ruDITchZomQmYujsO8jvBsQvdrkHmgOPZDOS
RZW+i/hZ8oG1oze/BjYh
=N62V
-----END PGP SIGNATURE-----