From 1406779771fbc37b87cb6d0b2fb411583e445c02 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 13 Aug 2021 13:27:35 -0400 Subject: [PATCH] Fix #1815, add retroactive CFE status asserts Add a pair of macros that decouple the function call from the expected status: - CFE_UtAssert_STATUS_STORE - CFE_UtAssert_STATUS_MAY_BE - CFE_UtAssert_STATUS_MUST_BE The first will make the function call and put the status into a temporary holding area, but will not assert on any particular result. The may/must be will assert the value of the holding area. --- modules/cfe_assert/inc/cfe_assert.h | 135 +++++++++++++++++++++ modules/cfe_assert/src/cfe_assert_priv.h | 22 ++++ modules/cfe_assert/src/cfe_assert_runner.c | 48 ++++++++ modules/cfe_testcase/src/es_cds_test.c | 28 ++++- 4 files changed, 228 insertions(+), 5 deletions(-) diff --git a/modules/cfe_assert/inc/cfe_assert.h b/modules/cfe_assert/inc/cfe_assert.h index dc5736920..b6a190a87 100644 --- a/modules/cfe_assert/inc/cfe_assert.h +++ b/modules/cfe_assert/inc/cfe_assert.h @@ -151,6 +151,101 @@ typedef void (*CFE_Assert_StatusCallback_t)(uint8 MessageType, const char *Prefi UtAssert_GenericUnsignedCompare(CFE_SB_MsgIdToValue(mid1), UtAssert_Compare_EQ, CFE_SB_MsgIdToValue(mid2), \ UtAssert_Radix_HEX, __FILE__, __LINE__, "MsgId Check: ", #mid1, #mid2) +/*****************************************************************************/ +/** +** \brief Calls a function that returns CFE_Status_t with deferred validation +** +** \par Description +** The typical method of using #UtAssert_INT32_EQ to validate the result of a CFE call +** will both invoke the function and check the result in a single macro. However, this +** requires advance knowledge of what the result is supposed to be, before the call is made. +** +** This macro does invokes the function like the other macros do, but does _not_ check the +** return value. Rather, it stores the return the value in a local buffer for later checking, +** using the #CFE_Assert_STATUS_MAY_BE or #CFE_Assert_STATUS_MUST_BE macros. +** +** \par Assumptions, External Events, and Notes: +** In some functional test circumstances, particularly where the test is not being run +** in a clean/isolated environment, it may not always be feasible to predict the correct +** return code from an API call. +** +** In these cases, the test program will need to check the result of the call itself, +** typically by storing the result on the stack and checking it for correctness by +** reading system state as necessary. +** +** While the normal UtAssert_INT32_EQ assertion macro can still be used to retroactively check +** any status variable value (including a value on the stack from a previous API call), this +** will not include the "full text" of the API call, and so the test log will not reflect the +** full call details. +** +** The pair of macros (CFE_Assert_STATUS_STORE and CFE_Assert_STATUS_WAS) can be used in these +** circumstances, to call an API function and record the full text of the call, when the expected +** status is not fully known, but still confirm the status was correct once it is known. +** +** \sa #CFE_Assert_STATUS_MAY_BE, #CFE_Assert_STATUS_MUST_BE +** +** \returns Actual CFE_Status_t value from the call +*/ +#define CFE_Assert_STATUS_STORE(FN) CFE_Assert_Status_Store(FN, __FILE__, __LINE__, #FN) + +/*****************************************************************************/ +/** +** \brief Retroactively check for an acceptable status value from CFE_Assert_STATUS_STORE +** +** \par Description +** The typical method of using #UtAssert_INT32_EQ to validate the result of a CFE call +** will both invoke the function and check the result in a single macro. However, this +** requires advance knowledge of what the result is supposed to be, before the call is made. +** +** This retroactive macro does _not_ invoke any function, but rather checks the stored status +** from a previous call to #CFE_Assert_STATUS_STORE. It should be used for each status +** code that should be considered acceptable from the previous function call. +** +** \par Assumptions, External Events, and Notes: +** +** While the normal UtAssert_INT32_EQ assertion macro can still be used to check any +** status variable value (including a value on the stack from a previous API call), this +** will not include the "full text" of the API call. This macro is intended for those cases +** where it is desired to log the full text (function + arguments) of the API call, but +** when the call has already been made and the status value is stored in a local variable. +** +** \sa #CFE_Assert_STATUS_STORE, #CFE_Assert_STATUS_MUST_BE +** +** \returns Boolean pass/fail status +** +******************************************************************************/ +#define CFE_Assert_STATUS_MAY_BE(expected) \ + CFE_Assert_Status_DeferredCheck(expected, UTASSERT_CASETYPE_NA, __FILE__, __LINE__, #expected) + +/*****************************************************************************/ +/** +** \brief Retroactively check for a required status value from CFE_Assert_STATUS_STORE +** +** \par Description +** The typical method of using #UtAssert_INT32_EQ to validate the result of a CFE call +** will both invoke the function and check the result in a single macro. However, this +** requires advance knowledge of what the result is supposed to be, before the call is made. +** +** This retroactive macro does _not_ invoke any function, but rather checks the stored status +** from a previous call to #CFE_Assert_STATUS_STORE. This should be used as the final +** assertion, after checking for other acceptable values via #CFE_Assert_STATUS_MAY_BE. +** +** \par Assumptions, External Events, and Notes: +** +** While the normal UtAssert_INT32_EQ assertion macro can still be used to check any +** status variable value (including a value on the stack from a previous API call), this +** will not include the "full text" of the API call. This macro is intended for those cases +** where it is desired to log the full text (function + arguments) of the API call, but +** when the call has already been made and the status value is stored in a local variable. +** +** \sa #CFE_Assert_STATUS_STORE, #CFE_Assert_STATUS_MAY_BE +** +** \returns Boolean pass/fail status +** +******************************************************************************/ +#define CFE_Assert_STATUS_MUST_BE(expected) \ + CFE_Assert_Status_DeferredCheck(expected, UTASSERT_CASETYPE_FAILURE, __FILE__, __LINE__, #expected) + /************************************************************************* ** Exported Functions *************************************************************************/ @@ -276,4 +371,44 @@ void CFE_Assert_CloseLogFile(void); bool CFE_UtAssert_StatusCheck(CFE_Status_t Status, bool ExpectSuccess, UtAssert_CaseType_t CaseType, const char *File, uint32 Line, const char *Text); +/*****************************************************************************/ +/** +** \brief Helper function for nominal CFE calls with deferred check +** +** \par Description +** This helper function will store the status into a temporary holding area, +** but _not_ assert on any specific value. +** +** \par Assumptions, External Events, and Notes: +** This facility should only be used by one task at a time. Normally tests +** are single-threaded, and CFE assert will serialize test apps, so this is +** not a concern in the typical test environment. +** +** However, if a particular test case uses child tasks, then the programmer must +** explicitly ensure that only one task uses this facility at a time. +** +** \returns Status value (pass through) +*/ +CFE_Status_t CFE_Assert_Status_Store(CFE_Status_t Status, const char *File, uint32 Line, const char *Text); + +/** +** \brief Helper function for nominal CFE calls with deferred check +** +** \par Description +** This helper function will assert on the status previously stored to a +** temporary holding area. +** +** \par Assumptions, External Events, and Notes: +** This facility should only be used by one task at a time. Normally tests +** are single-threaded, and CFE assert will serialize test apps, so this is +** not a concern in the typical test environment. +** +** However, if a particular test case uses child tasks, then the programmer must +** explicitly ensure that only one task uses this facility at a time. +** +** \returns Test pass status, returns true if status was successful, false if it failed. +*/ +bool CFE_Assert_Status_DeferredCheck(CFE_Status_t Status, UtAssert_CaseType_t CaseType, const char *File, uint32 Line, + const char *Text); + #endif /* CFE_ASSERT_H */ diff --git a/modules/cfe_assert/src/cfe_assert_priv.h b/modules/cfe_assert/src/cfe_assert_priv.h index c90539eb5..5925213f7 100644 --- a/modules/cfe_assert/src/cfe_assert_priv.h +++ b/modules/cfe_assert/src/cfe_assert_priv.h @@ -136,6 +136,28 @@ typedef struct */ char CurrentTestName[CFE_MISSION_MAX_API_LEN]; + /* The following members support the "Deferred" assert feature */ + + /** + * Actual CFE status value from a previous function call + */ + CFE_Status_t StoredStatus; + + /** + * Full text of previous function call that produced "StoredStatus" + */ + char StoredText[CFE_ASSERT_MAX_LOG_LINE_LENGTH]; + + /** + * File name of source file that produced "StoredStatus" + */ + char StoredFile[CFE_MISSION_MAX_PATH_LEN]; + + /** + * Line number of source file that produced "StoredStatus" + */ + uint32 StoredLine; + } CFE_Assert_Global_t; extern CFE_Assert_Global_t CFE_Assert_Global; diff --git a/modules/cfe_assert/src/cfe_assert_runner.c b/modules/cfe_assert/src/cfe_assert_runner.c index ade244352..aec11e210 100644 --- a/modules/cfe_assert/src/cfe_assert_runner.c +++ b/modules/cfe_assert/src/cfe_assert_runner.c @@ -89,6 +89,54 @@ bool CFE_UtAssert_StatusCheck(CFE_Status_t Status, bool ExpectSuccess, UtAssert_ return UtAssertEx(Result, CaseType, File, Line, "%s (0x%lx) is %s", Text, (unsigned long)Status, MatchText); } +CFE_Status_t CFE_Assert_Status_Store(CFE_Status_t Status, const char *File, uint32 Line, const char *Text) +{ + const char *BaseName; + + /* All this needs to do is save the code+text, will assert later */ + CFE_Assert_Global.StoredStatus = Status; + strncpy(CFE_Assert_Global.StoredText, Text, sizeof(CFE_Assert_Global.StoredText) - 1); + CFE_Assert_Global.StoredText[sizeof(CFE_Assert_Global.StoredText) - 1] = 0; + + BaseName = strrchr(File, '/'); + if (BaseName == NULL) + { + BaseName = File; + } + else + { + ++BaseName; + } + strncpy(CFE_Assert_Global.StoredFile, BaseName, sizeof(CFE_Assert_Global.StoredFile) - 1); + CFE_Assert_Global.StoredFile[sizeof(CFE_Assert_Global.StoredFile) - 1] = 0; + CFE_Assert_Global.StoredLine = Line; + + /* Status code is just passed thru so the test case can check it however it needs to */ + return Status; +} + +bool CFE_Assert_Status_DeferredCheck(CFE_Status_t Status, UtAssert_CaseType_t CaseType, const char *File, uint32 Line, + const char *Text) +{ + bool Result; + if (CFE_Assert_Global.StoredText[0] == 0) + { + /* If no status was stored, then this is a bug in the test program (need to store a result first) */ + UtAssertEx(false, UTASSERT_CASETYPE_FAILURE, File, Line, "TEST BUG: No stored status to assert (%s)", Text); + Result = false; + } + else + { + /* This produces a log message similar to what UtAssert_INT32_EQ would produce. + * Note the file/line will reflect where the call was made, not where this assertion was done */ + Result = UtAssertEx(Status == CFE_Assert_Global.StoredStatus, CaseType, CFE_Assert_Global.StoredFile, + CFE_Assert_Global.StoredLine, "%s (%ld) == %s (%ld)", CFE_Assert_Global.StoredText, + (long)CFE_Assert_Global.StoredStatus, Text, (long)Status); + } + + return Result; +} + void CFE_Assert_StatusReport(uint8 MessageType, const char *Prefix, const char *OutputMessage) { uint16 EventType; diff --git a/modules/cfe_testcase/src/es_cds_test.c b/modules/cfe_testcase/src/es_cds_test.c index 42a9778b0..dbca33a5d 100644 --- a/modules/cfe_testcase/src/es_cds_test.c +++ b/modules/cfe_testcase/src/es_cds_test.c @@ -42,22 +42,40 @@ void TestRegisterCDS(void) size_t BlockSize2 = 15; const char * Name = "CDS_Test"; const char * LongName = "VERY_LONG_NAME_CDS_Test"; - CFE_Status_t status; UtPrintf("Testing: CFE_ES_RegisterCDS"); - status = CFE_ES_RegisterCDS(&CDSHandlePtr, BlockSize2, Name); + /* + * Since this test app may be executed multiple times, or the system may have + * been booted from a processor reset rather than a power-on reset, it is possible + * that the CDS already exists at the time this test is executed. In this case + * the new CDS allocation path cannot be checked in functional test, but other CDS + * functions can still be called. + */ - if (status == CFE_ES_CDS_ALREADY_EXISTS) + CFE_Assert_STATUS_STORE(CFE_ES_RegisterCDS(&CDSHandlePtr, BlockSize2, Name)); + + if (CFE_Assert_STATUS_MAY_BE(CFE_ES_CDS_ALREADY_EXISTS)) { + /* + * add an informational message that the functional test is incomplete here, due + * to preconditions beyond the control of this test app. Need to clear the CDS + * memory and/or do a power-on reset to get full test. + */ UtAssert_NA("CDS already exists. CFE_ES_RegisterCDS new allocation could not be properly tested"); } else { - UtAssert_INT32_EQ(status, CFE_SUCCESS); + /* + * If not CFE_ES_CDS_ALREADY_EXISTS, then the only other acceptable status is CFE_SUCCESS, + * which indicates that the CDS was created and initialized from a clean slate. + */ + CFE_Assert_STATUS_MUST_BE(CFE_SUCCESS); + + /* In this case, calling CFE_ES_RegisterCDS() again should return the CFE_ES_CDS_ALREADY_EXISTS */ + UtAssert_INT32_EQ(CFE_ES_RegisterCDS(&CDSHandlePtr2, BlockSize2, Name), CFE_ES_CDS_ALREADY_EXISTS); } - UtAssert_INT32_EQ(CFE_ES_RegisterCDS(&CDSHandlePtr2, BlockSize2, Name), CFE_ES_CDS_ALREADY_EXISTS); UtAssert_INT32_EQ(CFE_ES_RegisterCDS(&CDSHandlePtr2, BlockSize, Name), CFE_SUCCESS); UtAssert_INT32_EQ(CFE_ES_RegisterCDS(NULL, BlockSize, Name), CFE_ES_BAD_ARGUMENT);