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

Fix #808, ES API functional tests #1251

Conversation

zanzaben
Copy link
Contributor

@zanzaben zanzaben commented Mar 23, 2021

Describe the contribution
Fixes #808
New tests for the ES Info API's

Testing performed
This is the test

Expected behavior changes
none

Additional Context
Dependant on #1268

Addresses are only valid on VxWorks system so the tests for when that is true have not be run and verified.

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC

@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch 3 times, most recently from fba12f9 to d73363d Compare March 29, 2021 19:07
@zanzaben zanzaben marked this pull request as ready for review March 29, 2021 19:07
@zanzaben zanzaben changed the title WIP Fix808 es api functional tests Fix808 es api functional tests Mar 29, 2021
@zanzaben zanzaben changed the base branch from main to integration-candidate March 29, 2021 19:07
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch 2 times, most recently from 40b86f9 to 8598638 Compare March 29, 2021 19:12
@zanzaben zanzaben added CCB:Ready Ready for discussion at the Configuration Control Board (CCB) dependency labels Mar 29, 2021
@astrogeco astrogeco changed the title Fix808 es api functional tests Fix #808, ES API functional tests Mar 29, 2021
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch 2 times, most recently from ee17610 to 241aa93 Compare March 31, 2021 12:53
@astrogeco astrogeco changed the base branch from integration-candidate to main March 31, 2021 13:16
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from 241aa93 to 36498e2 Compare March 31, 2021 14:37
@zanzaben zanzaben changed the base branch from main to integration-candidate March 31, 2021 15:51
@astrogeco
Copy link
Contributor

CCB:2021-03-31 APPROVED

  • Is there a #define to use in es_info_test.c:50?
  • Assert on addresses, not sure if "zero" is invalid
  • need to test on vxworks
  • Note on file names: use the same name but append "coverage_test" as a prefix. See osal example
  • Add a comment in cmake to specify that there is a convention
  • Add an info line that specifies which API is being tested. There's a difference between using an API and testing it

@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch 2 times, most recently from 3eec2b8 to f406692 Compare April 1, 2021 17:58
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled this to test it in conjunction with what I will propose for #1266, but found it does not build when using strict ID types.

CFE/CFS developers should ideally all set this option:

set(MISSION_RESOURCEID_MODE STRICT)

in your local global_build_options.cmake file. It isn't set that way by default in order to be compatible for external users that may have existing code, but all framework code should build with this strict option set.

@@ -87,6 +87,9 @@ void UT_BSP_DoText(uint8 MessageType, const char *OutputMessage)
Prefix = "N/A";
break;
case UTASSERT_CASETYPE_BEGIN:
CFE_ES_WriteToSysLog("\n");
CFE_ES_WriteToSysLog("\n");
CFE_ES_WriteToSysLog("\n"); /* add a bit of extra whitespace between tests */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be left out of this PR ideally - we can do something better in #1266

modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from f406692 to 3127a90 Compare April 1, 2021 20:51
@astrogeco astrogeco added IC:2021-04-06 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Apr 2, 2021
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from 07127ad to 69c31d2 Compare April 5, 2021 15:59
jphickey added a commit to zanzaben/cFE that referenced this pull request Apr 6, 2021
zanzaben added a commit to zanzaben/cFE that referenced this pull request Apr 6, 2021
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from a9d75b8 to d456d86 Compare April 6, 2021 13:28
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from d456d86 to de26ba4 Compare April 6, 2021 17:04
@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label Apr 7, 2021
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from de26ba4 to bc963b8 Compare April 7, 2021 17:22
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked again, just a couple more minor updates ... it's close!

modules/cfe_testcase/src/cfe_test.h Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/es_info_test.c Outdated Show resolved Hide resolved
@zanzaben zanzaben force-pushed the fix808_ES_API_Functional_Tests branch from bc963b8 to e8a43be Compare April 7, 2021 20:21
@astrogeco astrogeco merged commit 67ee18c into nasa:integration-candidate Apr 8, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Apr 8, 2021
ES API functional tests
astrogeco added a commit to nasa/cFS that referenced this pull request Apr 8, 2021
Combines:

nasa/cFE#1284
nasa/osal#951
nasa/PSP#289

Including:

nasa/cFE#1251 - ES Info API Functional test

nasa/osal#931 - Scrub include guards
nasa/osal#937 - Switch to use CLOCK_REALTIME
nasa/osal#938 - specify shell name in ShellOutputToFile

nasa/PSP#286 - use OSAL timebase for CFE timers
nasa/PSP#282 - modularize the ram, port, and eenasa/psp#om access
nasa/PSP#285 - add psp module to implement timebase
@zanzaben zanzaben deleted the fix808_ES_API_Functional_Tests branch May 21, 2021 16:15
@skliper skliper added this to 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
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add cFE ES Information API functional tests
4 participants