test: Usage of m_args vs. m_node.args/gArgs #25055

issue dongcarl openend this issue on May 2, 2022
  1. dongcarl commented at 7:06 pm on May 2, 2022: contributor

    I was curious about this while perusing through setup_common.cpp. It seems to me that:

    1. m_node.args is an alias to gArgs
    2. m_args is its own thing
    3. Extra arguments supplied to *TestingSetup ctors are parsed into m_node.args/gArgs, m_args is left alone
    4. The only usage of m_args is in the chainstate loading sequence I added and in various tests where only its GetDataDirBase() member is called

    Therefore it seems that the current uses of m_args in the chainstate loading sequence in TestingSetup::TestingSetup is incorrect, and we should likely remove m_args to avoid further confusion since it’s not serving any useful purpose.

    Would love some context here!

  2. ryanofsky commented at 7:45 pm on May 2, 2022: contributor

    I think the goal is that code being tested by unit tests should be using settings from the BasicTestingSetup::m_args, not gArgs. m_node.args is a pointer that currently points to gArgs so current tests work, but will later point to m_args, so future tests will be self-contained.. Eventually gArgs should go away entirely, or if it is kept, maybe it will just be used to pass options to the test framework itself (logging options, random seeds, etc).

    Probably a good strategy for writing tests is to use m_args where possible, use m_node.args if that’s not possible, and avoid ever referencing gArgs directly. But if this is too confusing we could eliminiate m_args like you suggest. The last place this was discussed was #21244 (review) so that might provide more context.

  3. maflcko added the label Brainstorming on May 3, 2022
  4. maflcko added the label Tests on May 3, 2022
  5. maflcko commented at 9:47 am on June 2, 2022: member
    Seems reasonable. Maybe add this as doxygen to m_args and close this issue?
  6. willcl-ark referenced this in commit 76672a967a on Oct 13, 2024
  7. willcl-ark referenced this in commit 1fe1b3ba8e on Oct 13, 2024
  8. fanquake closed this on Oct 15, 2024

  9. fanquake referenced this in commit 2ac5ba24bf on Oct 15, 2024

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: 2024-11-21 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me