From 05bfee3451d3b3f9aaeda63de75f9d48eecea237 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 24 Apr 2019 05:57:34 -0400 Subject: util_SettingsMerge test cleanup Followup to #15869. Treat "-wallet" as the network-specific argument in test instead of "-server", to make test output clearer and be more consistent with bitcoind. Update embedded hash to match changed output from this. --- src/test/util_tests.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/test') diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 0f1834240d..566fbae906 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -580,7 +580,7 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) // Test different ways settings can be merged, and verify results. This test can // be used to confirm that updates to settings code don't change behavior -// unintentially. +// unintentionally. // // The test covers: // @@ -687,7 +687,7 @@ struct SettingsMergeTestingSetup : public BasicTestingSetup { // Regression test covering different ways config settings can be merged. The // test parses and merges settings, representing the results as strings that get // compared against an expected hash. To debug, the result strings can be dumped -// to a file (see below). +// to a file (see comments below). BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) { CHash256 out_sha; @@ -706,7 +706,7 @@ BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) desc += network; parser.m_network = network; - const std::string& name = net_specific ? "server" : "wallet"; + const std::string& name = net_specific ? "wallet" : "server"; const std::string key = "-" + name; parser.AddArg(key, name, false, OptionsCategory::OPTIONS); if (net_specific) parser.SetNetworkOnlyArg(key); @@ -801,7 +801,7 @@ BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) // Results file is formatted like: // // || | | - BOOST_CHECK_EQUAL(out_sha_hex, "80964e17fbd3c5569d3c824d032e28e2d319ef57494735b0e76eb7aad9957f2c"); + BOOST_CHECK_EQUAL(out_sha_hex, "f4281d9bff4c0b398ff118386e3a40caa6bac7645e17b296d6a10b95bff632ae"); } BOOST_AUTO_TEST_CASE(util_FormatMoney) -- cgit v1.2.3 From 4b331159dfa599eced9f9d4e5780173367b43c74 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 6 May 2019 11:52:13 -0400 Subject: Add unit test NextString, ForEachNoDup functions Remove testcase generating code from util_SettingsMerge so it can be reused in new tests. The hash value expected in util_SettingsMerge changes as a result of this, but only because the testcases are generated in a different order, not because any cases are added or removed. It is possible to verify this with: SETTINGS_MERGE_TEST_OUT=new.txt test/test_bitcoin --run_test=util_tests/util_SettingsMerge git checkout HEAD~1 make test/test_bitcoin SETTINGS_MERGE_TEST_OUT=old.txt test/test_bitcoin --run_test=util_tests/util_SettingsMerge diff -u <(sort old.txt) <(sort new.txt) The new output is a little more readable, with simpler testcases sorted first. --- src/test/util.h | 32 ++++++++++++++++++++++++++++++++ src/test/util_tests.cpp | 43 ++++++++----------------------------------- 2 files changed, 40 insertions(+), 35 deletions(-) (limited to 'src/test') diff --git a/src/test/util.h b/src/test/util.h index 8ba647ec3f..f90cb0d623 100644 --- a/src/test/util.h +++ b/src/test/util.h @@ -34,5 +34,37 @@ std::string getnewaddress(CWallet& w); /** Returns the generated coin */ CTxIn generatetoaddress(const std::string& address); +/** + * Increment a string. Useful to enumerate all fixed length strings with + * characters in [min_char, max_char]. + */ +template +bool NextString(CharType (&string)[StringLength], CharType min_char, CharType max_char) +{ + for (CharType& elem : string) { + bool has_next = elem != max_char; + elem = elem < min_char || elem >= max_char ? min_char : CharType(elem + 1); + if (has_next) return true; + } + return false; +} + +/** + * Iterate over string values and call function for each string without + * successive duplicate characters. + */ +template +void ForEachNoDup(CharType (&string)[StringLength], CharType min_char, CharType max_char, Fn&& fn) { + for (bool has_next = true; has_next; has_next = NextString(string, min_char, max_char)) { + int prev = -1; + bool skip_string = false; + for (CharType c : string) { + if (c == prev) skip_string = true; + if (skip_string || c < min_char || c > max_char) break; + prev = c; + } + if (!skip_string) fn(); + } +} #endif // BITCOIN_TEST_UTIL_H diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 566fbae906..42b781db95 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -605,15 +606,17 @@ struct SettingsMergeTestingSetup : public BasicTestingSetup { //! debugging to make test results easier to understand. static constexpr int MAX_ACTIONS = 3; - enum Action { SET = 0, NEGATE, SECTION_SET, SECTION_NEGATE, END }; + enum Action { NONE, SET, NEGATE, SECTION_SET, SECTION_NEGATE }; using ActionList = Action[MAX_ACTIONS]; //! Enumerate all possible test configurations. template void ForEachMergeSetup(Fn&& fn) { - ForEachActionList([&](const ActionList& arg_actions) { - ForEachActionList([&](const ActionList& conf_actions) { + ActionList arg_actions = {}; + ForEachNoDup(arg_actions, SET, SECTION_NEGATE, [&] { + ActionList conf_actions = {}; + ForEachNoDup(conf_actions, SET, SECTION_NEGATE, [&] { for (bool soft_set : {false, true}) { for (bool force_set : {false, true}) { for (const std::string& section : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET}) { @@ -629,36 +632,6 @@ struct SettingsMergeTestingSetup : public BasicTestingSetup { }); } - //! Enumerate interesting combinations of actions. - template - void ForEachActionList(Fn&& fn) - { - ActionList actions = {SET}; - for (bool done = false; !done;) { - int prev_action = -1; - bool skip_actions = false; - for (Action action : actions) { - if ((prev_action == END && action != END) || (prev_action != END && action == prev_action)) { - // To cut down list of enumerated settings, skip enumerating - // settings with ignored actions after an END, and settings that - // repeat the same action twice in a row. - skip_actions = true; - break; - } - prev_action = action; - } - if (!skip_actions) fn(actions); - done = true; - for (Action& action : actions) { - action = Action(action < END ? action + 1 : 0); - if (action) { - done = false; - break; - } - } - } - } - //! Translate actions into a list of = setting strings. std::vector GetValues(const ActionList& actions, const std::string& section, @@ -668,7 +641,7 @@ struct SettingsMergeTestingSetup : public BasicTestingSetup { std::vector values; int suffix = 0; for (Action action : actions) { - if (action == END) break; + if (action == NONE) break; std::string prefix; if (action == SECTION_SET || action == SECTION_NEGATE) prefix = section + "."; if (action == SET || action == SECTION_SET) { @@ -801,7 +774,7 @@ BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) // Results file is formatted like: // // || | | - BOOST_CHECK_EQUAL(out_sha_hex, "f4281d9bff4c0b398ff118386e3a40caa6bac7645e17b296d6a10b95bff632ae"); + BOOST_CHECK_EQUAL(out_sha_hex, "b835eef5977d69114eb039a976201f8c7121f34fe2b7ea2b73cafb516e5c9dc8"); } BOOST_AUTO_TEST_CASE(util_FormatMoney) -- cgit v1.2.3 From f6bb11fd37f8a2c985786b688ea07699ba75780e Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 6 May 2019 17:12:42 -0400 Subject: Add test for ArgsManager::GetChainName There was some test coverage previously, but it was limited and didn't test conflicting and negated arguments. --- src/test/util_tests.cpp | 110 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 4 deletions(-) (limited to 'src/test') diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 42b781db95..51dd25ed1c 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -601,7 +601,7 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) // outside a network section, and non-network specific settings like "-server" // that aren't sensitive to the network. // -struct SettingsMergeTestingSetup : public BasicTestingSetup { +struct ArgsMergeTestingSetup : public BasicTestingSetup { //! Max number of actions to sequence together. Can decrease this when //! debugging to make test results easier to understand. static constexpr int MAX_ACTIONS = 3; @@ -661,11 +661,11 @@ struct SettingsMergeTestingSetup : public BasicTestingSetup { // test parses and merges settings, representing the results as strings that get // compared against an expected hash. To debug, the result strings can be dumped // to a file (see comments below). -BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) +BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup) { CHash256 out_sha; FILE* out_file = nullptr; - if (const char* out_path = getenv("SETTINGS_MERGE_TEST_OUT")) { + if (const char* out_path = getenv("ARGS_MERGE_TEST_OUT")) { out_file = fsbridge::fopen(out_path, "w"); if (!out_file) throw std::system_error(errno, std::generic_category(), "fopen failed"); } @@ -767,7 +767,7 @@ BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) // If check below fails, should manually dump the results with: // - // SETTINGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_SettingsMerge + // ARGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_ArgsMerge // // And verify diff against previous results to make sure the changes are expected. // @@ -777,6 +777,108 @@ BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) BOOST_CHECK_EQUAL(out_sha_hex, "b835eef5977d69114eb039a976201f8c7121f34fe2b7ea2b73cafb516e5c9dc8"); } +// Similar test as above, but for ArgsManager::GetChainName function. +struct ChainMergeTestingSetup : public BasicTestingSetup { + static constexpr int MAX_ACTIONS = 2; + + enum Action { NONE, ENABLE_TEST, DISABLE_TEST, NEGATE_TEST, ENABLE_REG, DISABLE_REG, NEGATE_REG }; + using ActionList = Action[MAX_ACTIONS]; + + //! Enumerate all possible test configurations. + template + void ForEachMergeSetup(Fn&& fn) + { + ActionList arg_actions = {}; + ForEachNoDup(arg_actions, ENABLE_TEST, NEGATE_REG, [&] { + ActionList conf_actions = {}; + ForEachNoDup(conf_actions, ENABLE_TEST, NEGATE_REG, [&] { fn(arg_actions, conf_actions); }); + }); + } +}; + +BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup) +{ + CHash256 out_sha; + FILE* out_file = nullptr; + if (const char* out_path = getenv("CHAIN_MERGE_TEST_OUT")) { + out_file = fsbridge::fopen(out_path, "w"); + if (!out_file) throw std::system_error(errno, std::generic_category(), "fopen failed"); + } + + ForEachMergeSetup([&](const ActionList& arg_actions, const ActionList& conf_actions) { + TestArgsManager parser; + LOCK(parser.cs_args); + parser.AddArg("-regtest", "regtest", false, OptionsCategory::OPTIONS); + parser.AddArg("-testnet", "testnet", false, OptionsCategory::OPTIONS); + + auto arg = [](Action action) { return action == ENABLE_TEST ? "-testnet=1" : + action == DISABLE_TEST ? "-testnet=0" : + action == NEGATE_TEST ? "-notestnet=1" : + action == ENABLE_REG ? "-regtest=1" : + action == DISABLE_REG ? "-regtest=0" : + action == NEGATE_REG ? "-noregtest=1" : nullptr; }; + + std::string desc; + std::vector argv = {"ignored"}; + for (Action action : arg_actions) { + const char* argstr = arg(action); + if (!argstr) break; + argv.push_back(argstr); + desc += " "; + desc += argv.back(); + } + std::string error; + BOOST_CHECK(parser.ParseParameters(argv.size(), argv.data(), error)); + BOOST_CHECK_EQUAL(error, ""); + + std::string conf; + for (Action action : conf_actions) { + const char* argstr = arg(action); + if (!argstr) break; + desc += " "; + desc += argstr + 1; + conf += argstr + 1; + } + std::istringstream conf_stream(conf); + BOOST_CHECK(parser.ReadConfigStream(conf_stream, "filepath", error)); + BOOST_CHECK_EQUAL(error, ""); + + desc += " || "; + try { + desc += parser.GetChainName(); + } catch (const std::runtime_error& e) { + desc += "error: "; + desc += e.what(); + } + desc += "\n"; + + out_sha.Write((const unsigned char*)desc.data(), desc.size()); + if (out_file) { + BOOST_REQUIRE(fwrite(desc.data(), 1, desc.size(), out_file) == desc.size()); + } + }); + + if (out_file) { + if (fclose(out_file)) throw std::system_error(errno, std::generic_category(), "fclose failed"); + out_file = nullptr; + } + + unsigned char out_sha_bytes[CSHA256::OUTPUT_SIZE]; + out_sha.Finalize(out_sha_bytes); + std::string out_sha_hex = HexStr(std::begin(out_sha_bytes), std::end(out_sha_bytes)); + + // If check below fails, should manually dump the results with: + // + // CHAIN_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_ChainMerge + // + // And verify diff against previous results to make sure the changes are expected. + // + // Results file is formatted like: + // + // || + BOOST_CHECK_EQUAL(out_sha_hex, "b284f4b4a15dd6bf8c06213a69a004b1960388e1d9917173927db52ac220927f"); +} + BOOST_AUTO_TEST_CASE(util_FormatMoney) { BOOST_CHECK_EQUAL(FormatMoney(0), "0.00"); -- cgit v1.2.3