Unit test macros and example use with SB

[CDKnightNASA]

Describe the contribution
sample macro-ification of UT code for CCB consideration.
Partial implementation of #491
Fix #418

Testing performed
made SB unit tests and confirmed all passed.

Expected behavior changes
simplified UT code

System(s) tested on
Debian Linux

Contributor Info - All information REQUIRED for consideration of pull request
[email protected]

EDIT: Added this fixes #418

[CDKnightNASA]

Initial skim through -- I like the fact that it gets rid of so much redundant code.

I do have some requests though - Could we make the macro use UT Assert directly, rather than doing an sprintf and UT_Text() call? I implemented a similar macro on other apps, and an example can be seen in the sample_lib UT code here:

https://github.com/nasa/sample_lib/blob/09d5ff7f8cb46d98ee1f88b107b9761147d620d3/unit-test/coveragetest/sample_lib_coveragetest_common.h#L45-L59

Basically the macro is to call a function, and assert on the int32 return value. The macro may even be part of UT assert itself so it doesn't need to be re-implemented, although this does allow each component to customize it so that might be useful too.

I've committed changes that use UtAssertEx() calls instead of my code. :D

[astrogeco]

CCB 20200212 - Approved concept, @CDKnightNASA will go forward with implementation and update macros for reporting values and prefixes etc.

[CDKnightNASA]

Note that the UtAssertEx function always generates an entry in the stdout, so the sb_UT would generate ~1,800 "tests" instead of the 182 of the trunk. I assume the latter is the "correct" behavior?

[jphickey]

I actually prefer to see all the test cases.

The only exception is if it is a sanity check of the test case itself i.e. like a test buffer being of sufficient size or otherwise using UT assert API to check for fundamental flaws in the test case itself as opposed to the FSW function outputs. (In this case the "pass" messages really are just clutter).

But in any case where it is checking the output of the FSW code, it should make an entry in the log. The count doesn't matter. Note you can also separately suppress "pass" output and show only failures if the log file gets too big.

[CDKnightNASA]

A lot of checks in sb_UT are checking of setup steps to make sure the setup is correct...So if you want to see all of the setup steps in the log...?

A good example is Test_CreatePipe_MaxPipes() which calls CreatePipe CFE_PLATFORM_SB_MAX_PIPES + 1 times, and reports each CreatePipe, even though we only really care about the last one and only really care if it (or any prev. calls) fails.

[skliper]

I think we discussed this? If not, my suggestion is just keep a status or check that they all create and report success or failure once. No benefit to the reporting being inside the loop.

Oh, and break up the success and failure calls. Do the successful creation loop first, report if it passed or failed, then do the +1.

edit added 2nd statement.

[CDKnightNASA]

I actually prefer to see all the test cases.

The only exception is if it is a sanity check of the test case itself i.e. like a test buffer being of sufficient size or otherwise using UT assert API to check for fundamental flaws in the test case itself as opposed to the FSW function outputs. (In this case the "pass" messages really are just clutter).

But in any case where it is checking the output of the FSW code, it should make an entry in the log. The count doesn't matter. Note you can also separately suppress "pass" output and show only failures if the log file gets too big.

Nice thing is with macros, we could have a single #define to enable/disable the SETUP/TEARDOWN/etc. reporting.

[CDKnightNASA]

I've taken a fresh crack at this, modeling it more like the OSAL unit test macros, to make them semantically clear as to what the tests are doing. I have some questions for the CCB:

1. func ok or some compilers not support?
2. One test per fn or multiple tests? All test fn's call UT_Report() only at the end so reporting is inconsistent.
3. Should assert() report on success or only on failure (as it currently does)?
4. On a failed "setup" function call, should test drop through?

[astrogeco]

CCB 20200422 - Discussed. House them in ut_support so they can be used in all subsystems. Will benefit from a targeted review.

[CDKnightNASA]

FYI I've written up a page in the wiki (that could be incorporated into the git repo):

https://github.com/nasa/cFE/wiki/Unit-Test-Coding-Standards

[CDKnightNASA]

some more updates, squashed commit

[skliper]

I'd like the CCB to consider merging in the ut_support.c/h changes whether or not the sb_UT.c changes are accepted, so that future unit test code can use the new macros. I can make it a separate pull request, if folks would rather.

Separate may be easier? Although I don't foresee any issues with accepting sb_UT.c. I would like to get the support changes in quickly such that we can start updating the rest as soon as the core code updates are done.

[skliper]

Note I just fought with es_UT for a while and almost lost, can't wait to update it.

[skliper]

Great! Want to run just sb_UT and drop the test log on here?

o.txt

I thought it was going to output the expected and actual values? I may have missed it...

[CDKnightNASA]

Great! Want to run just sb_UT and drop the test log on here?

o.txt

I thought it was going to output the expected and actual values? I may have missed it...

Only on a failed test.

[astrogeco]

CCB 2020-04-29 : APPROVED

[astrogeco]

@CDKnightNASA can you resolve the conflicts?

[CDKnightNASA]

working this, lots of conflicts. :/

[CDKnightNASA]

conflicts resolved

[CDKnightNASA]

I have some other goofs to commit, plus some cleanup to use the new macros, will push them within the hour.

[skliper]

@CDKnightNASA once merged to IC, it's easier if you open a new pr

[CDKnightNASA]

@CDKnightNASA once merged to IC, it's easier if you open a new pr

Ok, sorry I broke some of the MID abstraction in sb_UT...Given how many other changes in PSP, OSAL, etc. it is difficult for me to test my changes anyways until everything settles back into all of the master branches. :o