From 151f3e9cf1bbcf30a4fc7749682e66b4a73ddfc2 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 11 Apr 2019 16:54:47 -0400 Subject: [PATCH] Add settings merge test to prevent regresssions --- src/test/util_tests.cpp | 229 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 6795308e8..0f1834240 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -158,6 +158,9 @@ struct TestArgsManager : public ArgsManager AddArg(args[i], "", false, OptionsCategory::OPTIONS); } } + using ArgsManager::ReadConfigStream; + using ArgsManager::cs_args; + using ArgsManager::m_network; }; BOOST_AUTO_TEST_CASE(util_ParseParameters) @@ -575,6 +578,232 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); } +// 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. +// +// The test covers: +// +// - Combining different setting actions. Possible actions are: configuring a +// setting, negating a setting (adding "-no" prefix), and configuring/negating +// settings in a network section (adding "main." or "test." prefixes). +// +// - Combining settings from command line arguments and a config file. +// +// - Combining SoftSet and ForceSet calls. +// +// - Testing "main" and "test" network values to make sure settings from network +// sections are applied and to check for mainnet-specific behaviors like +// inheriting settings from the default section. +// +// - Testing network-specific settings like "-wallet", that may be ignored +// outside a network section, and non-network specific settings like "-server" +// that aren't sensitive to the network. +// +struct SettingsMergeTestingSetup : 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; + + enum Action { SET = 0, NEGATE, SECTION_SET, SECTION_NEGATE, END }; + 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) { + for (bool soft_set : {false, true}) { + for (bool force_set : {false, true}) { + for (const std::string& section : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET}) { + for (const std::string& network : {CBaseChainParams::MAIN, CBaseChainParams::TESTNET}) { + for (bool net_specific : {false, true}) { + fn(arg_actions, conf_actions, soft_set, force_set, section, network, net_specific); + } + } + } + } + } + }); + }); + } + + //! 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, + const std::string& name, + const std::string& value_prefix) + { + std::vector values; + int suffix = 0; + for (Action action : actions) { + if (action == END) break; + std::string prefix; + if (action == SECTION_SET || action == SECTION_NEGATE) prefix = section + "."; + if (action == SET || action == SECTION_SET) { + for (int i = 0; i < 2; ++i) { + values.push_back(prefix + name + "=" + value_prefix + std::to_string(++suffix)); + } + } + if (action == NEGATE || action == SECTION_NEGATE) { + values.push_back(prefix + "no" + name + "=1"); + } + } + return values; + } +}; + +// 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). +BOOST_FIXTURE_TEST_CASE(util_SettingsMerge, SettingsMergeTestingSetup) +{ + CHash256 out_sha; + FILE* out_file = nullptr; + if (const char* out_path = getenv("SETTINGS_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, bool soft_set, bool force_set, + const std::string& section, const std::string& network, bool net_specific) { + TestArgsManager parser; + LOCK(parser.cs_args); + + std::string desc = "net="; + desc += network; + parser.m_network = network; + + const std::string& name = net_specific ? "server" : "wallet"; + const std::string key = "-" + name; + parser.AddArg(key, name, false, OptionsCategory::OPTIONS); + if (net_specific) parser.SetNetworkOnlyArg(key); + + auto args = GetValues(arg_actions, section, name, "a"); + std::vector argv = {"ignored"}; + for (auto& arg : args) { + arg.insert(0, "-"); + desc += " "; + desc += arg; + argv.push_back(arg.c_str()); + } + std::string error; + BOOST_CHECK(parser.ParseParameters(argv.size(), argv.data(), error)); + BOOST_CHECK_EQUAL(error, ""); + + std::string conf; + for (auto& conf_val : GetValues(conf_actions, section, name, "c")) { + desc += " "; + desc += conf_val; + conf += conf_val; + conf += "\n"; + } + std::istringstream conf_stream(conf); + BOOST_CHECK(parser.ReadConfigStream(conf_stream, "filepath", error)); + BOOST_CHECK_EQUAL(error, ""); + + if (soft_set) { + desc += " soft"; + parser.SoftSetArg(key, "soft1"); + parser.SoftSetArg(key, "soft2"); + } + + if (force_set) { + desc += " force"; + parser.ForceSetArg(key, "force1"); + parser.ForceSetArg(key, "force2"); + } + + desc += " || "; + + if (!parser.IsArgSet(key)) { + desc += "unset"; + BOOST_CHECK(!parser.IsArgNegated(key)); + BOOST_CHECK_EQUAL(parser.GetArg(key, "default"), "default"); + BOOST_CHECK(parser.GetArgs(key).empty()); + } else if (parser.IsArgNegated(key)) { + desc += "negated"; + BOOST_CHECK_EQUAL(parser.GetArg(key, "default"), "0"); + BOOST_CHECK(parser.GetArgs(key).empty()); + } else { + desc += parser.GetArg(key, "default"); + desc += " |"; + for (const auto& arg : parser.GetArgs(key)) { + desc += " "; + desc += arg; + } + } + + std::set ignored = parser.GetUnsuitableSectionOnlyArgs(); + if (!ignored.empty()) { + desc += " | ignored"; + for (const auto& arg : ignored) { + desc += " "; + desc += arg; + } + } + + 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: + // + // SETTINGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_SettingsMerge + // + // And verify diff against previous results to make sure the changes are expected. + // + // Results file is formatted like: + // + // || | | + BOOST_CHECK_EQUAL(out_sha_hex, "80964e17fbd3c5569d3c824d032e28e2d319ef57494735b0e76eb7aad9957f2c"); +} + BOOST_AUTO_TEST_CASE(util_FormatMoney) { BOOST_CHECK_EQUAL(FormatMoney(0), "0.00");