Test ReadConfigFile #12812

pull Empact wants to merge 1 commits into bitcoin:master from Empact:test-read-config-file changing 3 files +56 −1

Empact commented at 10:15 AM on March 28, 2018: member

I stared writing this for #12765, pulling it out here for easier review.
fanquake added the label Tests on Mar 28, 2018
Empact force-pushed on Mar 28, 2018

jnewbery commented at 3:37 PM on March 29, 2018: member

unit tests failing in travis:

test/getarg_tests.cpp(39): error in "configfile": check gArgs.GetBoolArg("-foo", false) failed
test/getarg_tests.cpp(43): error in "configfile": check !gArgs.GetBoolArg("-bar", true) failed
test/getarg_tests.cpp(46): error in "configfile": check gArgs.GetArg("-baz", "") == "11" failed [ != 11]
test/getarg_tests.cpp(47): error in "configfile": check gArgs.GetArg("-baz", "eleven") == "11" failed [eleven != 11]
test/getarg_tests.cpp(49): error in "configfile": check gArgs.GetArg("-qux", "") == "eleven" failed [ != eleven]
test/getarg_tests.cpp(53): error in "configfile": check gArgs.GetArg("-baz", 0) == 11 failed [0 != 11]
test/getarg_tests.cpp(54): error in "configfile": check gArgs.GetArg("-qux", 11) == 0 failed [11 != 0]
test/getarg_tests.cpp(58): error in "configfile": check !gArgs.GetBoolArg("-quux", true) failed
test/getarg_tests.cpp(70): error in "configfile": exception std::runtime_error is expected

Empact force-pushed on Mar 29, 2018
Empact force-pushed on Mar 29, 2018
Empact force-pushed on Mar 29, 2018
Empact commented at 12:58 AM on March 30, 2018: member

@jnewbery Seems like it was a datadir caching issue, clearing the cache worked.

Ironically, ParseParameters('-datadir') does not clear the datadir cache, probably because ParseParameters is called once and at the beginning of the run.
Test ReadConfigFile 3ea6a91f16
Empact commented at 1:00 AM on March 30, 2018: member

Rerunning after rebase for good measure.
Empact force-pushed on Mar 30, 2018
conscott commented at 2:19 PM on March 30, 2018: contributor

Tested ACK 3ea6a91f16c076e2774700c0364dfc1e7aca5b2a
in src/test/getarg_tests.cpp:35 in 3ea6a91f16
```
  29 | @@ -30,6 +30,50 @@ static void ResetArgs(const std::string& strArg)
  30 |      gArgs.ParseParameters(vecChar.size(), vecChar.data());
  31 |  }
  32 |  
  33 | +BOOST_AUTO_TEST_CASE(configfile)
  34 | +{
  35 | +  fs::path expectedConfigPath(fs::system_complete("test/data/bitcoin.conf"));
```
laanwj commented at 11:25 AM on April 2, 2018:

The practice for the unit tests has been to embed all test files in the source code - by including the generated test files (in this case bitcoin.conf.raw.h, I think). The reason is that it should be possible to copy test_bitcoin to a new platform (for example by installing the distributed binaries) and run it without copying the entire source tree. If you really need a file (for testing) instead of data in memory, create the file in a temporary directory.
jnewbery commented at 1:52 PM on April 2, 2018: member

Concept ACK, but agree with @laanwj about the test file. Take a look at https://github.com/bitcoin/bitcoin/blob/821980c00feef143a6ecf6af65926e18cfd30396/src/Makefile.test.include#L161 to see how the .json.h files are generated.
ryanofsky commented at 3:37 PM on April 3, 2018: member

Take a look at bitcoin/src/Makefile.test.include

Feel free to fix this whatever way is simplest, but I would probably chose to avoid dealing with make, and instead just put the config in a multiline raw R"(...)" string that maybe gets written to a temporary file at runtime, if necessary.
laanwj commented at 4:50 PM on April 3, 2018: member

instead just put the config in a multiline raw R"(...)" string that maybe gets written to a temporary file at runtime, if necessary.

What, C++11 has multi-line string literals? TIL
ajtowns commented at 12:19 AM on April 5, 2018: member

I think #12878 covers most of this (without needing the separate file, due to splitting ReadConfigStream out of ReadConfigFile), and that the -datadir and file-reading testing is probably better done via the functional test suite rather than the unit tests.
Empact commented at 1:32 AM on April 5, 2018: member

Agreed, I prefer #12878 👍
Empact closed this on Apr 5, 2018
Empact deleted the branch on May 30, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12765 Store the current config file at read time, and return it from GetConfigFile if set