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 #832 Override default stub logic #839

Closed
wants to merge 6 commits into from

Conversation

asgibson
Copy link
Contributor

@asgibson asgibson commented Mar 5, 2021

Describe the contribution
This is a new set hook function that allows a test designer to bypass the remaining default stub implementation after the hook is called. Test designers can do this as needed, but current tests will be unaffected because it must be set in a test to be used. Current tests will never set the override and their expectations of the stub code running after the hook are still valid. Stubs will need updated to make the check for the override, but that is all that will need to change in them.

Testing performed
I have written 7 unit type tests to show that the behavior without the override set continues to be the same. Each of these tests is doubled with a control test, for a total of 14 tests. The tests are run with no changes, with cFS commit 5ca472a and using my working version of cfs_cf (unit-test-cf-3.0), this shows that the results of each pair of tests are the same; which is the expectation as they are duplicates. The repo for cfs_cf is given the new UT_SetHookOverrideStubFunction instead of UT_SetHookFunction in 7 of the tests with each duplicate test left untouched as the control test. A build is attempted showing that the function does not exist. The repo, osal, is updated with the changes implementing the override and the repo, cfe, gets a two stubs updated to allow the bypass (CFE_MSG_GetSize, CFE_MSG_GetMsgId). After a successful full rebuild, the 14 tests are run again. The results confirm the changes have provided the desired behavior. While the control tests all still receive the same results as the no updates test run, all of the override tests have their expected changes.

Details of the specific tests:

1 Test_GetSize_Returns_1 & Test_GetSize_Returns_1_Control

  • Initial testing: both receive TSF - as expected, forced return value >= 0, no data buffer set
  • Override Control: receives TSF - as expected, forced return value >= 0, no data buffer set
  • Override Updated: No TSF, No extra pass == No stub code after hook - as expected Override enabled, no data buffer set

2 Test_GetSize_wUTSDB_Returns_1 & Test_GetSize_wUTSDB_Returns_1_Control

  • Initial testing: both receive extra PASS - as expected, forced return value >= 0, yes data buffer set
  • Override Control: receive extra PASS - as expected, forced return value >= 0, yes data buffer set
  • Override Updated: No TSF, No extra pass == No stub code after hook - as expected Override enabled, yes data buffer set

3 Test_GetSize_Returns_0 & Test_GetSize_Returns_0_Control

  • For brevity, these results are the same as 1, I used 1, 0, -1 in my tests because it is the cross over point, >= 0

4 Test_GetSize_wUTSDB_Returns_0 & Test_GetSize_wUTSDB_Returns_0_Control

  • Same as 2

5 Test_GetSize_Returns_neg1 & Test_GetSize_Returns_neg1_Control
6 Test_GetSize_wUTSDB_Returns_neg1 & Test_GetSize_wUTSDB_Returns_neg1_Control

  • all grouped together because all are same, no TSF, no extra PASS, forced return < 0, code does not run anyway

7 Test_GetSize_OtherKey &Test_GetSize_OtherKey_Control

  • Initial testing: both receive 3 TSF - as expected, forced return value >= 0, 2 to CFE_MSG_GetSize, 1 to CFE_MSG_GetMsgId
  • Override Control: same as Initial testing - as expected
  • Override Updated: 1 TSF, no extra PASS - as expected, both CFE_MSG_GetSize overridden, but CFE_MSG_GetMsgId is not
    OverrideExperiment.txt
    do-no-harm.txt

Expected behavior changes
Test designers will have the option to use UT_SetHookOverrideStubFunction instead of UT_SetHookFunction.
When this is selected any stub behavior that appears after the hook function is called can be bypassed; as long as the stub checks for the override and allows it.
No other behavior will be changed, as it was a requirement that the change have no impact to current operations.

System(s) tested on

Additional context
The override control tests show there is no behavioral changes unless the new override function is used. The override updated tests show that the desired outcome is achieved in each instance.

Contributor Info - All information REQUIRED for consideration of pull request
Alan Gibson NASA/GSFC 587

asgibson added 6 commits March 5, 2021 06:20
Added function to check for override.
Added set hook functions to provide override.
Created new entry type for stub override.
Added private function for set hook overrides to both use
for setting the override stub entry.
Implemented both set hook override functions (standard and Va)
Implemented public access to override check function.
Took out Va version as a test subject could not be located
May be readded later if there is a need
Left UT_DoSetOverride in the code to add Va back in when needed
Removed UT_DoSetOverride.
Due to the Va function not being adopted, there is only 1 use of this
code and it does not require encapsulation.
Removed in defference to not adding more functions.
@skliper skliper requested a review from jphickey April 2, 2021 18:22
@astrogeco astrogeco added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Apr 11, 2021
@skliper skliper added CCB:Ignore Incomplete Pull Request with open actions. and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 14, 2021
@skliper
Copy link
Contributor

skliper commented Apr 14, 2021

@jphickey is extending on this concept with auto-generation of stubs that handles return codes correctly, holding off review for now

@asgibson
Copy link
Contributor Author

Fixed via other update, #966

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Ignore Incomplete Pull Request with open actions. duplicate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants