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

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/wdpy changing 2 files +18 −9
  1. ryanofsky commented at 10:41 PM on November 21, 2019: contributor

    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

  2. fanquake added the label Tests on Nov 21, 2019
  3. ryanofsky cross-referenced this on Nov 21, 2019 from issue util: Forbid ambiguous multiple assignments in config file by ryanofsky
  4. DrahtBot commented at 12:42 AM on November 22, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  5. laanwj commented at 9:34 PM on November 22, 2019: member

    Concept ACK

  6. 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
  7. ryanofsky cross-referenced this on Nov 24, 2019 from issue refactor: Remove settings merge reverse precedence code by ryanofsky
  8. ryanofsky cross-referenced this on Nov 24, 2019 from issue Simpler settings interpretation by ryanofsky
  10. 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)

  11. MarcoFalke approved
  12. MarcoFalke cross-referenced this on Feb 5, 2020 from issue test: replace 'regtest' leftovers by self.chain by theStack
  13. ryanofsky referenced this in commit b339f83907 on Feb 10, 2020
  14. ryanofsky force-pushed on Feb 10, 2020
  15. ryanofsky commented at 8:02 PM on February 10, 2020: contributor

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

  16. 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

  17. ryanofsky referenced this in commit b5f65611d2 on Feb 11, 2020
  18. ryanofsky referenced this in commit affbdfa19d on Feb 11, 2020
  19. ryanofsky force-pushed on Feb 11, 2020
  20. ryanofsky referenced this in commit 0df0e23d0e on Feb 11, 2020
  21. ryanofsky commented at 9:24 PM on February 11, 2020: contributor

    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

  22. DrahtBot cross-referenced this on Feb 11, 2020 from issue Testschains: Many regtests with different genesis and default datadir by jtimon
  23. DrahtBot cross-referenced this on Feb 11, 2020 from issue Tests: Chainparams: Make regtest almost fully customizable by jtimon
  24. DrahtBot cross-referenced this on May 12, 2020 from issue [wip] util: Improve evaluation of includeconf lines in network sections of the config file by AkioNak
  25. ryanofsky referenced this in commit a3c0a70a30 on Aug 11, 2020
  26. ryanofsky force-pushed on Aug 11, 2020
  27. 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
  28. ryanofsky closed this on Sep 2, 2020
  29. ryanofsky reopened this on Sep 2, 2020
  30. ryanofsky referenced this in commit ccb3106be8 on Sep 2, 2020
  31. ryanofsky referenced this in commit 8fa28f3bb9 on Sep 2, 2020
  32. ryanofsky force-pushed on Sep 2, 2020
  33. ryanofsky force-pushed on Sep 2, 2020
  34. 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: 
    def 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.

  35. MarcoFalke approved
  36. MarcoFalke commented at 7:55 AM on September 6, 2020: member

    ACK

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

  37. MarcoFalke cross-referenced this on Oct 17, 2020 from issue qa: BitcoinTestFramework fails with extra_args=-nolisten by hebasto
  38. ryanofsky force-pushed on Jan 19, 2021
  39. ryanofsky commented at 8:43 PM on January 19, 2021: contributor

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

  40. MarcoFalke commented at 3:51 PM on January 21, 2021: member

    ACK ff44cae279bef7997f76db18deb1e41b39f05cb6

  41. MarcoFalke merged this on Jan 21, 2021
  42. MarcoFalke closed this on Jan 21, 2021
  43. sidhujag referenced this in commit 6d25d48353 on Jan 21, 2021
  44. bitcoin locked this on Aug 18, 2022
Contributors
ryanofskyDrahtBotlaanwjinstagibbsMarcoFalke
Labels
Tests
Linked (view graph)
#16115 On bitcoind startup, write config args to debug.log#17493 util: Forbid ambiguous multiple assignments in config file#17508 Simpler settings interpretation#17581 refactor: Remove settings merge reverse precedence code#18069 test: replace 'regtest' leftovers by self.chain#20177 qa: BitcoinTestFramework fails with extra_args=-nolisten
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC