From d7befd4189cbfafb8984468fc13318fa087dc812 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 16 Feb 2021 16:28:19 -0500 Subject: [PATCH] Fix tests that ensure log levels are not set for local/remote roles. These tests were broken because they were being gated by resetLogLevel. So they were not setting the log levels, but not because of the role setting. Because resetLogLevel was being checked last coverage testing indicated that the tests were working. Fix the resetLogLevel parameter in the tests and move resetLogLevel to be tested first so coverage reporting works as expected. This isn't perfect but it is an improvement. --- src/config/parse.c | 2 +- test/src/module/config/parseTest.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/config/parse.c b/src/config/parse.c index 92a04db6a..848afacbf 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -1109,7 +1109,7 @@ configParse(const Storage *storage, unsigned int argListSize, const char *argLis THROW(ParamInvalidError, "command does not allow parameters"); // Enable logging (except for local and remote commands) so config file warnings will be output - if (config->commandRole != cfgCmdRoleLocal && config->commandRole != cfgCmdRoleRemote && resetLogLevel) + if (resetLogLevel && config->commandRole != cfgCmdRoleLocal && config->commandRole != cfgCmdRoleRemote) logInit(logLevelWarn, logLevelWarn, logLevelOff, false, 0, 1, false); // Only continue if command options need to be validated, i.e. a real command is running or we are getting help for a diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index a7d2dc91a..490af78f1 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -776,8 +776,9 @@ testRun(void) configParse(storageTest, strLstSize(argList), strLstPtr(argList), false), OptionInvalidValueError, "'/path1/path2//' cannot contain // for 'pg1-path' option"); - // Local and remove commands should not modify log levels during parsing // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("local/remote roles should not modify log levels"); + argList = strLstNew(); strLstAdd(argList, strNew("pgbackrest")); hrnCfgArgRawZ(argList, cfgOptPg, "2"); @@ -791,7 +792,7 @@ testRun(void) logLevelStdOut = logLevelError; logLevelStdErr = logLevelError; - TEST_RESULT_VOID(configParse(storageTest, strLstSize(argList), strLstPtr(argList), false), "load local config"); + TEST_RESULT_VOID(configParse(storageTest, strLstSize(argList), strLstPtr(argList), true), "load local config"); TEST_RESULT_STR_Z(cfgOptionStr(cfgOptPgPath), "/path/to/2", "default pg-path"); TEST_RESULT_INT(cfgCommandRole(), cfgCmdRoleLocal, " command role is local"); TEST_RESULT_BOOL(cfgLockRequired(), false, " backup:local command does not require lock"); @@ -810,7 +811,7 @@ testRun(void) logLevelStdOut = logLevelError; logLevelStdErr = logLevelError; - TEST_RESULT_VOID(configParse(storageTest, strLstSize(argList), strLstPtr(argList), false), "load remote config"); + TEST_RESULT_VOID(configParse(storageTest, strLstSize(argList), strLstPtr(argList), true), "load remote config"); TEST_RESULT_INT(cfgCommandRole(), cfgCmdRoleRemote, " command role is remote"); TEST_RESULT_STR_Z(cfgCommandRoleStr(cfgCmdRoleRemote), "remote", " remote role name"); TEST_RESULT_INT(logLevelStdOut, logLevelError, "console logging is error");