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

Stub for CFE_ES_RunLoop default return is false, should it be? #927

Closed
asgibson opened this issue Sep 30, 2020 · 3 comments
Closed

Stub for CFE_ES_RunLoop default return is false, should it be? #927

asgibson opened this issue Sep 30, 2020 · 3 comments

Comments

@asgibson
Copy link
Contributor

Describe the bug
CFE_ES_RunLoop uses UT_DEFAULT_IMPL which typically returns CFE_SUCCESS, which happens to also equal 0. CFE_ES_RunLoop returns UT_DEFAULT_IMPL !=0, which causes the default return to be false (0 != 0). Is that the desired default behavior?

To Reproduce
Steps to reproduce the behavior:

  1. Write a unit test that expects CFE_ES_RunLoop to succeed by default.
  2. Run test, see that is not the behavior.

Expected behavior
I had expected 'true' to be the default.

Code snips

bool CFE_ES_RunLoop(uint32 *ExitStatus)
{
UT_Stub_RegisterContext(UT_KEY(CFE_ES_RunLoop), ExitStatus);
return UT_DEFAULT_IMPL(CFE_ES_RunLoop) != 0;

To run a single loop for a unit test, this is required:
UT_SetForceFail(UT_KEY(CFE_ES_RunLoop), true);
UT_SetDeferredRetcode(UT_KEY(CFE_ES_RunLoop), 2, false);

The main reason this requirement does not make sense is UT_SetForceFail makes one think it should fail, not succeed.

System observed on:
RHEL 7.6

Additional context
If this is the desired behavior, close and disregard this issue.

Reporter Info
Alan Gibson NASA GSFC/587

@jphickey
Copy link
Contributor

Yes, to default to false is indeed desired behavior, because in the typical use-case this function is the conditional on a while loop. So if defaulted to true one would have an infinite loop if no return code was configured.

Defaulting to false is just an easy way to keep it simple and avoid an infinite loop if the test case forgets to explicitly configure something. If you want one loop you should be able to just do:

UT_SetDeferredRetcode(UT_KEY(CFE_ES_RunLoop), 1, true)

And nothing else, don't need to bother with "force fail" unless you want many iterations.

@skliper
Copy link
Contributor

skliper commented Sep 30, 2020

Also related to nasa/osal#559 (UT_SetForceFail misnomer).

@skliper skliper removed the bug label Sep 30, 2020
@skliper skliper closed this as completed Sep 30, 2020
@asgibson
Copy link
Contributor Author

The single loop was an example, so yes many iterations will require the force fail.

Where is the behavior for CFE_ES_RunLoop stub defined outside of the code? Do all bool return functions for stubs have this same behavior or just this one because of the loop usage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants