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

UT_SetForceFail is a misnomer #559

Closed
asgibson opened this issue Aug 17, 2020 · 5 comments · Fixed by #646 or #662
Closed

UT_SetForceFail is a misnomer #559

asgibson opened this issue Aug 17, 2020 · 5 comments · Fixed by #646 or #662
Assignees
Labels
bug unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Milestone

Comments

@asgibson
Copy link
Contributor

Describe the bug
UT_SetForceFail assumes that the value given, that is being set as return value, is a fail condition. There may be sometimes that a function returns more than 1 value that is not considered a fail.

To Reproduce
Use in a unit test where there is a stubbed function with more than 1 "successful" return value.

Expected behavior
Change to something like UT_SetForcedReturnValue to make it more generic.

Code snips
N/A

System observed on:
RHEL 7.6

Additional context
Not a required or debilitating situation, but a name change may be make the function's effects more clear.

Reporter Info
Alan Gibson NASA-GSFC/587

@asgibson asgibson added the unit-test Tickets related to the OSAL unit testing (functional and/or coverage) label Aug 17, 2020
@jphickey
Copy link
Contributor

I concur on that this is not an ideal name -- I was actually thinking this for quite some time but didn't think changing the name was worth breaking test code over it. But it shouldn't be too hard to apply the deprecation procedure if folks agree to change the name.

How about UT_SetDefaultReturnValue() - I think better indicates the fact that "deferred" values and hook return values will take precedence, and that this value gets used when no other return value is identified.

Should discuss in CCB - fairly easy to do, just need other stakeholders to concur on this one.

@jphickey jphickey added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) question labels Aug 18, 2020
@asgibson
Copy link
Contributor Author

I agree with your call on the name.

Thanks.

@astrogeco
Copy link
Contributor

astrogeco commented Aug 19, 2020

@asgibson will you join today's CCB or will @jphickey "represent" you on this?

@asgibson
Copy link
Contributor Author

@astrogeco I will not be joining today. I believe @jphickey and I are agreed on the name change. Even if you wish to change the name to something else, as long as the method name is descriptive of what it can accomplish, I am flexible on the actual words used.

@astrogeco
Copy link
Contributor

astrogeco commented Aug 19, 2020

CCB 2020-08-19 APPROVED concept

@astrogeco astrogeco removed the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 19, 2020
@skliper skliper removed the question label Sep 1, 2020
zanzaben added a commit to zanzaben/osal that referenced this issue Nov 12, 2020
zanzaben added a commit to zanzaben/osal that referenced this issue Nov 13, 2020
zanzaben added a commit to zanzaben/osal that referenced this issue Nov 23, 2020
@astrogeco astrogeco added this to the 6.0.0 milestone Dec 1, 2020
@astrogeco astrogeco added the bug label Dec 1, 2020
astrogeco added a commit that referenced this issue Dec 1, 2020
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants