Improve macros and coverage rules that were hiding missing coverage.

The branch coverage exclusion rules were overly broad and included functions that ended in a capital letter, which disabled all coverage for the statement.  Improve matching so that all characters in the name must be upper-case for a match.

Some macros with internal branches accepted parameters that might contain conditionals.  This made it impossible to tell which branches belonged to which, and in any case an overzealous exclusion rule was ignoring all branches in such cases.  Add the DEBUG_COVERAGE flag to build a modified version of the macros without any internal branches to be used for coverage testing.  In most cases, the branches were optimizations (like checking logWill()) that improve production performance but are not needed for testing.  In other cases, a parameter needed to be added to the underlying function to handle the branch during coverage testing.

Also tweak the coverage rules so that macros without conditionals are automatically excluded from branch coverage as long as they are not themselves a parameter.

Finally, update tests and code where missing coverage was exposed by these changes.  Some code was updated to remove existing coverage exclusions when it was a simple change.

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)

doc/xml/release.xml

@@ -159,6 +159,10 @@
                         <p>Use <code>THROW_ON_SYS_ERROR*()</code> to improve code coverage.</p>
                     </release-item>
 
+                    <release-item>
+                        <p>Improve macros and coverage rules that were hiding missing coverage.</p>
+                    </release-item>
+
                     <release-item>
                         <p>Improve efficiency of <code>FUNCTION_LOG*()</code> macros.</p>
                     </release-item>

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));

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(true);                                                                                                 \
-                STACK_TRACE_POP();                                                                                                 \
-                                                                                                                                   \
             return result;                                                                                                         \
         }                                                                                                                          \
         while(0);
 
     #define FUNCTION_TEST_RETURN_VOID()                                                                                            \
-        do                                                                                                                         \
+        STACK_TRACE_POP(true);
-        {                                                                                                                          \
-            if (stackTraceTest())                                                                                                  \
-                STACK_TRACE_POP();                                                                                                 \
-        }                                                                                                                          \
-        while(0);
 #else
     #define FUNCTION_TEST_BEGIN()
     #define FUNCTION_TEST_PARAM(typeMacroPrefix, param)

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
 }

src/common/error.h

@@ -141,6 +141,33 @@ The seldom used "THROWP" variants allow an error to be thrown with a pointer to
 /***********************************************************************************************************************************
 Throw an error when a system call fails
 ***********************************************************************************************************************************/
+// 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)                                                                          \
+        errorInternalThrowSys(error, &errorType, __FILE__, __func__, __LINE__, message)
+
+    #define THROW_ON_SYS_ERROR_FMT(error, errorType, ...)                                                                          \
+        errorInternalThrowSysFmt(error, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__)
+
+    #define THROWP_ON_SYS_ERROR(error, errorType, message)                                                                         \
+        errorInternalThrowSys(error, errorType, __FILE__, __func__, __LINE__, message)
+
+    #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, ...)                                                                                    \
@@ -177,6 +204,7 @@ Throw an error when a system call fails
             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, ...)
+#ifdef DEBUG_COVERAGE
-    __attribute__((format(printf, 6, 7))) __attribute__((__noreturn__));
+    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

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)));
 }
 
 /***********************************************************************************************************************************

src/common/io/filter/filter.c

@@ -144,8 +144,12 @@ ioFilterDone(const IoFilter *this)
 
     ASSERT(this != NULL);
 
-    FUNCTION_TEST_RETURN(
+    bool result = false;
-        this->flushing && (this->interface.done != NULL ? this->interface.done(this->driver) : !ioFilterInputSame(this)));
+
+    if (this->flushing)
+        result = this->interface.done != NULL ? this->interface.done(this->driver) : !ioFilterInputSame(this);
+
+    FUNCTION_TEST_RETURN(result);
 }
 
 /***********************************************************************************************************************************

src/common/io/tls/client.c

@@ -253,19 +253,15 @@ 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);
+            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))));
         }
     }
-        }
-    }
     MEM_CONTEXT_TEMP_END();
 
     FUNCTION_LOG_RETURN(BOOL, result);

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)
 

src/common/regExp.c

@@ -41,9 +41,12 @@ regExpError(int error)
         FUNCTION_TEST_PARAM(INT, error);
     FUNCTION_TEST_END();
 
+    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);
-        regExpError(result);                                                    // {+uncoverable}
 
     FUNCTION_TEST_RETURN(result == 0);
 }

src/common/stackTrace.c

@@ -271,10 +271,12 @@ stackTracePop(void)
 #else
 
 void
-stackTracePop(const char *fileName, const char *functionName)
+stackTracePop(const char *fileName, const char *functionName, bool test)
 {
     ASSERT(stackSize > 0);
 
+    if (!test || stackTraceTest())
+    {
         stackSize--;
 
         StackTraceData *data = &stackTrace[stackSize];
@@ -282,6 +284,7 @@ stackTracePop(const char *fileName, const char *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
 

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()                                                                                                      \
+    #define STACK_TRACE_POP(test)                                                                                                  \
-        stackTracePop(__FILE__, __func__);
+        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);

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
+        // Add terminating linefeed for pretty print
-        if (indent > 0 && !strEndsWithZ(jsonStr, "\n"))
+        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
+        // Add terminating linefeed for pretty print
-        if (indent > 0 && !strEndsWithZ(jsonStr, "\n"))
+        if (indent > 0)
             strCat(jsonStr, "\n");
 
         // Duplicate the string into the calling context

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)                                                                                                          \

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));
                 }

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);

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));
+    const char *moduleName = (const char *)SvPV_nolen(ST(0));       // {uncoverable_branch - Perl macro}
-    dXSTARG;                                                        // {uncovered - internal Perl macro branch}
+    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));
+    int code = (int)SvIV(get_sv("iResult", 0));                                     // {uncoverable_branch - Perl macro}
-    bool errorC = (int)SvIV(get_sv("bErrorC", 0));
+    bool errorC = (int)SvIV(get_sv("bErrorC", 0));                                  // {uncoverable_branch - Perl macro}
-    char *message = SvPV_nolen(get_sv("strMessage", 0));                            // {uncovered - internal Perl macro branch}
+    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}

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)));
+
+            // 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)));
 
-            THROWP_FMT(
+                strCat(throwMessage, "\n");
-                type, "%s: %s%s", strPtr(this->errorPrefix), message == NULL ? "no details available" : strPtr(message),
+                strCat(throwMessage, stack == NULL ? "no stack trace available" : strPtr(stack));
-                type == &AssertError || logWill(logLevelDebug) ?
+            }
-                    (stack == NULL ? "\nno stack trace available" : strPtr(strNewFmt("\n%s", strPtr(stack)))) : "");
+
+            THROWP(type, strPtr(throwMessage));
         }
 
         // Get output

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++;

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

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

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

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
+# OBJECT_DEFINE_[A-Z0-9_]+\( - exclude object definitions
-# assert\( - exclude asserts since it usually not possible to trigger both branches
+# \s{4}[A-Z][A-Z0-9_]+\([^\?]*\) - exclude macros that do not take a conditional parameter and are not themselves a parameter
-# testBegin\( - exclude because it generally returns true, and is not needed for coverage
+# \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

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");
     }
 
     // *****************************************************************************************************************************

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");

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;

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();
     }

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");
     }

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");

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)
         {

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);
 

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)

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");

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);

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");
 

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");
+                    "raised from test client: sample error message\nstack data");
-                TEST_ERROR(protocolClientNoOp(client), UnknownError, "raised from test client: no details available");
+
+                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

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");