Incomplete coverage test for cfe_es_task.c

[avan989]

Is your feature request related to a problem? Please describe.
Missing code coverage for the following:

CFE_ES_TaskInit
363 0 : snprintf(EventBuffer, sizeof(EventBuffer), "Mission %s", GLOBAL_CONFIGDATA.Config);

     385           3 :     if ( Status != CFE_SUCCESS )
     386             :     {
     387           0 :        CFE_ES_WriteToSysLog("ES:Error sending version event:RC=0x%08X\n", (unsigned int)Status);
     388           0 :        return(Status);
     389             :     }

     395           3 :     if ( Status != CFE_SUCCESS )
     396             :     {
     397           0 :        CFE_ES_WriteToSysLog("ES:Error sending build info event:RC=0x%08X\n", (unsigned int)Status);
     398           0 :        return(Status);
     399             :     }

CFE_ES_HousekeepingCmd

      696           0 :             CFE_ES_TaskData.HkPacket.Payload.PerfTriggerMask[PerfIdx] = 0;
     697             :         }
     698             :     }

    708             :         {
     709           0 :             CFE_ES_TaskData.HkPacket.Payload.PerfFilterMask[PerfIdx] = 0;
     710             :         }

Requester Info
Anh Van, NASA Goddard

[pepepr08]

I think CFE_ES_HousekeepingCmd contains dead code (the else clause) since both macros will always be the same.

cFE/modules/es/fsw/src/cfe_es_task.c

Lines 796 to 820 in 9d4fcae

	for (PerfIdx = 0; PerfIdx < CFE_ES_PERF_TRIGGERMASK_EXT_SIZE; ++PerfIdx)
	{
	if (PerfIdx < CFE_ES_PERF_TRIGGERMASK_INT_SIZE)
	{
	CFE_ES_Global.TaskData.HkPacket.Payload.PerfTriggerMask[PerfIdx] =
	CFE_ES_Global.ResetDataPtr->Perf.MetaData.TriggerMask[PerfIdx];
	}
	else
	{
	CFE_ES_Global.TaskData.HkPacket.Payload.PerfTriggerMask[PerfIdx] = 0;
	}
	}
	for (PerfIdx = 0; PerfIdx < CFE_ES_PERF_FILTERMASK_EXT_SIZE; ++PerfIdx)
	{
	if (PerfIdx < CFE_ES_PERF_FILTERMASK_INT_SIZE)
	{
	CFE_ES_Global.TaskData.HkPacket.Payload.PerfFilterMask[PerfIdx] =
	CFE_ES_Global.ResetDataPtr->Perf.MetaData.FilterMask[PerfIdx];
	}
	else
	{
	CFE_ES_Global.TaskData.HkPacket.Payload.PerfFilterMask[PerfIdx] = 0;
	}
	}

CFE_ES_PERF_TRIGGERMASK_INT_SIZE same as CFE_ES_PERF_TRIGGERMASK_EXT_SIZE
and
CFE_ES_PERF_FILTERMASK_INT_SIZE same as CFE_ES_PERF_FILTERMASK_EXT_SIZE

They ultimately use the same macro CFE_MISSION_ES_PERF_MAX_IDS

[skliper]

Likely intended to be able to support a smaller CFE_PLATFORM_ES_PERF_MAX_IDS, although not implemented that way yet. Could you open an issue so we can track it? Need to either fully support a platform limit or remove the internal/external defines and logic. For this round we'll just document the open issue in relation to the uncovered code.

[pepepr08]

Likely intended to be able to support a smaller CFE_PLATFORM_ES_PERF_MAX_IDS, although not implemented that way yet. Could you open an issue so we can track it? Need to either fully support a platform limit or remove the internal/external defines and logic. For this round we'll just document the open issue in relation to the uncovered code.

See #1652

[pepepr08]

       22:  271:    ModuleNamePtr = GLOBAL_CONFIGDATA.CoreModuleList;
       22:  272:    if (ModuleNamePtr != NULL)
        -:  273:    {
    #####:  274:        while (Status == CFE_SUCCESS && ModuleNamePtr->Name != NULL)
        -:  275:        {
    #####:  276:            Status = CFE_ES_GenerateSingleVersionEvent("Core Module", ModuleNamePtr->Name);
    #####:  277:            if (Status != CFE_SUCCESS)
        -:  278:            {
    #####:  279:                CFE_ES_WriteToSysLog("%s: Error sending core module version event:RC=0x%08X\n", __func__,
        -:  280:                                     (unsigned int)Status);
        -:  281:            }
    #####:  282:            ++ModuleNamePtr;
        -:  283:        }
        -:  284:    }
        -:  285:
        -:  286:    /*
        -:  287:     * Advertise PSP module versions
        -:  288:     */
       22:  289:    StaticModulePtr = GLOBAL_CONFIGDATA.PspModuleList;
       22:  290:    if (StaticModulePtr != NULL)
        -:  291:    {
    #####:  292:        while (Status == CFE_SUCCESS && StaticModulePtr->Name != NULL)
        -:  293:        {
    #####:  294:            Status = CFE_ES_GenerateSingleVersionEvent("PSP Module", StaticModulePtr->Name);
    #####:  295:            if (Status != CFE_SUCCESS)
        -:  296:            {
    #####:  297:                CFE_ES_WriteToSysLog("%s: Error sending PSP module version event:RC=0x%08X\n", __func__,
        -:  298:                                     (unsigned int)Status);
        -:  299:            }
    #####:  300:            ++StaticModulePtr;
        -:  301:        }
        -:  302:    }

Testing these lines requires modifying GLOBAL_CONFIGDATA, which is const. The PSP repo defines this global (in ut_psp_stubs.c). Even if modified to add cover this lines, then we wouldn't be able to cover the branch case when the values are NULL.

ut_psp_stubs.c

CFE_ConfigName_t CFE_CORE_MODULE_LIST_UT[] = {
    { "core_api" },
    { NULL } /* End of list */
};

const CFE_StaticModuleLoadEntry_t CFE_PSP_MODULE_LIST_UT[] = {
    {"ut_module", NULL},
    { NULL, NULL } /* End of list */
};

const CFE_ConfigKeyValue_t CFE_MODULE_VERSION_TABLE_UT[] = {
    { "Mission", "ut_build" },
    { NULL, NULL } /* End of list */
};

/**
 * Instantiation of global system-wide configuration struct
 * This contains build info plus pointers to the PSP and CFE
 * configuration structures.  Everything will be linked together
 * in the final executable.
 */
Target_ConfigData GLOBAL_CONFIGDATA = {.MissionVersion       = "MissionUnitTest",
                                       .CfeVersion           = "CfeUnitTest",
                                       .OsalVersion          = "OsalUnitTest",
                                       .Config               = "MissionConfig",
                                       .Date                 = "MissionBuildDate",
                                       .User                 = "MissionBuildUser",
                                       .Default_CpuName      = "UnitTestCpu",
                                       .Default_CpuId        = 1,
                                       .Default_SpacecraftId = 0x42,
                                       .CoreModuleList       = CFE_CORE_MODULE_LIST_UT,
                                       .PspModuleList        = CFE_PSP_MODULE_LIST_UT,
                                       .ModuleVersionList    = CFE_MODULE_VERSION_TABLE_UT,
                                       .CfeConfig            = &GLOBAL_CFE_CONFIGDATA};

[pepepr08]

      106:  189:const char *CFE_ES_FindConfigKeyValue(const CFE_ConfigKeyValue_t *ConfigList, const char *KeyName)
        -:  190:{
        -:  191:    const char *ValuePtr;
        -:  192:
      106:  193:    ValuePtr = NULL;
      106:  194:    if (KeyName != NULL && ConfigList != NULL)
        -:  195:    {
    #####:  196:        while (ConfigList->Key != NULL)
        -:  197:        {
    #####:  198:            if (strcmp(KeyName, ConfigList->Key) == 0)
        -:  199:            {
    #####:  200:                ValuePtr = ConfigList->Value;
    #####:  201:                break;
        -:  202:            }
        -:  203:
    #####:  204:            ++ConfigList;
        -:  205:        }
        -:  206:    }
        -:  207:
      106:  208:    return ValuePtr;
        -:  209:}

Testing the non NULL case on this function requires access to this internal-only function. Possible solutions to test these lines is to add:
In es_UT.h:

/* Access to internal ES function */
#include "target_config.h"
const char *CFE_ES_FindConfigKeyValue(const CFE_ConfigKeyValue_t *ConfigList, const char *KeyName);

In es_UT.c:

    /* Test CFE_ES_FindConfigKeyValue loop */
    ES_ResetUnitTest();
    UT_String = CFE_ES_FindConfigKeyValue(CFE_MODULE_VERSION_TABLE_UT, "Mission");
    CFE_UtAssert_STRINGBUF_EQ(UT_String, 8, "ut_build", 8);

[skliper]

See nasa/PSP#298 for supporting override of GLOBAL_CONFIGDATA from test context.