diff --git a/doc/RELEASE.md b/doc/RELEASE.md index 83a97983d..fc34431a0 100644 --- a/doc/RELEASE.md +++ b/doc/RELEASE.md @@ -7,13 +7,15 @@ These instructions are temporary until a fully automated report is implemented. - In `test/src/lcov.conf` remove: ``` # Specify the regular expression of lines to exclude - lcov_excl_line=\{\+*uncovered|\{\+*uncoverable + lcov_excl_line=lcov_excl_line=\{\+{0,1}uncovered[^_]|\{\+{0,1}uncoverable[^_] # Coverage rate limits genhtml_hi_limit = 100 genhtml_med_limit = 90 ``` +And change `uncover(ed|able)_branch` to `uncoverable_branch`. + - In `test/lib/pgBackRestTest/Common/JobTest.pm` modify: ``` if (!$bTest || $iTotalLines != $iCoveredLines || $iTotalBranches != $iCoveredBranches) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 4720d0b8e..46b7bba70 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -159,6 +159,10 @@

Use THROW_ON_SYS_ERROR*() to improve code coverage.

+ +

Improve macros and coverage rules that were hiding missing coverage.

+
+

Improve efficiency of FUNCTION_LOG*() macros.

diff --git a/src/command/help/help.c b/src/command/help/help.c index 6b1f42297..7021e10ac 100644 --- a/src/command/help/help.c +++ b/src/command/help/help.c @@ -361,14 +361,14 @@ helpRender(void) { strCat(result, "\ndeprecated name"); - if (cfgDefOptionHelpNameAltValueTotal(optionDefId) > 1) // {uncovered - no option has more than one alt name} + if (cfgDefOptionHelpNameAltValueTotal(optionDefId) > 1) // {uncovered_branch - no option with more than one} strCat(result, "s"); // {+uncovered} strCat(result, ": "); for (unsigned int nameAltIdx = 0; nameAltIdx < cfgDefOptionHelpNameAltValueTotal(optionDefId); nameAltIdx++) { - if (nameAltIdx != 0) // {uncovered - no option has more than one alt name} + if (nameAltIdx != 0) // {uncovered_branch - no option with more than one} strCat(result, ", "); // {+uncovered} strCat(result, cfgDefOptionHelpNameAltValue(optionDefId, nameAltIdx)); diff --git a/src/common/debug.h b/src/common/debug.h index 2f5bccab5..d1f649706 100644 --- a/src/common/debug.h +++ b/src/common/debug.h @@ -224,9 +224,9 @@ Macros to return function results (or void) { \ typePre FUNCTION_LOG_##typeMacroPrefix##_TYPE typePost FUNCTION_LOG_RETURN_result = result; \ \ - STACK_TRACE_POP(); \ + STACK_TRACE_POP(false); \ \ - if (logWill(FUNCTION_LOG_LEVEL())) \ + IF_LOG_WILL(FUNCTION_LOG_LEVEL()) \ { \ char buffer[STACK_TRACE_PARAM_MAX]; \ \ @@ -259,7 +259,7 @@ Macros to return function results (or void) #define FUNCTION_LOG_RETURN_VOID() \ do \ { \ - STACK_TRACE_POP(); \ + STACK_TRACE_POP(false); \ \ LOG_WILL(FUNCTION_LOG_LEVEL(), 0, "=> void"); \ } \ @@ -299,20 +299,13 @@ test macros are compiled out. #define FUNCTION_TEST_RETURN(result) \ do \ { \ - if (stackTraceTest()) \ - STACK_TRACE_POP(); \ - \ + STACK_TRACE_POP(true); \ return result; \ } \ while(0); #define FUNCTION_TEST_RETURN_VOID() \ - do \ - { \ - if (stackTraceTest()) \ - STACK_TRACE_POP(); \ - } \ - while(0); + STACK_TRACE_POP(true); #else #define FUNCTION_TEST_BEGIN() #define FUNCTION_TEST_PARAM(typeMacroPrefix, param) diff --git a/src/common/error.c b/src/common/error.c index ba3992598..b87fc893d 100644 --- a/src/common/error.c +++ b/src/common/error.c @@ -425,8 +425,19 @@ Throw a system error ***********************************************************************************************************************************/ void errorInternalThrowSys( - int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) +#ifdef DEBUG_COVERAGE + bool error, +#else + int errNo, +#endif + const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) { +#ifdef DEBUG_COVERAGE + if (error) + { + int errNo = errno; +#endif + // Format message with system message appended if (errNo == 0) { @@ -437,12 +448,27 @@ errorInternalThrowSys( snprintf(messageBufferTemp, ERROR_MESSAGE_BUFFER_SIZE - 1, "%s: [%d] %s", message, errNo, strerror(errNo)); errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + +#ifdef DEBUG_COVERAGE + } +#endif } void errorInternalThrowSysFmt( - int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) +#ifdef DEBUG_COVERAGE + bool error, +#else + int errNo, +#endif + const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) { +#ifdef DEBUG_COVERAGE + if (error) + { + int errNo = errno; +#endif + // Format message va_list argument; va_start(argument, format); @@ -454,4 +480,8 @@ errorInternalThrowSysFmt( snprintf(messageBufferTemp + messageSize, ERROR_MESSAGE_BUFFER_SIZE - 1 - messageSize, ": [%d] %s", errNo, strerror(errNo)); errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + +#ifdef DEBUG_COVERAGE + } +#endif } diff --git a/src/common/error.h b/src/common/error.h index 3365e7e2c..162294c4e 100644 --- a/src/common/error.h +++ b/src/common/error.h @@ -141,42 +141,70 @@ The seldom used "THROWP" variants allow an error to be thrown with a pointer to /*********************************************************************************************************************************** Throw an error when a system call fails ***********************************************************************************************************************************/ -#define THROW_SYS_ERROR(errorType, message) \ - errorInternalThrowSys(errno, &errorType, __FILE__, __func__, __LINE__, message) -#define THROW_SYS_ERROR_FMT(errorType, ...) \ - errorInternalThrowSysFmt(errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) -#define THROWP_SYS_ERROR(errorType, message) \ - errorInternalThrowSys(errno, errorType, __FILE__, __func__, __LINE__, message) -#define THROWP_SYS_ERROR_FMT(errorType, ...) \ - errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) +// When coverage testing define special versions of the macros that don't contain branches. These macros are less efficient because +// they need to call errorInternalThrowSys*() before determining if there is an error or not, but they allow coverage testing for +// THROW*_ON*() calls that contain conditionals. +#ifdef DEBUG_COVERAGE + #define THROW_SYS_ERROR(errorType, message) \ + errorInternalThrowSys(true, &errorType, __FILE__, __func__, __LINE__, message) + #define THROW_SYS_ERROR_FMT(errorType, ...) \ + errorInternalThrowSysFmt(true, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROWP_SYS_ERROR(errorType, message) \ + errorInternalThrowSys(true, errorType, __FILE__, __func__, __LINE__, message) + #define THROWP_SYS_ERROR_FMT(errorType, ...) \ + errorInternalThrowSysFmt(true, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) -#define THROW_ON_SYS_ERROR(error, errorType, message) \ - do \ - { \ - if (error) \ - errorInternalThrowSys(errno, &errorType, __FILE__, __func__, __LINE__, message); \ - } while(0) + #define THROW_ON_SYS_ERROR(error, errorType, message) \ + errorInternalThrowSys(error, &errorType, __FILE__, __func__, __LINE__, message) -#define THROW_ON_SYS_ERROR_FMT(error, errorType, ...) \ - do \ - { \ - if (error) \ - errorInternalThrowSysFmt(errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ - } while(0) + #define THROW_ON_SYS_ERROR_FMT(error, errorType, ...) \ + errorInternalThrowSysFmt(error, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) -#define THROWP_ON_SYS_ERROR(error, errorType, message) \ - do \ - { \ - if (error) \ - errorInternalThrowSys(errno, errorType, __FILE__, __func__, __LINE__, message); \ - } while(0) + #define THROWP_ON_SYS_ERROR(error, errorType, message) \ + errorInternalThrowSys(error, errorType, __FILE__, __func__, __LINE__, message) -#define THROWP_ON_SYS_ERROR_FMT(error, errorType, ...) \ - do \ - { \ - if (error) \ - errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ - } while(0) + #define THROWP_ON_SYS_ERROR_FMT(error, errorType, ...) \ + errorInternalThrowSysFmt(error, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + +// Else define the normal macros which check for an error first +#else + #define THROW_SYS_ERROR(errorType, message) \ + errorInternalThrowSys(errno, &errorType, __FILE__, __func__, __LINE__, message) + #define THROW_SYS_ERROR_FMT(errorType, ...) \ + errorInternalThrowSysFmt(errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROWP_SYS_ERROR(errorType, message) \ + errorInternalThrowSys(errno, errorType, __FILE__, __func__, __LINE__, message) + #define THROWP_SYS_ERROR_FMT(errorType, ...) \ + errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + + #define THROW_ON_SYS_ERROR(error, errorType, message) \ + do \ + { \ + if (error) \ + errorInternalThrowSys(errno, &errorType, __FILE__, __func__, __LINE__, message); \ + } while(0) + + #define THROW_ON_SYS_ERROR_FMT(error, errorType, ...) \ + do \ + { \ + if (error) \ + errorInternalThrowSysFmt(errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ + } while(0) + + #define THROWP_ON_SYS_ERROR(error, errorType, message) \ + do \ + { \ + if (error) \ + errorInternalThrowSys(errno, errorType, __FILE__, __func__, __LINE__, message); \ + } while(0) + + #define THROWP_ON_SYS_ERROR_FMT(error, errorType, ...) \ + do \ + { \ + if (error) \ + errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ + } while(0) +#endif #define THROW_SYS_ERROR_CODE(errNo, errorType, message) \ errorInternalThrowSys(errNo, &errorType, __FILE__, __func__, __LINE__, message) @@ -211,12 +239,33 @@ void errorInternalThrow( void errorInternalThrowFmt( const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) __attribute__((format(printf, 5, 6))) __attribute__((__noreturn__)); + void errorInternalThrowSys( - int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) +#ifdef DEBUG_COVERAGE + bool error, +#else + int errNo, +#endif + const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) +#ifdef DEBUG_COVERAGE + ; +#else __attribute__((__noreturn__)); +#endif + void errorInternalThrowSysFmt( - int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) - __attribute__((format(printf, 6, 7))) __attribute__((__noreturn__)); +#ifdef DEBUG_COVERAGE + bool error, +#else + int errNo, +#endif + const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) + __attribute__((format(printf, 6, 7))) +#ifdef DEBUG_COVERAGE + ; +#else + __attribute__((__noreturn__)); +#endif /*********************************************************************************************************************************** Macros for function logging diff --git a/src/common/ini.c b/src/common/ini.c index cd4da3c37..c2e3e08f7 100644 --- a/src/common/ini.c +++ b/src/common/ini.c @@ -144,7 +144,7 @@ iniGetList(const Ini *this, const String *section, const String *key) // Get the value const Variant *result = iniGetInternal(this, section, key, false); - FUNCTION_TEST_RETURN(result == NULL ? false : strLstNewVarLst(varVarLst(result))); + FUNCTION_TEST_RETURN(result == NULL ? NULL : strLstNewVarLst(varVarLst(result))); } /*********************************************************************************************************************************** diff --git a/src/common/io/filter/filter.c b/src/common/io/filter/filter.c index 69755be87..bb01bec91 100644 --- a/src/common/io/filter/filter.c +++ b/src/common/io/filter/filter.c @@ -144,8 +144,12 @@ ioFilterDone(const IoFilter *this) ASSERT(this != NULL); - FUNCTION_TEST_RETURN( - this->flushing && (this->interface.done != NULL ? this->interface.done(this->driver) : !ioFilterInputSame(this))); + bool result = false; + + if (this->flushing) + result = this->interface.done != NULL ? this->interface.done(this->driver) : !ioFilterInputSame(this); + + FUNCTION_TEST_RETURN(result); } /*********************************************************************************************************************************** diff --git a/src/common/io/tls/client.c b/src/common/io/tls/client.c index c981c3355..223ba7422 100644 --- a/src/common/io/tls/client.c +++ b/src/common/io/tls/client.c @@ -253,17 +253,13 @@ tlsClientHostVerify(const String *host, X509 *certificate) if (!altNameFound) { X509_NAME *subjectName = X509_get_subject_name(certificate); + CHECK(subjectName != NULL); - if (subjectName != NULL) // {uncovered - not sure how to create cert with null common name} - { - int commonNameIndex = X509_NAME_get_index_by_NID(subjectName, NID_commonName, -1); + int commonNameIndex = X509_NAME_get_index_by_NID(subjectName, NID_commonName, -1); + CHECK(commonNameIndex >= 0); - if (commonNameIndex >= 0) // {uncovered - it seems this must be >= 0 if CN is not null} - { - result = tlsClientHostVerifyName( - host, asn1ToStr(X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subjectName, commonNameIndex)))); - } - } + result = tlsClientHostVerifyName( + host, asn1ToStr(X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subjectName, commonNameIndex)))); } } MEM_CONTEXT_TEMP_END(); diff --git a/src/common/log.h b/src/common/log.h index e6f9a8893..31338b02d 100644 --- a/src/common/log.h +++ b/src/common/log.h @@ -46,17 +46,26 @@ usage. #define LOG(logLevel, code, ...) \ LOG_PID(logLevel, 0, code, __VA_ARGS__) +// Define a macro to test logWill() that can be removed when performing coverage testing. Checking logWill() saves a function call +// for logging calls that won't be output anywhere, but since the macro then contains a branch it causes coverage problems. +#ifdef DEBUG_COVERAGE + #define IF_LOG_WILL(logLevel) +#else + #define IF_LOG_WILL(logLevel) \ + if (logWill(logLevel)) +#endif + #define LOG_WILL(logLevel, code, ...) \ do \ { \ - if (logWill(logLevel)) \ + IF_LOG_WILL(logLevel) \ LOG(logLevel, code, __VA_ARGS__); \ } while(0) #define LOG_WILL_PID(logLevel, processId, code, ...) \ do \ { \ - if (logWill(logLevel)) \ + IF_LOG_WILL(logLevel) \ LOG_PID(logLevel, processId, code, __VA_ARGS__); \ } while(0) diff --git a/src/common/regExp.c b/src/common/regExp.c index 82e480f58..36348f73a 100644 --- a/src/common/regExp.c +++ b/src/common/regExp.c @@ -41,9 +41,12 @@ regExpError(int error) FUNCTION_TEST_PARAM(INT, error); FUNCTION_TEST_END(); - char buffer[4096]; - regerror(error, NULL, buffer, sizeof(buffer)); - THROW(FormatError, buffer); + if (error != 0 && error != REG_NOMATCH) + { + char buffer[4096]; + regerror(error, NULL, buffer, sizeof(buffer)); + THROW(FormatError, buffer); + } FUNCTION_TEST_RETURN_VOID(); } @@ -102,8 +105,7 @@ regExpMatch(RegExp *this, const String *string) int result = regexec(&this->regExp, strPtr(string), 0, NULL, 0); // Check for an error - if (result != 0 && result != REG_NOMATCH) // {uncoverable - no error condition known} - regExpError(result); // {+uncoverable} + regExpError(result); FUNCTION_TEST_RETURN(result == 0); } diff --git a/src/common/stackTrace.c b/src/common/stackTrace.c index 05108f9fb..ad758942f 100644 --- a/src/common/stackTrace.c +++ b/src/common/stackTrace.c @@ -271,16 +271,19 @@ stackTracePop(void) #else void -stackTracePop(const char *fileName, const char *functionName) +stackTracePop(const char *fileName, const char *functionName, bool test) { ASSERT(stackSize > 0); - stackSize--; + if (!test || stackTraceTest()) + { + stackSize--; - StackTraceData *data = &stackTrace[stackSize]; + StackTraceData *data = &stackTrace[stackSize]; - if (strcmp(data->fileName, fileName) != 0 || strcmp(data->functionName, functionName) != 0) - THROW_FMT(AssertError, "popping %s:%s but expected %s:%s", fileName, functionName, data->fileName, data->functionName); + if (strcmp(data->fileName, fileName) != 0 || strcmp(data->functionName, functionName) != 0) + THROW_FMT(AssertError, "popping %s:%s but expected %s:%s", fileName, functionName, data->fileName, data->functionName); + } } #endif diff --git a/src/common/stackTrace.h b/src/common/stackTrace.h index 1ef130979..eab6bd952 100644 --- a/src/common/stackTrace.h +++ b/src/common/stackTrace.h @@ -21,11 +21,11 @@ Macros to access internal functions stackTracePush(__FILE__, __func__, logLevel) #ifdef NDEBUG - #define STACK_TRACE_POP() \ + #define STACK_TRACE_POP(test) \ stackTracePop(); #else - #define STACK_TRACE_POP() \ - stackTracePop(__FILE__, __func__); + #define STACK_TRACE_POP(test) \ + stackTracePop(__FILE__, __func__, test); #endif /*********************************************************************************************************************************** @@ -46,7 +46,7 @@ LogLevel stackTracePush(const char *fileName, const char *functionName, LogLevel #ifdef NDEBUG void stackTracePop(void); #else - void stackTracePop(const char *fileName, const char *functionName); + void stackTracePop(const char *fileName, const char *functionName, bool test); #endif size_t stackTraceToZ(char *buffer, size_t bufferSize, const char *fileName, const char *functionName, unsigned int fileLine); diff --git a/src/common/type/json.c b/src/common/type/json.c index 95f1dac2b..e72bb685a 100644 --- a/src/common/type/json.c +++ b/src/common/type/json.c @@ -883,8 +883,8 @@ jsonFromKv(const KeyValue *kv, unsigned int indent) strCat(indentDepth, strPtr(indentSpace)); strCat(jsonStr, strPtr(jsonFromKvInternal(kv, indentSpace, indentDepth))); - // Add terminating linefeed for pretty print if it is not already added - if (indent > 0 && !strEndsWithZ(jsonStr, "\n")) + // Add terminating linefeed for pretty print + if (indent > 0) strCat(jsonStr, "\n"); // Duplicate the string into the calling context @@ -973,8 +973,8 @@ jsonFromVar(const Variant *var, unsigned int indent) else strCat(jsonStr, strPtr(jsonFromKvInternal(varKv(var), indentSpace, indentDepth))); - // Add terminating linefeed for pretty print if it is not already added - if (indent > 0 && !strEndsWithZ(jsonStr, "\n")) + // Add terminating linefeed for pretty print + if (indent > 0) strCat(jsonStr, "\n"); // Duplicate the string into the calling context diff --git a/src/common/type/variant.h b/src/common/type/variant.h index 534d044ad..aa36b0f87 100644 --- a/src/common/type/variant.h +++ b/src/common/type/variant.h @@ -173,7 +173,7 @@ By convention all variant constant identifiers are appended with _VAR. ***********************************************************************************************************************************/ // Create a Bool Variant constant inline from a bool #define VARBOOL(dataParam) \ - (dataParam ? BOOL_TRUE_VAR : BOOL_FALSE_VAR) + ((const Variant *)&(const VariantBoolConst){.type = varTypeBool, .data = dataParam}) // Create a Double Variant constant inline from a double #define VARDBL(dataParam) \ diff --git a/src/config/load.c b/src/config/load.c index f3b1102c9..4619bbc72 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -192,8 +192,10 @@ cfgLoadUpdateOption(void) cfgOptionName(cfgOptRepoRetentionDiff + optionIdx)); } } - else if (strEqZ(archiveRetentionType, CFGOPTVAL_TMP_REPO_RETENTION_ARCHIVE_TYPE_INCR)) + else { + CHECK(strEqZ(archiveRetentionType, CFGOPTVAL_TMP_REPO_RETENTION_ARCHIVE_TYPE_INCR)); + LOG_WARN("%s option '%s' is not set", strPtr(msgArchiveOff), cfgOptionName(cfgOptRepoRetentionArchive + optionIdx)); } diff --git a/src/config/parse.c b/src/config/parse.c index c320fdd72..52a3512f6 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -335,7 +335,7 @@ cfgFileLoad( // NOTE: Pas storageLocal(), strNewFmt("%s/%s", strPtr(configIncludePath), strPtr(strLstGet(list, listIdx))), .ignoreMissing = true)); - if (fileBuffer != NULL) // {uncovered - NULL can only occur if file is missing after file list is retrieved} + if (fileBuffer != NULL) // {uncovered_branch - NULL can only occur if file is missing after file list is retrieved} { // Convert the contents of the file buffer to a string object String *configPart = strNewBuf(fileBuffer); diff --git a/src/perl/exec.c b/src/perl/exec.c index f8ca645dc..114e7052f 100644 --- a/src/perl/exec.c +++ b/src/perl/exec.c @@ -83,12 +83,12 @@ XS_EUPXS(embeddedModuleGet) // Ensure all parameters were passed dVAR; dXSARGS; - if (items != 1) // {uncovered - no invalid calls} + if (items != 1) // {uncovered_branch - no invalid calls} croak_xs_usage(cv, "moduleName"); // {+uncovered} // Get module name - const char *moduleName = (const char *)SvPV_nolen(ST(0)); - dXSTARG; // {uncovered - internal Perl macro branch} + const char *moduleName = (const char *)SvPV_nolen(ST(0)); // {uncoverable_branch - Perl macro} + dXSTARG; // {uncoverable_branch - Perl macro} // Find module const char *result = NULL; @@ -104,13 +104,13 @@ XS_EUPXS(embeddedModuleGet) } // Error if the module was not found - if (result == NULL) // {uncovered - no invalid modules in embedded Perl} + if (result == NULL) // {uncovered_branch - no invalid modules in embedded Perl} croak("unable to load embedded module '%s'", moduleName); // {+uncovered} // Return module data sv_setpv(TARG, result); XSprePUSH; - PUSHTARG; // {uncovered - internal Perl macro branch} + PUSHTARG; // {uncoverable_branch - Perl macro} XSRETURN(1); } @@ -222,11 +222,11 @@ perlExec(void) perlEval(perlMain()); // Return result code - int code = (int)SvIV(get_sv("iResult", 0)); - bool errorC = (int)SvIV(get_sv("bErrorC", 0)); - char *message = SvPV_nolen(get_sv("strMessage", 0)); // {uncovered - internal Perl macro branch} + int code = (int)SvIV(get_sv("iResult", 0)); // {uncoverable_branch - Perl macro} + bool errorC = (int)SvIV(get_sv("bErrorC", 0)); // {uncoverable_branch - Perl macro} + char *message = SvPV_nolen(get_sv("strMessage", 0)); // {uncoverable_branch - Perl macro} - if (code >= errorTypeCode(&AssertError)) // {uncovered - success tested in integration} + if (code >= errorTypeCode(&AssertError)) // {uncovered_branch - tested in integration} { if (errorC) // {+uncovered} RETHROW(); // {+uncovered} @@ -234,7 +234,7 @@ perlExec(void) THROW_CODE(code, strlen(message) == 0 ? PERL_EMBED_ERROR : message); // {+uncovered} } - FUNCTION_LOG_RETURN(INT, code); // {+uncovered} + FUNCTION_LOG_RETURN(INT, code); // {+uncovered} } /*********************************************************************************************************************************** diff --git a/src/protocol/client.c b/src/protocol/client.c index f15ccfeea..1d466445f 100644 --- a/src/protocol/client.c +++ b/src/protocol/client.c @@ -162,12 +162,21 @@ protocolClientReadOutput(ProtocolClient *this, bool outputRequired) { const ErrorType *type = errorTypeFromCode(varIntForce(error)); const String *message = varStr(kvGet(responseKv, VARSTR(PROTOCOL_OUTPUT_STR))); - const String *stack = varStr(kvGet(responseKv, VARSTR(PROTOCOL_ERROR_STACK_STR))); - THROWP_FMT( - type, "%s: %s%s", strPtr(this->errorPrefix), message == NULL ? "no details available" : strPtr(message), - type == &AssertError || logWill(logLevelDebug) ? - (stack == NULL ? "\nno stack trace available" : strPtr(strNewFmt("\n%s", strPtr(stack)))) : ""); + // Required part of the message + String *throwMessage = strNewFmt( + "%s: %s", strPtr(this->errorPrefix), message == NULL ? "no details available" : strPtr(message)); + + // Add stack trace if the error is an assertion or debug-level logging is enabled + if (type == &AssertError || logWill(logLevelDebug)) + { + const String *stack = varStr(kvGet(responseKv, VARSTR(PROTOCOL_ERROR_STACK_STR))); + + strCat(throwMessage, "\n"); + strCat(throwMessage, stack == NULL ? "no stack trace available" : strPtr(stack)); + } + + THROWP(type, strPtr(throwMessage)); } // Get output diff --git a/src/protocol/parallel.c b/src/protocol/parallel.c index a73c00e84..7a19557df 100644 --- a/src/protocol/parallel.c +++ b/src/protocol/parallel.c @@ -148,7 +148,7 @@ protocolParallelProcess(ProtocolParallel *this) FD_SET((unsigned int)handle, &selectSet); // Set the max handle - if (handle > handleMax) // {+uncovered - handles are often in ascending order} + if (handle > handleMax) // {+uncovered_branch - often in ascending order} handleMax = handle; clientRunningTotal++; diff --git a/src/storage/posix/storage.c b/src/storage/posix/storage.c index c820eb7a6..2a75c244e 100644 --- a/src/storage/posix/storage.c +++ b/src/storage/posix/storage.c @@ -551,7 +551,7 @@ storagePosixPathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool if (unlink(strPtr(file)) == -1) { // These errors indicate that the entry is actually a path so we'll try to delete it that way - if (errno == EPERM || errno == EISDIR) // {uncovered - EPERM is not returned on tested systems} + if (errno == EPERM || errno == EISDIR) // {uncovered_branch - no EPERM on tested systems} storagePosixPathRemove(this, file, false, true); // Else error else diff --git a/test/lib/pgBackRestTest/Common/JobTest.pm b/test/lib/pgBackRestTest/Common/JobTest.pm index a49eb6b26..66ecace09 100644 --- a/test/lib/pgBackRestTest/Common/JobTest.pm +++ b/test/lib/pgBackRestTest/Common/JobTest.pm @@ -411,6 +411,7 @@ sub run ($self->{oTest}->{&TEST_DEBUG_UNIT_SUPPRESS} ? '' : " -DDEBUG_UNIT") . (vmWithBackTrace($self->{oTest}->{&TEST_VM}) && $self->{bBackTrace} ? ' -DWITH_BACKTRACE' : '') . ($self->{oTest}->{&TEST_CDEF} ? " $self->{oTest}->{&TEST_CDEF}" : '') . + (vmCoverageC($self->{oTest}->{&TEST_VM}) && $self->{bCoverageUnit} ? ' -DDEBUG_COVERAGE' : '') . ($self->{bDebug} ? '' : ' -DNDEBUG') . ($self->{bDebugTestTrace} ? ' -DDEBUG_TEST_TRACE' : ''); # Flags used to buid harness files diff --git a/test/src/common/harnessDebug.h b/test/src/common/harnessDebug.h index b9bdb82e7..0b7b81963 100644 --- a/test/src/common/harnessDebug.h +++ b/test/src/common/harnessDebug.h @@ -56,11 +56,11 @@ C Debug Harness while (0) #define FUNCTION_HARNESS_RESULT(typeMacroPrefix, result) \ - STACK_TRACE_POP(); \ + STACK_TRACE_POP(false); \ return result \ #define FUNCTION_HARNESS_RESULT_VOID() \ - STACK_TRACE_POP(); + STACK_TRACE_POP(false); #endif #endif diff --git a/test/src/lcov.conf b/test/src/lcov.conf index ea6c97faf..c03301b0b 100644 --- a/test/src/lcov.conf +++ b/test/src/lcov.conf @@ -5,14 +5,16 @@ lcov_branch_coverage=1 # Specify the regular expression of lines to exclude from branch coverage # -# [A-Z0-9_]+\( - exclude all macros since they are tested separately -# assert\( - exclude asserts since it usually not possible to trigger both branches -# testBegin\( - exclude because it generally returns true, and is not needed for coverage +# OBJECT_DEFINE_[A-Z0-9_]+\( - exclude object definitions +# \s{4}[A-Z][A-Z0-9_]+\([^\?]*\) - exclude macros that do not take a conditional parameter and are not themselves a parameter +# \s{4}(TEST_|HARNESS_)[A-Z0-9_]+\( - exclude macros used in unit tests +# ASSERT/(|assert\( - exclude asserts since it usually not possible to trigger both branches +# (testBegin\( - exclude because it generally returns true, and is not needed for coverage # switch \( - lcov requires default: to show complete coverage but --Wswitch-enum enforces all enum values be present -lcov_excl_br_line=[A-Z0-9_]+\(|assert\(|testBegin\(| switch \( +lcov_excl_br_line=OBJECT_DEFINE_[A-Z0-9_]+\(|\s{4}[A-Z][A-Z0-9_]+\([^\?]*\)|\s{4}(TEST_|HARNESS_)[A-Z0-9_]+\(|\s{4}(ASSERT|assert|switch\s)\(|\(testBegin\(|\{\+{0,1}uncover(ed|able)_branch # Specify the regular expression of lines to exclude -lcov_excl_line=\{\+*uncovered|\{\+*uncoverable +lcov_excl_line=\{\+{0,1}uncovered[^_]|\{\+{0,1}uncoverable[^_] # Coverage rate limits genhtml_hi_limit = 100 diff --git a/test/src/module/command/archiveCommonTest.c b/test/src/module/command/archiveCommonTest.c index a97f5b63e..d00bc5e75 100644 --- a/test/src/module/command/archiveCommonTest.c +++ b/test/src/module/command/archiveCommonTest.c @@ -224,6 +224,10 @@ testRun(void) " 123456781234567812345678-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" ", 123456781234567812345678-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.gz" "\nHINT: are multiple primaries archiving to this stanza?"); + + TEST_RESULT_STR( + walSegmentFind(storageRepo(), strNew("9.6-2"), strNew("123456781234567812345678.partial")), NULL, + "did not find partial segment"); } // ***************************************************************************************************************************** diff --git a/test/src/module/common/cryptoTest.c b/test/src/module/common/cryptoTest.c index 057d07fa2..b9dcc5225 100644 --- a/test/src/module/common/cryptoTest.c +++ b/test/src/module/common/cryptoTest.c @@ -65,7 +65,7 @@ testRun(void) int nonZeroTotal = 0; for (unsigned int charIdx = 0; charIdx < bufferSize; charIdx++) - if (buffer[charIdx] != 0) // {uncovered - ok if there are no zeros} + if (buffer[charIdx] != 0) // {uncoverable_branch - ok if there are no zeros} nonZeroTotal++; TEST_RESULT_INT_NE(nonZeroTotal, 0, "check that there are non-zero values in the buffer"); diff --git a/test/src/module/common/errorTest.c b/test/src/module/common/errorTest.c index 6ca14972e..2e7da0db7 100644 --- a/test/src/module/common/errorTest.c +++ b/test/src/module/common/errorTest.c @@ -256,6 +256,8 @@ testRun(void) // ***************************************************************************************************************************** if (testBegin("THROW_SYS_ERROR() and THROW_SYS_ERROR_FMT()")) { + THROW_ON_SYS_ERROR_FMT(false, AssertError, "no error"); + TRY_BEGIN() { errno = E2BIG; diff --git a/test/src/module/common/exitTest.c b/test/src/module/common/exitTest.c index 64e33a716..beafb20af 100644 --- a/test/src/module/common/exitTest.c +++ b/test/src/module/common/exitTest.c @@ -34,7 +34,7 @@ testRun(void) exitInit(); raise(SIGTERM); } - HARNESS_FORK_CHILD_END(); + HARNESS_FORK_CHILD_END(); // {uncoverable - signal is raised in block} } HARNESS_FORK_END(); } diff --git a/test/src/module/common/iniTest.c b/test/src/module/common/iniTest.c index b44953739..1193235b9 100644 --- a/test/src/module/common/iniTest.c +++ b/test/src/module/common/iniTest.c @@ -63,6 +63,7 @@ testRun(void) TEST_RESULT_VOID(iniSet(ini, strNew("section2"), strNew("key2"), strNew("7")), "set section2, key"); TEST_RESULT_BOOL(iniSectionKeyIsList(ini, strNew("section2"), strNew("key2")), true, "section2, key2 is a list"); TEST_RESULT_STR(strPtr(strLstJoin(iniGetList(ini, strNew("section2"), strNew("key2")), "|")), "2|7", "get list"); + TEST_RESULT_STR(iniGetList(ini, strNew("section2"), strNew("key-missing")), NULL, "get missing list"); TEST_RESULT_VOID(iniFree(ini), "free ini"); } diff --git a/test/src/module/common/ioTest.c b/test/src/module/common/ioTest.c index ed99ff5cc..48a84f5e9 100644 --- a/test/src/module/common/ioTest.c +++ b/test/src/module/common/ioTest.c @@ -298,6 +298,7 @@ testRun(void) TEST_RESULT_PTR(ioFilterMove(NULL, memContextTop()), NULL, " move NULL filter to top context"); TEST_RESULT_BOOL(ioReadOpen(bufferRead), true, " open"); + TEST_RESULT_INT(ioReadHandle(bufferRead), -1, " handle invalid"); TEST_RESULT_BOOL(ioReadEof(bufferRead), false, " not eof"); TEST_RESULT_SIZE(ioRead(bufferRead, buffer), 2, " read 2 bytes"); TEST_RESULT_SIZE(ioRead(bufferRead, buffer), 0, " read 0 bytes (full buffer)"); @@ -433,6 +434,7 @@ testRun(void) TEST_RESULT_VOID(ioWriteFilterGroupSet(bufferWrite, filterGroup), " add filter group to write io"); TEST_RESULT_VOID(ioWriteOpen(bufferWrite), " open buffer write object"); + TEST_RESULT_INT(ioWriteHandle(bufferWrite), -1, " handle invalid"); TEST_RESULT_VOID(ioWriteLine(bufferWrite, BUFSTRDEF("AB")), " write line"); TEST_RESULT_VOID(ioWrite(bufferWrite, bufNew(0)), " write 0 bytes"); TEST_RESULT_VOID(ioWrite(bufferWrite, NULL), " write 0 bytes"); diff --git a/test/src/module/common/ioTlsTest.c b/test/src/module/common/ioTlsTest.c index 22cf65c72..56911c6b4 100644 --- a/test/src/module/common/ioTlsTest.c +++ b/test/src/module/common/ioTlsTest.c @@ -130,7 +130,7 @@ testRun(void) // Certificate location and validation errors // ------------------------------------------------------------------------------------------------------------------------- // Add test hosts - if (system( // {uncovered - test code} + if (system( // {uncoverable_branch} "echo \"127.0.0.1 test.pgbackrest.org host.test2.pgbackrest.org test3.pgbackrest.org\" |" " sudo tee -a /etc/hosts > /dev/null") != 0) { diff --git a/test/src/module/common/logTest.c b/test/src/module/common/logTest.c index 0a6c7a9b1..648a57c5e 100644 --- a/test/src/module/common/logTest.c +++ b/test/src/module/common/logTest.c @@ -84,7 +84,7 @@ testLogResult(const char *logFile, const char *expected) char actual[32768]; testLogLoad(logFile, actual, sizeof(actual)); - if (strcmp(actual, expected) != 0) // {uncovered - no errors in test} + if (strcmp(actual, expected) != 0) // {uncoverable_branch} THROW_FMT( // {+uncovered} AssertError, "\n\nexpected log:\n\n%s\n\nbut actual log was:\n\n%s\n\n", expected, actual); diff --git a/test/src/module/common/stackTraceTest.c b/test/src/module/common/stackTraceTest.c index 85cfcebf3..c54bc23fc 100644 --- a/test/src/module/common/stackTraceTest.c +++ b/test/src/module/common/stackTraceTest.c @@ -64,19 +64,19 @@ testRun(void) stackTraceInit(testExe()); #endif - TEST_ERROR(stackTracePop("file1", "function1"), AssertError, "assertion 'stackSize > 0' failed"); + TEST_ERROR(stackTracePop("file1", "function1", false), AssertError, "assertion 'stackSize > 0' failed"); assert(stackTracePush("file1", "function1", logLevelDebug) == logLevelDebug); assert(stackSize == 1); TEST_ERROR( - stackTracePop("file2", "function2"), AssertError, + stackTracePop("file2", "function2", false), AssertError, "popping file2:function2 but expected file1:function1"); assert(stackTracePush("file1", "function1", logLevelDebug) == logLevelDebug); TEST_ERROR( - stackTracePop("file1", "function2"), AssertError, + stackTracePop("file1", "function2", false), AssertError, "popping file1:function2 but expected file1:function1"); TRY_BEGIN() @@ -172,9 +172,18 @@ testRun(void) "stack trace"); #endif - stackTracePop("file4.c", "function4"); + stackTracePop("file4.c", "function4", false); assert(stackSize == 4); + // Check that stackTracePop() works with test tracing + stackTracePush("file_test.c", "function_test", logLevelDebug); + stackTracePop("file_test.c", "function_test", true); + + // Check that stackTracePop() does nothing when test tracing is disabled + stackTraceTestStop(); + stackTracePop("bogus.c", "bogus", true); + stackTraceTestStart(); + THROW(ConfigError, "test"); } CATCH(ConfigError) diff --git a/test/src/module/config/defineTest.c b/test/src/module/config/defineTest.c index 949015a84..0dc333950 100644 --- a/test/src/module/config/defineTest.c +++ b/test/src/module/config/defineTest.c @@ -91,6 +91,7 @@ testRun(void) TEST_RESULT_BOOL(cfgDefOptionInternal(cfgDefCmdRestore, cfgDefOptTest), true, "option test is internal"); TEST_RESULT_BOOL(cfgDefOptionMulti(cfgDefOptRecoveryOption), true, "recovery-option is multi"); + TEST_RESULT_BOOL(cfgDefOptionMulti(cfgDefOptDbInclude), true, "db-include is multi"); TEST_RESULT_BOOL(cfgDefOptionMulti(cfgDefOptStartFast), false, "start-fast is not multi"); TEST_RESULT_STR(cfgDefOptionPrefix(cfgDefOptPgHost), "pg", "option prefix"); diff --git a/test/src/module/config/loadTest.c b/test/src/module/config/loadTest.c index 97eac273b..d775ee912 100644 --- a/test/src/module/config/loadTest.c +++ b/test/src/module/config/loadTest.c @@ -250,6 +250,18 @@ testRun(void) " HINT: to retain differential backups indefinitely (without warning), set option 'repo1-retention-diff'" " to the maximum."); + argList = strLstNew(); + strLstAdd(argList, strNew("pgbackrest")); + strLstAdd(argList, strNew("--stanza=db")); + strLstAdd(argList, strNew("--no-log-timestamp")); + strLstAdd(argList, strNew("--repo1-retention-archive-type=diff")); + strLstAdd(argList, strNew("--repo1-retention-archive=3")); + strLstAdd(argList, strNew("--repo1-retention-diff=2")); + strLstAdd(argList, strNew("--repo1-retention-full=1")); + strLstAdd(argList, strNew("expire")); + + TEST_RESULT_VOID(harnessCfgLoad(strLstSize(argList), strLstPtr(argList)), "load config with success"); + // ------------------------------------------------------------------------------------------------------------------------- setenv("PGBACKREST_REPO1_S3_KEY", "mykey", true); setenv("PGBACKREST_REPO1_S3_KEY_SECRET", "mysecretkey", true); diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index 45db2fb09..4922b2466 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -1146,6 +1146,7 @@ testRun(void) setenv("PGBACKREST_RESET_REPO1_HOST", "", true); setenv("PGBACKREST_TARGET", "xxx", true); setenv("PGBACKREST_ONLINE", "y", true); + setenv("PGBACKREST_DELTA", "y", true); setenv("PGBACKREST_START_FAST", "n", true); setenv("PGBACKREST_PG1_SOCKET_PATH", "/path/to/socket", true); @@ -1155,6 +1156,7 @@ testRun(void) "[global]\n" "compress-level=3\n" "spool-path=/path/to/spool\n" + "lock-path=/\n" "\n" "[global:backup]\n" "repo1-hardlink=y\n" @@ -1194,6 +1196,8 @@ testRun(void) TEST_RESULT_BOOL(cfgOptionTest(cfgOptPgHost), false, " pg1-host is not set (command line reset override)"); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgPath)), "/path/to/db", " pg1-path is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptPgPath), cfgSourceConfig, " pg1-path is source config"); + TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptLockPath)), "/", " lock-path is set"); + TEST_RESULT_INT(cfgOptionSource(cfgOptLockPath), cfgSourceConfig, " lock-path is source config"); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptPgSocketPath)), "/path/to/socket", " pg1-socket-path is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptPgSocketPath), cfgSourceConfig, " pg1-socket-path is config param"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptOnline), false, " online not is set"); @@ -1210,6 +1214,8 @@ testRun(void) TEST_RESULT_INT(cfgOptionSource(cfgOptCompressLevel), cfgSourceConfig, " compress-level is source config"); TEST_RESULT_BOOL(cfgOptionBool(cfgOptBackupStandby), false, " backup-standby not is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptBackupStandby), cfgSourceDefault, " backup-standby is source default"); + TEST_RESULT_BOOL(cfgOptionBool(cfgOptDelta), true, " delta is set"); + TEST_RESULT_INT(cfgOptionSource(cfgOptDelta), cfgSourceConfig, " delta is source config"); TEST_RESULT_BOOL(cfgOptionInt64(cfgOptBufferSize), 65536, " buffer-size is set"); TEST_RESULT_INT(cfgOptionSource(cfgOptBufferSize), cfgSourceConfig, " backup-standby is source config"); diff --git a/test/src/module/protocol/protocolTest.c b/test/src/module/protocol/protocolTest.c index b91309cfc..86c40ddd8 100644 --- a/test/src/module/protocol/protocolTest.c +++ b/test/src/module/protocol/protocolTest.c @@ -252,7 +252,7 @@ testRun(void) // Throw errors TEST_RESULT_STR(strPtr(ioReadLine(read)), "{\"cmd\":\"noop\"}", "noop with error text"); - ioWriteStrLine(write, strNew("{\"err\":25,\"out\":\"sample error message\"}")); + ioWriteStrLine(write, strNew("{\"err\":25,\"out\":\"sample error message\",\"errStack\":\"stack data\"}")); ioWriteFlush(write); TEST_RESULT_STR(strPtr(ioReadLine(read)), "{\"cmd\":\"noop\"}", "noop with no error text"); @@ -321,8 +321,15 @@ testRun(void) // Throw errors TEST_ERROR( protocolClientNoOp(client), AssertError, - "raised from test client: sample error message\nno stack trace available"); - TEST_ERROR(protocolClientNoOp(client), UnknownError, "raised from test client: no details available"); + "raised from test client: sample error message\nstack data"); + + harnessLogLevelSet(logLevelDebug); + TEST_ERROR( + protocolClientNoOp(client), UnknownError, + "raised from test client: no details available\nno stack trace available"); + harnessLogLevelReset(); + + // No output expected TEST_ERROR(protocolClientNoOp(client), AssertError, "no output required by command"); // Get command output diff --git a/test/src/module/storage/posixTest.c b/test/src/module/storage/posixTest.c index fa4454e09..6de52acf6 100644 --- a/test/src/module/storage/posixTest.c +++ b/test/src/module/storage/posixTest.c @@ -116,6 +116,9 @@ testRun(void) TEST_ERROR_FMT( storagePosixFileClose(-99, fileName, true), FileCloseError, "unable to close '%s': [9] Bad file descriptor", strPtr(fileName)); + TEST_ERROR_FMT( + storagePosixFileClose(-99, fileName, false), PathCloseError, + "unable to close '%s': [9] Bad file descriptor", strPtr(fileName)); TEST_RESULT_VOID(storagePosixFileClose(handle, fileName, true), "close file"); @@ -709,7 +712,10 @@ testRun(void) fileName = strNewFmt("%s/sub2/testfile", testPath()); TEST_ASSIGN( - file, storageNewWriteP(storageTest, fileName, .modePath = 0700, .modeFile = 0600), "new write file (set mode)"); + file, + storageNewWriteP( + storageTest, fileName, .modePath = 0700, .modeFile = 0600, .group = strNew(getgrgid(getgid())->gr_name)), + "new write file (set mode)"); TEST_RESULT_VOID(ioWriteOpen(storageWriteIo(file)), " open file"); TEST_RESULT_VOID(ioWriteClose(storageWriteIo(file)), " close file"); TEST_RESULT_VOID(storageWritePosixClose(storageWriteDriver(file)), " close file again");