-
Notifications
You must be signed in to change notification settings - Fork 200
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
Further simplify SB unit tests #734
Comments
There is also no need for separate "start"/"end" reporting, etc. The UT assert already has a way to identify the start of each test routine/block as well as designated setup and teardown phases. The SB test functions should follow this pattern rather than making their own. |
Entertaining tidbit of information. We have a one-line abstraction function here: cFE/fsw/cfe-core/src/sb/cfe_sb_util.c Line 186 in d217ca3
The unit test for that function is here, weighing in at 124 lines: cFE/fsw/cfe-core/unit-test/sb_UT.c Lines 4766 to 4890 in d217ca3
But what makes it really interesting is that for some reason this is feeling the need to call this one line function with every possible uint16 value for every possible header type including header types that SB does not support even though its a one line function that does not even look at header type. In all, there are 524,288 "assert" actions to cover this one line function. because it also checks the packet content itself in addition to the return code for every value. Edit: In all fairness, it actually covers the "get" and "set" so there are two lines of FSW code being tested by these 500,000 asserts. |
Makes the SB unit test closer to recommended UT assert patterns Do not keep a separate "TestStat" state variable outside UT assert. Do not report separate status messages from the asserts. Use UT assert. Do not "reset" in the middle of a test routine, split into separate test routines where this is done. No need for "START" and "STARTBLOCK" or "ENDBLOCK". UT assert has messages for these test actions. Each block should be a separate test routine and then these are unnecessary.
Fix #734, continue cleanup of SB unit test
Is your feature request related to a problem? Please describe.
Recently the software bus unit tests were updated to employ macros for common code bits, but this only amplifies the fact that the SB unit tests don't follow the typical UT assert model.
In particular, they "collect" a bunch of conditions together and then assert at the end that all the conditions where true. There is no reason to do this deferred reporting with UT assert, and it only serves to obfuscate the true failure because the UT assert failure message line number can be quite different than what actually failed.
Describe the solution you'd like
Now that the macros/wrappers are in place, rather than having them only mimic the old test logic, have them implement the preferred UT assert patterns.
TestStat
global variable. Do not keep global state outside UT assert.VERBOSE
compile-time option. The UT assert has a runtime verbosity flag, just callUtDebug
and the message will only be printed when set to verbose mode.Additional context
This really just the next step along the path to using UT assert as it was intended, and removing the "compatibility crutches" that were put into place because CFE (and SB in particular) didn't employ the same test patterns as other apps/modules.
Requester Info
Joseph Hickey, Vantage Systems, Inc.
The text was updated successfully, but these errors were encountered: