Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIs Missing Unit Tests #548

Closed
dmknutsen opened this issue Mar 10, 2020 · 2 comments
Closed

APIs Missing Unit Tests #548

dmknutsen opened this issue Mar 10, 2020 · 2 comments

Comments

@dmknutsen
Copy link
Contributor

Describe the bug
The following APIs need to have additional Unit Tests added:

cfe_es_api.c:CFE_ES_DeleteApp - Should test on boundary conditions for AppID
cfe_es_api.c:CFE_ES_GetAppIDByName - Don’t see where this is ever called in unit test
cfe_es_api.c:CFE_ES_ReloadApp - Should test on boundary conditions for AppID
cfe_es_api.c:CFE_ES_RestartApp - No success path test
cfe_esmempool.c:CFE_ES_GetMemPoolStats - No success path test
cfe_sb.h:CFE_SB_MsgId_Equal - Don’t see where this is ever called in unit test
cfe_sb.h:CFE_SB_MsgIdToValue - Don’t see where this is ever called in unit test
cfe_sb.h:CFE_SB_ValueToMsgId - Don’t see where this is ever called in unit test
cfe_sb_api.c:CFE_SB_SubscribeFull - There are 3 cases where CFE_SB_BAD_ARGUMENT can get returned…UT should test each one individually. Also this is outside the scope of the argument validation audit, but there are 3 additional return codes not tested in the unit test.
cfe_sb_api.c:CFE_SB_SubscribeLocal - Consider subscribing to message with limit greater than CFE_PLATFORM_SB_DEFAULT_MSG_LIMIT
cfe_tbl_api.c:CFE_TBL_DumpToBuffer - No success path test
cfe_tbl_api.c:CFE_TBL_GetAddresses - Should test on boundary conditions for NumTables
cfe_tbl_api.c:CFE_TBL_GetStatus - Don’t see a success path test
cfe_tbl_api.c:CFE_TBL_ReleaseAddresses - No success path test. Also, should test on boundary conditions for NumTables
cfe_tbl_api.c:CFE_TBL_Update - Don’t see a success path test
cfe_tbl_api.c:CFE_TBL_Validate - No success path test
cfe_time_tone.c:CFE_TIME_Local1HzISR - Don’t see where this is ever called in unit test

Expected behavior
Unit Tests test all success/error paths

System observed on:
NA code review/audit

Reporter Info
Dan Knutsen
NASA Goddard

@ArielSAdamsNASA
Copy link

ArielSAdamsNASA commented Aug 10, 2021

1. cfe_es_api.c:CFE_ES_DeleteApp - Should test on boundary conditions for AppID

In cFE/modules/es/ut-coverage/es_UT.c

  • Test deleting an app that doesn't exist
  • Does not check if App ID is greater than or less than the min/max values of possible APIDs, but checks that app doesn't exist

2. cfe_es_api.c:CFE_ES_GetAppIDByName - Don’t see where this is ever called in unit test

Called in cFE/modules/es/ut-coverage/es_UT.c

    /* Test CFE_ES_GetAppIDByName error with null AppID pointer and valid name */
    ES_ResetUnitTest();
    UtAssert_INT32_EQ(CFE_ES_GetAppIDByName(NULL, "UT"), CFE_ES_BAD_ARGUMENT);

    /* Test CFE_ES_GetAppIDByName error with valid AppID and NULL name */
    ES_ResetUnitTest();
    ES_UT_SetupSingleAppId(CFE_ES_AppType_EXTERNAL, CFE_ES_AppState_RUNNING, "UT", NULL, NULL);
    UtAssert_INT32_EQ(CFE_ES_GetAppIDByName(&AppId, NULL), CFE_ES_BAD_ARGUMENT);

3. cfe_es_api.c:CFE_ES_ReloadApp - Should test on boundary conditions for AppID

In cFE/modules/es/ut-coverage/es_UT.c

  • Test CFE_ES_ReloadApp with bad AppID argument
  • Test reloading a core app
  • Test reloading an app that is currently not running
  • Test success initiating an app reload
  • Test Reload app: file doesn't exist
  • Does not check if App ID is greater than or less than the min/max values of possible APIDs, but checks if app is CFE_ES_AppType_CORE, CFE_ES_APPID_UNDEFINED, or CFE_ES_AppType_EXTERNAL.

4. cfe_es_api.c:CFE_ES_RestartApp - No success path test

5. cfe_esmempool.c:CFE_ES_GetMemPoolStats - No success path test

6. cfe_sb.h:CFE_SB_MsgId_Equal - Don’t see where this is ever called in unit test

  • Called in cFE/modules/evs/ut-coverage/evs_UT.c
    /* Note implementation initializes both short and long message */
    UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_MSG_Init)), 2);
    UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_SB_TransmitMsg)), 1);
    CFE_UtAssert_TRUE(CFE_SB_MsgId_Equal(MsgData.MsgId, ShortFmtSnapshotData.MsgId));
    CFE_UtAssert_FALSE(CFE_SB_MsgId_Equal(MsgData.MsgId, LongFmtSnapshotData.MsgId));

7. cfe_sb.h:CFE_SB_MsgIdToValue - Don’t see where this is ever called in unit test

  • Called in cFE/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c
    UtPrintf("Set msg to all 0, set msgid to max and verify");
    memset(&msg, 0, sizeof(msg));
    CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
    UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0);
    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID));
    UT_DisplayPkt(&msg, sizeof(msg));
    CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
    UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), CFE_PLATFORM_SB_HIGHEST_VALID_MSGID);

8. cfe_sb.h:CFE_SB_ValueToMsgId - Don’t see where this is ever called in unit test

  • Called in cFE/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c
    UtPrintf("Set ApId msgid bits only and verify");
    CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(TEST_MAX_APID)));
    CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
    UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), MSG_APID_FLAG);
    CFE_UtAssert_SUCCESS(CFE_MSG_GetApId(&msg, &apid));
    UtAssert_INT32_EQ(apid, TEST_MAX_APID);

9. cfe_sb_api.c:CFE_SB_SubscribeFull - There are 3 cases where CFE_SB_BAD_ARGUMENT can get returned…UT should test each one individually. Also this is outside the scope of the argument validation audit, but there are 3 additional return codes not tested in the unit test.

  • Exercise in CFE_SB_Subscribe. Enough for unit test level.

10. cfe_sb_api.c:CFE_SB_SubscribeLocal - Consider subscribing to message with limit greater than CFE_PLATFORM_SB_DEFAULT_MSG_LIMIT

11. cfe_tbl_api.c:CFE_TBL_DumpToBuffer - No success path test

12. cfe_tbl_api.c:CFE_TBL_GetAddresses - Should test on boundary conditions for NumTables

13. cfe_tbl_api.c:CFE_TBL_GetStatus - Don’t see a success path test

14. cfe_tbl_api.c:CFE_TBL_ReleaseAddresses - No success path test. Also, should test on boundary conditions for NumTables

15. cfe_tbl_api.c:CFE_TBL_Update - Don’t see a success path test

16. cfe_tbl_api.c:CFE_TBL_Validate - No success path test

17. cfe_time_tone.c:CFE_TIME_Local1HzISR - Don’t see where this is ever called in unit test

  • Not called in files in the ut-coverage folder

@skliper
Copy link
Contributor

skliper commented Aug 11, 2021

Great work. ID checks against undefined along with a success case are sufficient for testing the "range" so those are good enough. Confirming CFE_TIME_Local1HzISR will get called from the TIME coverage test update issue #473. With this analysis and the remaining missing tests having specific issues, closing as duplicate/OBE.

@skliper skliper closed this as completed Aug 11, 2021
@skliper skliper removed this from the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants