Skip to content

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Fixes #8

Adds a complete example unit test to the Sample Application. Uses the UT assert framework with common stubs provided by CFE and other modules.

Obtains 100% line coverage on the current app implementation.

Testing performed
Build with SIMULATION=native ENABLE_UNIT_TESTS=TRUE

Execute the unit tests per "make test" and confirm correct output
Execute "make lcov" to collect coverage statistics
Verified that the Sample application is included in results and achieves 100% line coverage.

Expected behavior changes
No changes to FSW.

System(s) tested on:
Ubuntu 18.04 LTS 64 bit

Additional context
There is a bug in the application implementation, described in #25. The unit tests actually will fail until this is fixed.

Contributor Info
Joseph Hickey, Vantage Systems, Inc.

Community contributors
You must attach a signed CLA (required for acceptance) or reference one already submitted

@jphickey jphickey force-pushed the fix-8-add-sample-app-tests branch from 4f39222 to 64b5704 Compare November 21, 2019 21:17
@jphickey
Copy link
Contributor Author

NOTE: rebased this commit so it is directly off master for review purposes, rather than as a child of the #23 change. It builds fine independently however the test will not actually pass until both #23 and #25 are merged too.

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.

Is it possible to replace the #include of the *.c file?

@skliper skliper added this to the 1.2.0 milestone Dec 30, 2019
@skliper skliper added the enhancement New feature or request label Dec 30, 2019
@skliper
Copy link
Contributor

skliper commented Jan 22, 2020

CCB 20200122 - Discussed, the issue is accessing private members.

Meeting standard of not #include .c is higher priority than proper scoping (static)
Preference is if the test could get coverage using external methods

@astrogeco astrogeco added the CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. label Feb 11, 2020
@astrogeco astrogeco removed the CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. label Feb 19, 2020
@jphickey jphickey force-pushed the fix-8-add-sample-app-tests branch 3 times, most recently from 0e89595 to c444468 Compare March 13, 2020 17:29
Use the UT Assert framework and stubs provided by other
modules to perform unit testing on the SAMPLE_APP.

This gets 100% line coverage on the current implementation.
@jphickey jphickey force-pushed the fix-8-add-sample-app-tests branch from c444468 to a029470 Compare March 13, 2020 17:38
@jphickey jphickey requested a review from skliper March 13, 2020 17:40
@jphickey
Copy link
Contributor Author

The latest commit rebases this to "master" and removes the wrapper, compiling the FSW file directly as part of coverage test. Should be good to go.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 13, 2020
@astrogeco astrogeco changed the title Add sample app tests Fix #8, Add sample app tests Mar 18, 2020
@astrogeco astrogeco added CCB - 20200318 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 18, 2020
@astrogeco astrogeco dismissed skliper’s stale review March 18, 2020 21:08

Addressed in force push

@astrogeco astrogeco dismissed their stale review March 25, 2020 13:57

We decided to keep the 6.7 comment fix here.

@astrogeco astrogeco changed the base branch from master to integration-candidate March 25, 2020 13:57
@astrogeco astrogeco merged commit a34c02e into integration-candidate Mar 25, 2020
@skliper skliper deleted the fix-8-add-sample-app-tests branch April 2, 2020 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit test example
3 participants