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.

Sign up for GitHub

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

Jump to bottom

Fix #1677, Add Message API Functional Test #1745

Merged
merged 1 commit into from
Aug 16, 2021
Merged

Fix #1677, Add Message API Functional Test #1745

merged 1 commit into from
Aug 16, 2021

Conversation

paulober
Copy link
Contributor

@paulober paulober commented Aug 1, 2021

Describe the contribution

Contributor Info - All information REQUIRED for consideration of pull request
Paul Oberosler, Individual

skliper
skliper reviewed Aug 2, 2021
View reviewed changes
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Good work! Suggestions below.

modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
@skliper skliper added this to the 7.0.0 milestone Aug 2, 2021
@skliper skliper mentioned this pull request Aug 2, 2021
Closed
zanzaben
zanzaben reviewed Aug 3, 2021
View reviewed changes
Copy link
Contributor

@zanzaben zanzaben left a comment

Choose a reason for hiding this comment

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

These should really be broken up into multiple tests instead of testing all 32 functions in one. It makes it easier to read if you try to group them up to around 4-6 functions per test.

modules/cfe_testcase/src/msg_api_test.c Outdated Show resolved Hide resolved
@paulober paulober force-pushed the fix-1677-functional-tests-for-message-apis branch 8 times, most recently from 09bba8a to 0194a10 Compare August 6, 2021 16:17
@paulober paulober requested a review from skliper August 7, 2021 09:59
@astrogeco
Copy link
Contributor

@PavLL looks like one of the tests are failing to build

OS_ModuleLoad_Impl():111:Error loading shared library: ./cf/cfe_testcase.so: undefined symbol: CFE_MSG_GetSystem
1980-012-14:03:28.50122 CFE_ES_LoadModule: Could not load file:/cf/cfe_testcase.so. EC = 0xFFFFFFFF
EVS Port1 66/1/CFE_ES 26: Failed to start CFE_TEST_APP from /cf/cfe_testcase.so, RC = 0xC8000005
grep: cf/cfe_test.tmp: No such file or directory
Error: Process completed with exit code 2.

@skliper
Copy link
Contributor

skliper commented Aug 9, 2021

Oh, that API is only available if the system is configured to use "extended headers"... could just drop from this test for now.

@paulober paulober force-pushed the fix-1677-functional-tests-for-message-apis branch from 808145e to ee0104e Compare August 9, 2021 23:46
@skliper
Copy link
Contributor

skliper commented Aug 10, 2021

So for the default headers the following APIs are not available:
CFE_MSG_GetEDSVersion/CFE_MSG_SetEDSVersion
CFE_MSG_GetEndian/CFE_MSG_SetEndian
CFE_MSG_GetPlaybackFlag/CFE_MSG_SetPlaybackFlag
CFE_MSG_GetSybsystem/CFE_MSG_SetSubsystem
CFE_MSG_GetSystem/CFE_MSG_SetSystem

@astrogeco astrogeco added community Community contribution, YAY! unit-test CCB:Ready Ready for discussion at the Configuration Control Board (CCB) conflicts and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Aug 11, 2021
@astrogeco
Copy link
Contributor

CCB:2021-08-11 APPROVED with CHANGES

  • Group tests into multiple subfunctions

@paulober paulober force-pushed the fix-1677-functional-tests-for-message-apis branch 7 times, most recently from 991524b to 7428160 Compare August 14, 2021 14:03
@paulober paulober force-pushed the fix-1677-functional-tests-for-message-apis branch 5 times, most recently from 1aaa717 to 9e91caf Compare August 14, 2021 15:39
@paulober paulober force-pushed the fix-1677-functional-tests-for-message-apis branch from 9e91caf to 1dc0aea Compare August 14, 2021 15:45
@paulober paulober requested a review from zanzaben August 14, 2021 19:41
@skliper
Copy link
Contributor

skliper commented Aug 16, 2021

Hi @PavLL - good work! Could you hold off on changes now so we can do a couple final tweaks and get this merged? We can always loop back once it's in if there's any other updates needed.

@astrogeco astrogeco changed the base branch from main to integration-candidate August 16, 2021 14:50
@astrogeco astrogeco merged commit b17a86a into nasa:integration-candidate Aug 16, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Aug 16, 2021
@astrogeco
Add nasa/cFE#1745, Add Message Api Functional Test
d21741c
This was referenced Aug 16, 2021
Merged
Merged
@zanzaben zanzaben mentioned this pull request Aug 16, 2021
Closed
@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label Aug 16, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Aug 17, 2021
@astrogeco @skliper
@jphickey @zanzaben @nmullane @PavLL
IC:2021-08-17, cFE v6.8.0-rc1+dev
c2119ea 
**Combines**

nasa/cFE#1808, v6.8.0-rc1+dev873

**Includes**

- nasa/cFE#1790, Port "CFE_UtAssert_SuccessCheck" and related macros from coverage test to functional test #1784, add CFE assert macros to functional test
- nasa/cFE#1779, Adds invalid id syslog to for CFE_ES_DeleteApp and CFE_ES_ReloadApp and verifies required reporting
- nasa/cFE#1785, Stop memory leak & add cds size test.
- nasa/cFE#1765, Mark read only inputs as const
- nasa/cFE#1804, Check resource ID idx is less than max
- nasa/cFE#1778, Add ES Application Behavior Functional Tests
- nasa/cFE#1801, Update CFE_ES_RunLoop documentation
- nasa/cFE#1745, Add Message Api Functional Test
- nasa/cFE#1798, update in/out status and nonnull/nonzero tags
- nasa/cFE#1783, Add External Time Source Functional Tests
- nasa/cFE#1800, Add Perf API functional tests

Co-authored-by: Jacob Hageman <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
Co-authored-by: Alex Campbell <[email protected]>
Co-authored-by: Niall Mullane <[email protected]>
Co-authored-by: Paul <[email protected]>
astrogeco added a commit to nasa/cFS that referenced this pull request Aug 17, 2021
@astrogeco @skliper
@jphickey @zanzaben @nmullane @PavLL
IC:2021-08-17, cFE v6.8.0-rc1+dev873
94721a6 
**Combines**

nasa/cFE#1808, v6.8.0-rc1+dev873

**Includes**

- nasa/cFE#1790, Port "CFE_UtAssert_SuccessCheck" and related macros from coverage test to functional test #1784, add CFE assert macros to functional test
- nasa/cFE#1779, Adds invalid id syslog to for CFE_ES_DeleteApp and CFE_ES_ReloadApp and verifies required reporting
- nasa/cFE#1785, Stop memory leak & add cds size test.
- nasa/cFE#1765, Mark read only inputs as const
- nasa/cFE#1804, Check resource ID idx is less than max
- nasa/cFE#1778, Add ES Application Behavior Functional Tests
- nasa/cFE#1801, Update CFE_ES_RunLoop documentation
- nasa/cFE#1745, Add Message Api Functional Test
- nasa/cFE#1798, update in/out status and nonnull/nonzero tags
- nasa/cFE#1783, Add External Time Source Functional Tests
- nasa/cFE#1800, Add Perf API functional tests

Co-authored-by: Jacob Hageman <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
Co-authored-by: Alex Campbell <[email protected]>
Co-authored-by: Niall Mullane <[email protected]>
Co-authored-by: Paul <[email protected]>
@paulober paulober deleted the fix-1677-functional-tests-for-message-apis branch August 28, 2021 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zanzaben zanzaben zanzaben approved these changes

@skliper skliper Awaiting requested review from skliper

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB community Community contribution, YAY! conflicts unit-test
Projects
None yet
Milestone
Caelum
Development

Successfully merging this pull request may close these issues.

Add functional tests for cFE Message header APIs
4 participants
@paulober @astrogeco @skliper @zanzaben