Skip to content
Permalink
Browse files

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.
  • Loading branch information...
dwsteele committed May 11, 2019
1 parent f819a32 commit 87f36e814ea95696870711018c050725e8e7269f
@@ -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)
@@ -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>
@@ -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));
@@ -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)
@@ -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
}
Oops, something went wrong.

0 comments on commit 87f36e8

Please sign in to comment.
You can’t perform that action at this time.