test: Change feature_config_args.py not to rely on strange regtest=0 behavior

ryanofsky commented at 10:41 pm on November 21, 2019: member

Update test to simply generate a normal mainnet configuration file instead of using a crazy setup where a regtest=1 config file using an includeconf in the [regtest] section includes another config file that specifies regtest=0, retroactively switching the network to mainnet.

This setup was fragile and only worked because the triggered InitError happened early enough that none of the ignored [regtest] options mattered (only affecting log output).

This change was originally made as part of #17493

fanquake added the label Tests on Nov 21, 2019

DrahtBot commented at 0:42 am on November 22, 2019: member

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

Conflicts

No conflicts as of last run.

laanwj commented at 9:34 pm on November 22, 2019: member

Concept ACK

ryanofsky renamed this:
~~Change feature_config_args.py not to rely on strange regtest=0 behavior~~
test: Change feature_config_args.py not to rely on strange regtest=0 behavior
on Nov 24, 2019

instagibbs commented at 2:34 pm on November 26, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/17556/commits/228fb7ef80cdf806353bc7d4fa2859a7d4dd5d87

in test/functional/test_framework/util.py:310 in 228fb7ef80 outdated

305+def write_config(config_path, n, chain, extra_config=""):
306     # Translate chain name to config name
307     if chain == 'testnet3':
308         chain_name_conf_arg = 'testnet'
309         chain_name_conf_section = 'test'
310+    elif chain == "main":

MarcoFalke commented at 10:59 am on February 5, 2020:

Ok, this is my fault for putting the wrong comment here, but I think the chain name is not actually the chain name, but the name of the subfolder in the datadir. E.g. testnet3, regtest, or `` (the empty string) for the main chain. This if-elif logic should switch the subfolder name to the chain name.

Also, to simplify the logic, I don’t think we need the distinction between conf_arg and conf_section. You may just use a single chain_name symbol and then put that into -chain=... and the section below.

ryanofsky commented at 8:01 pm on February 10, 2020:

Thanks, added second commit with suggested cleanups

ryanofsky commented at 10:36 pm on September 2, 2020:

re: #17556 (review)

Thanks, added second commit with suggested cleanups

Had to drop second commit because it was causing feature_backwards_compatibility.py failures. (Previous versions require regtest=1 not chain=regtest)

MarcoFalke approved

ryanofsky referenced this in commit b339f83907 on Feb 10, 2020

ryanofsky force-pushed on Feb 10, 2020

ryanofsky commented at 8:02 pm on February 10, 2020: member

Updated 228fb7ef80cdf806353bc7d4fa2859a7d4dd5d87 -> b339f839072617df76578e9e4196d62270b57bb8 (pr/wdpy.1 -> pr/wdpy.2, compare) using "" chain subdirectory instead of "main" and implementing cleanup suggestions

in test/functional/test_framework/util.py:353 in b339f83907 outdated

318-        f.write("{}=1\n".format(chain_name_conf_arg))
319-        f.write("[{}]\n".format(chain_name_conf_section))
320+        chain_name = chain
321+    with open(config_path, 'w', encoding='utf8') as f:
322+        f.write("chain={}\n".format(chain_name))
323+        f.write("[{}]\n".format(chain_name))

MarcoFalke commented at 8:34 pm on February 10, 2020:

feature_config_args needs to be updated for this change as well

ryanofsky commented at 8:39 pm on February 10, 2020:

feature_config_args needs to be updated for this change as well

Why/how?

MarcoFalke commented at 9:03 pm on February 10, 2020:

It parses the debug.log to check for the chain section or something

MarcoFalke commented at 9:31 pm on February 10, 2020:

(See the test failure)

ryanofsky commented at 10:23 pm on February 10, 2020:

Oh, this seems to be a silent merge conflict with #16115. Doesn’t happen on my local branch since I never rebased it. Will debug

ryanofsky referenced this in commit b5f65611d2 on Feb 11, 2020

ryanofsky referenced this in commit affbdfa19d on Feb 11, 2020

ryanofsky force-pushed on Feb 11, 2020

ryanofsky referenced this in commit 0df0e23d0e on Feb 11, 2020

ryanofsky commented at 9:24 pm on February 11, 2020: member

Rebased b339f839072617df76578e9e4196d62270b57bb8 -> affbdfa19d742a00cb56e24de346c9a181679313 (pr/wdpy.2 -> pr/wdpy.3, compare) fixing silent merge conflict with #16115. Thanks Marco for looking into the test failure Updated affbdfa19d742a00cb56e24de346c9a181679313 -> a3c0a70a301de1426b2928d8503eb6eee2e82b28 (pr/wdpy.3 -> pr/wdpy.4, compare) just editing commit message Rebased a3c0a70a301de1426b2928d8503eb6eee2e82b28 -> 8fa28f3bb9574407abc3e6a83a013ccd2af3e330 (pr/wdpy.4 -> pr/wdpy.5, compare) to get past cirrus freebsd failures https://cirrus-ci.com/task/6666211096264704 Updated 8fa28f3bb9574407abc3e6a83a013ccd2af3e330 -> f147b00e0a03c642103e7678a2df4eaa533cfecb (pr/wdpy.5 -> pr/wdpy.6, compare) dropping second commit to avoid feature_backwards_compatibility.py failures https://travis-ci.org/github/bitcoin/bitcoin/jobs/723528480#L5088

ryanofsky referenced this in commit a3c0a70a30 on Aug 11, 2020

ryanofsky force-pushed on Aug 11, 2020

test: Change feature_config_args.py not to rely on strange regtest=0 behavior

Update test to simply generate a normal mainnet configuration file instead of
using a crazy setup where a regtest=1 config file using an includeconf in the
[regtest] section includes another config file that specifies regtest=0,
retroactively switching the network to mainnet.

This setup was fragile and only worked because the triggered InitError happened
early enough that none of the ignored [regtest] options mattered (only
affecting log output).

ff44cae279

ryanofsky closed this on Sep 2, 2020

ryanofsky reopened this on Sep 2, 2020

ryanofsky referenced this in commit ccb3106be8 on Sep 2, 2020

ryanofsky referenced this in commit 8fa28f3bb9 on Sep 2, 2020

ryanofsky force-pushed on Sep 2, 2020

in test/functional/test_framework/util.py:343 in f147b00e0a outdated

339+    os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
340+    os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
341+    return datadir
342+
343+
344+def write_config(config_path, n, chain, extra_config=""):

MarcoFalke commented at 7:54 am on September 6, 2020:

0def write_config(config_path, *, n, chain, extra_config=""):

nit: would be good to force named args to avoid mixing up n and chain. E.g. setting chain=0 will work, but is a confusing way to say mainnet.

ryanofsky commented at 8:38 pm on January 19, 2021:

re: #17556 (review)

nit: would be good to force named args to avoid mixing up n and chain. E.g. setting chain=0 will work, but is a confusing way to say mainnet.

Thanks, added and updated callers.

MarcoFalke approved

MarcoFalke commented at 7:55 am on September 6, 2020: member

ACK

let me know whether you want to address the nit or not

ryanofsky force-pushed on Jan 19, 2021

ryanofsky commented at 8:43 pm on January 19, 2021: member

Updated f147b00e0a03c642103e7678a2df4eaa533cfecb -> ff44cae279bef7997f76db18deb1e41b39f05cb6 (pr/wdpy.6 -> pr/wdpy.7, compare) just requiring write_config keyword arguments

MarcoFalke commented at 3:51 pm on January 21, 2021: member

ACK ff44cae279bef7997f76db18deb1e41b39f05cb6

MarcoFalke merged this on Jan 21, 2021

MarcoFalke closed this on Jan 21, 2021

sidhujag referenced this in commit 6d25d48353 on Jan 21, 2021

DrahtBot locked this on Aug 18, 2022

test: Change feature_config_args.py not to rely on strange regtest=0 behavior #17556

Conflicts