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 #471, improve SB coverage test #1668

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Add test cases to exercise all functions, lines, and branches to the extent reasonably possible. Improves the coverage stats
significantly:

functions 98.9% -> 100%
lines 96.4% -> 99.8%
branches 87.1% -> 94.9%

Fixes #471

Testing performed
Build and run coverage test, check LCOV reports

Expected behavior changes
More complete branch/line coverage

System(s) tested on
Ubuntu

Additional context
Remaining uncovered lines/branches are not possible to be reached due to the way the code is structured, or because it would require an alternate implementation of SBR (note that SB+SBR are currently tested as a single unit, even though they are technically separate modules now). For example, the "direct" SBR implementation cannot have collisions, hence the collision handling in SB cannot be reached. Making stubs for SBR may allow this to be tested. For example this conditional is not reachable with direct mode:

if (Collisions != 0)
{
CFE_EVS_SendEventWithAppID(CFE_SB_HASHCOLLISION_EID, CFE_EVS_EventType_DEBUG, CFE_SB_Global.AppId,
"Msg hash collision: MsgId = 0x%x, collisions = %u",
(unsigned int)CFE_SB_MsgIdToValue(MsgId), (unsigned int)Collisions);
}

Other lines in CFE_SB_AppInit are also not reachable (will report in separate issue ticket).

Additionally, many internal switch statements can only be reached with values for which there is a corresponding case - that is, there is no default case nor is it possible to reach the switch statement with any value other than the listed values. However gcov still reports this as an un-executed branch even though all possible cases are indeed covered.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Add test cases to exercise all functions, lines, and branches to
the extent reasonably possible.  Improves the coverage stats
significantly:

  functions 98.9% -> 100%
  lines 96.4% -> 99.8%
  branches 87.1% -> 94.9%

Remaining uncovered lines/branches are not possible to be reached
due to the way the code is structured, or because it would require
an alternate implementation of SBR (note that SB+SBR are currently
tested as a single unit, even though they are technically separate
modules now).  For example, the "direct" SBR implementation cannot
have collisions, hence the collision handling in SB cannot be
reached.  Making stubs for SBR may allow this to be tested.
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 15, 2021
@skliper
Copy link
Contributor

skliper commented Jul 16, 2021

Could you turn the coverage comments related to SBR/collisions into an issue and mark w/ the coverage tag? That'll help in creating the coverage report/justification.

@jphickey
Copy link
Contributor Author

jphickey commented Jul 16, 2021

Could you turn the coverage comments related to SBR/collisions into an issue and mark w/ the coverage tag? That'll help in creating the coverage report/justification.

I noted this under #1462 and added coverage tag to that existing ticket. We can also write a new issue if you think its easier to track that way.

@astrogeco
Copy link
Contributor

@skliper are we good to merge this one?

@astrogeco astrogeco changed the base branch from main to integration-candidate July 21, 2021 13:52
@astrogeco astrogeco merged commit c71a27e into nasa:integration-candidate Jul 21, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 21, 2021
nasa/cFE#1668, improve SB coverage test

nasa/cFE#1694, correct function name in UT_BSP_Unlock
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 21, 2021
**Combines**

nasa/cFE#1665, v6.8.0-rc1+dev762
nasa/osal#1113, v5.1.0-rc1+dev573

**Includes**

nasa/cFE#1664, remove default .dat extension
nasa/cFE#1660, Change CI to use Test Log.
nasa/cFE#1670, Update API doxygen list
nasa/cFE#1671, update documentation for CFE_ES_GetPoolBufInfo
nasa/cFE#1674, CFE_SB_MsgHdrSize returns size_t
nasa/cFE#1668, improve SB coverage test
nasa/cFE#1694, correct function name in UT_BSP_Unlock

nasa/osal#1106, Add independent OS_rename functional test parameter checks

Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored-by: Alex Campbell <zanzaben@users.noreply.github.com>
Co-authored-by: Oliver Hamburger <oliverhamburger@users.noreply.github.com>
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jul 21, 2021
@astrogeco
Copy link
Contributor

CCB:2021-07-21 APPROVED

@jphickey jphickey deleted the fix-471-sb-coverage branch August 3, 2021 15:19
@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.

Incomplete coverage test for src/sb
3 participants