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

CF method CF_Assert call branches cannot be tested during unit testing #32

Closed
jphickey opened this issue Nov 23, 2021 · 2 comments · Fixed by #153
Closed

CF method CF_Assert call branches cannot be tested during unit testing #32

jphickey opened this issue Nov 23, 2021 · 2 comments · Fixed by #153
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1733] CF method CF_Assert call branches cannot be tested during unit testing
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Wed Sep 22 16:31:48 2021

Original Description:
Calls to CF_Assert() cannot be stubbed by unit testing.
The reasons:

  1. CF_Assert uses assert which will kill the running process and subsequently the unit test runner when called by code under test.
  2. CF_Assert can be stubbed
        a. The stub cannot stop the return to the code under test.
        b. The value required to cover the branch will cause a segfault upon return -- as the intent of CF_Assert is to kill the app before it takes cFS with it.
        c. There is currently no allowable way to have these branches tested during automated runs.
        d. It is possible to run ad hoc tests to show the assert occurs, but the unit test will stop and not return to the original test code as the automated tests do.
@jphickey
Copy link
Contributor Author

Imported comment by internal user on Wed Nov 17 10:31:54 2021

Many of the assert statements in CF are not necessary and may be candidates for outright removal. For example, inCF_CFDP_R2_SubstateSendFin.

sret = CF_CFDP_SendFin(t, t->state_data.r.r2.dc, t->state_data.r.r2.fs, t->history->cc);

CF_Assert(sret!=CF_SEND_ERROR); /* CF_CFDP_SendFin does not return CF_SEND_ERROR */

What value does it add to assert that a local function did not return the value it does not return? In reality it only matters if it was CF_SEND_SUCCESS or not. Assertions like this unnecessary clutter in the code.

@jphickey
Copy link
Contributor Author

jphickey commented Jan 6, 2022

One possible solution here is to convert the assert statements into DEBUG asserts, and default to compiling them out. This way the information is still preserved in the code (i.e. that the value must be within some range or not NULL, etc) but these are not flagged as branches that are not covered in the tests. This way a user could still turn them back on if they want to.

@jphickey jphickey self-assigned this Jan 6, 2022
jphickey added a commit to jphickey/CF that referenced this issue Jan 6, 2022
Changes CF_Assert to be opt-in rather than opt-out, so that under normal
verification and validation the asserts will _not_ be included, but they
can still be added back during development, if desired.

They mainly exist as notes to developers as to what is supposed to be true,
once debugged it is impossible to get these conditions, by definition.

Also removes one channel calculation that was only for assert.  Note that
the same condition is asserted later, so it was redundant anyway.
astrogeco added a commit that referenced this issue Jan 12, 2022
@skliper skliper added this to the Draco milestone Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants