nasa / osal

The Core Flight System (cFS) Operating System Abstraction Layer (OSAL)
Apache License 2.0
545 stars 213 forks source link

OSAL should use UT framework similar to that of CFE #40

Closed skliper closed 4 years ago

skliper commented 4 years ago

With trac ticket #29 (and related #30) now implemented, the OSAL tests can be used as one piece of a build verification tool suite.

However, the implementation is currently very basic - it does not use any real testing framework, it simply counts errors using a global integer added to each test.

CFE has a more sophisticated UT framework consisting of the following functions:

OSAL could benefit from using the same framework to run its tests. Most importantly, using the common "UT_Report()" API ensures that any errors that occur will be counted and logged in a consistent way. This is particularly important for automated tests, as a simple "grep" command can reliably find failures within log files containing thousands of test cases.

skliper commented 4 years ago

Imported from trac issue 17. Created by jphickey on 2015-02-09T11:06:56, last modified: 2015-11-20T16:22:16

skliper commented 4 years ago

Trac comment by jphickey on 2015-02-09 12:23:41:

== More info ==

In addition to simply making the OSAL tests cleaner and more consistent, this actually has real benefits to the build management as well. Since CFE and OSAL are separate repos (which is logical), it also means that any change to an OSAL header file may need a corresponding change to the CFE code. For most application code this is not an issue; as long as the change is compatible, the compiler does the "right thing" and all code will still work.

The real build problem, however, occurs in the "ut_osapi_stubs.c" file within the CFE build. Unlike normal app code, this file actually //implements// stub versions of OSAL calls, so the compiler is very picky about the prototypes being exactly identical here. To summarize the problem:

This means a lot of potential pitfalls going forward. as it becomes really easy to end up with incompatible sources as change sets are mixed and matched. There is a corresponding ticket on the CFE side for the same issue: [https://babelfish.arc.nasa.gov/trac/cfs_cfe/ticket/32]

== Plan ==

  1. Migrate "ut_osapi_stubs.c" to the OSAL repo so that the implementation will always change with the prototypes in the same change set. (It will likely need to coexist in both places for a transitional period)

  2. Migrate implementation of common UT functions such as UT_Report(), UT_SetRtnCode() etc to a separate UT library.

  3. Migrate implementation of the UT_Text() output function to the OSAL UT BSP- this can be the existing pc-linux-ut bsp or a target-specific UT bsp. Note that this implementation only needs to be in the "ut" BSP, not the normal runtime.

  4. Provide the UT library + BSP as a platform for the CFE unit tests to execute.

Additional benefits: moving code into the OSAL BSP should also remove the need for a lot of the {{{#ifdef / #endif}}} blocks currently in the CFE unit tests, namely the alternate implementation for the ARINC653 environment which does not use stdio printf(). This will clean all of that up by putting the special code into the BSP which is unique to that platform. The CFE unit tests could then be easily built and run on any plaftorm with an OSAL "ut" bsp implemented. The current #ifdefs (e.g. {{{#ifdef CFE_LINUX}}}) will not scale to many platforms - it appears that only Linux and the ARINC653 environment are supported.

skliper commented 4 years ago

Trac comment by jphickey on 2015-02-09 12:26:43:

Any comments/suggestions on the above ideas are welcome. This is not something I've implemented (yet) but as long as we have some agreement on the benefits here I would be happy to implement this - should not take too long.

skliper commented 4 years ago

Trac comment by glimes on 2015-02-10 12:04:03:

Joe, I've nosed into this territory a couple of times.

The transition plan looks sensible.

I'd like to see a clearer demarcation between API calls manipulating stubs and mock systems, from the API calls that report the results of unit testing: the former are intrinsically specific to the system being tested, while the latter may be kept stable from project to project, which means we could deploy a common UT reporting library across the board.

I have been able to integrate CFE unit tests into a Bamboo build, so moving OSAL toward that API sounds like a good idea to me.

Special constraints are that doing this requires organizing the supporting text for a failed condition along with the indication that the condition failed, all presented as a single blob of text into the output; this also has to identify the associated test case.

''Side note'': Bamboo notes a testsuite name and testcase name for each failure, and tracks everything based on the name tuple, so the vertical depth of the heirarchy available for the entire collection of tests run by a build is uncomfortable, but seems to be acceptable for the CFE unit tests. The actual Bamboo presentation of those results is over on our ITAR server, and linking from here would be futile, rather than being a helpful example of an implementation. I'll figure out some way to pull this example out into the light.

skliper commented 4 years ago

Trac comment by jphickey on 2015-02-10 13:06:38:

Thanks for the comments. I think this will achieve some of what you are looking for in terms of isolation between the pass/fail reporting calls and the stub functions.

I am putting together a header file and C file that reflect the ideas I'm thinking of here. None of the UT code has been modified to use it yet, but it could act as a preview of sorts to show how the API would look/work.

This mostly looks like the existing API so there shouldn't be any major concerns, the major difference being that it shifts some logic into a UT-specific BSP. One of those things it shifts is how the specific pass/fail messages are formatted, so the BSP is now in complete control of how these look and what they contain, whether they go to a terminal or log file, etc.

So when the automatic integration build is done it can link against a UT BSP that outputs everything to a log file that is nicely formatted to feed into the report generator.

skliper commented 4 years ago

Trac comment by jphickey on 2015-02-25 21:49:57:

Pushed branch "trac-17-osal_basic_ut_framework" containing basic unit test API in OSAL.

Commit [changeset:794cb07]

This modification adds a "ut_framework" library to the OSAL build which implements the core functions of the CFE unit test framework such that it can be used to test any application. It also aims to be a little cleaner and easier to work with, with most stub functions being easily implemented by a macro.

Note 1 - this branch is currently based off my previous ic-merge-jph-all branch. This can be cherry-picked and moved to the development branch when the current IC branch is finalized. It has been pushed here for now so it can be seen/reviewed.

Note 2 - The basic functions added here have been tested individually, but none of the existing unit tests have been modified (yet) to actually use this. Once a good unit test baseline is available, unit tests can then be modified/cleaned up and this would be a good time to transition.

skliper commented 4 years ago

Trac comment by glimes on 2015-03-02 18:28:16:

I found myself looking for usage examples. Got any handy?

skliper commented 4 years ago

Trac comment by jphickey on 2015-03-03 13:55:43:

As an example, I pushed branch "trac-17-port_osal_core_test", commit [changeset:a9ab7bb]

This shows an example of how the "osal-core-test" would be implemented using the basic framework.

FYI- the original commit (trac-17-osal_basic_ut_framework) was amended to change prototypes and add some ease-of-use macros.

skliper commented 4 years ago

Trac comment by jphickey on 2015-03-18 23:08:08:

This is ready for review and integration.

The complete work under this trac ticket is implemented as three separate commits:

[changeset:1e1aa1ac] - This implements the basic UT framework [changeset:c01e08e4] - This ports the OSAL "core" test to use the framework. Only one test is ported at this point, pending approval from the CCB. The objective here is to prove that the UT framework implemented above is functioning correctly. If agreement is reached that this is the right thing then the rest of the tests can be ported. [changeset:7cdbe6e0] - This implements stub functions of all the OSAL functions. This is intended to replace the OSAL stub functions implemented in CFE.

skliper commented 4 years ago

Trac comment by jphickey on 2015-04-14 11:23:09:

Additional commit added on the trac-17 branch to address an issue with unit tests. When running unit tests with the same pc-linux BSP that normal application code uses, signals are blocked during OS_Application_Startup(). This will prevent timers from operating since these rely on signals.

Typically, a task would be created such that OS_Application_Startup() can return and then signals would be unblocked. Not all of the test code was doing this. This commit fixes those tests that were broken.

Commit: [changeset:ad447ab]

skliper commented 4 years ago

Trac comment by jphickey on 2015-06-26 22:40:03:

== REBASED COMMIT IDS ==

The work on this ticket has been rebased to use the UT assert library, which was added under #80.

Disregard all prior commit ids listed

Here is a list of current commits on the branch {{{track-17-rebased-ut-framework}}}

Testing Note In addition to the typical Linux testing, this set (combined 57 + 17 UT assert-based OSAL black-box tests) has also been successfully executed on RTEMS. This is possible due to the BSP integration and the fact that the same support code that can boot the real application on RTEMS can be re-purposed to boot the unit tests as well.

skliper commented 4 years ago

Trac comment by glimes on 2015-06-30 11:41:35:

Recommend accept.

I kicked Bamboo and now it's building ic-2015-06-30 which is currently pointing at the end of this sequence of commits.

Please excuse some very rough patches in the build, I'm in the middle of shifting between test runner methods.

[edit: finished reviewing tickets. will be working on getting this cleaned up between now and CCB meeting]

This build is a substantial improvement over development and provides the baseline for further improvements.

skliper commented 4 years ago

Trac comment by acudmore on 2015-06-30 12:32:31:

Recommend accept. Good idea to keep the OSAL stubs in the OSAL rather than the cFE.

skliper commented 4 years ago

Trac comment by sstrege on 2015-06-30 12:40:40:

The ut_osfileapi_stubs.c file also needs to be added to /src/ut_stubs

With that addition I recommend/accept

skliper commented 4 years ago

Trac comment by glimes on 2015-06-30 14:29:21:

Per CCB meeting 2015-06-30, accepted for inclusion in development, after a few small updates we discussed in the meeting.

skliper commented 4 years ago

Trac comment by jphickey on 2015-06-30 15:14:23:

Per CCB discussion - I will add the "osfileapi" stubs to this with the caveat that there is no application currently in the build that actually uses these stubs, so the operation of the fileapi stub functions cannot be confirmed at this time.

As a descendant of the #56 fix, this will also need to be rebased after making the updates to that ticket (so the commit ids will change again).

skliper commented 4 years ago

Trac comment by jphickey on 2015-07-01 23:27:22:

This has now been rebased again onto a merge that contains the updates for #56 and also incorporates the items discussed at the 2015-06-30 CCB meeting.

The current and hopefully final list of commits on the trac-17 branch: