From 87f36e814ea95696870711018c050725e8e7269f Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 11 May 2019 14:51:51 -0400 Subject: [PATCH] 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 | 4 +- doc/xml/release.xml | 4 + src/command/help/help.c | 4 +- src/common/debug.h | 17 +-- src/common/error.c | 34 +++++- src/common/error.h | 119 ++++++++++++++------ src/common/ini.c | 2 +- src/common/io/filter/filter.c | 8 +- src/common/io/tls/client.c | 14 +-- src/common/log.h | 13 ++- src/common/regExp.c | 12 +- src/common/stackTrace.c | 13 ++- src/common/stackTrace.h | 8 +- src/common/type/json.c | 8 +- src/common/type/variant.h | 2 +- src/config/load.c | 4 +- src/config/parse.c | 2 +- src/perl/exec.c | 20 ++-- src/protocol/client.c | 19 +++- src/protocol/parallel.c | 2 +- src/storage/posix/storage.c | 2 +- test/lib/pgBackRestTest/Common/JobTest.pm | 1 + test/src/common/harnessDebug.h | 4 +- test/src/lcov.conf | 12 +- test/src/module/command/archiveCommonTest.c | 4 + test/src/module/common/cryptoTest.c | 2 +- test/src/module/common/errorTest.c | 2 + test/src/module/common/exitTest.c | 2 +- test/src/module/common/iniTest.c | 1 + test/src/module/common/ioTest.c | 2 + test/src/module/common/ioTlsTest.c | 2 +- test/src/module/common/logTest.c | 2 +- test/src/module/common/stackTraceTest.c | 17 ++- test/src/module/config/defineTest.c | 1 + test/src/module/config/loadTest.c | 12 ++ test/src/module/config/parseTest.c | 6 + test/src/module/protocol/protocolTest.c | 13 ++- test/src/module/storage/posixTest.c | 8 +- 38 files changed, 279 insertions(+), 123 deletions(-) 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");