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

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

Closed
skliper opened this issue Sep 30, 2019 · 17 comments
Closed

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

skliper opened this issue Sep 30, 2019 · 17 comments
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

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:

  • UT_Text() - informational / log file text printing function;
  • UT_Report() - provides a common method to check for a condition, along with code to log PASSED/FAILED in a consistent manner;
  • UT_SetRtnCode() - ability to tailor the response code of stub functions in order to exercise error paths;
  • UT_ReportFailures() - test summary generator

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 skliper added this to the osal-4.2 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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:

  • The prototypes are in a ".h" file supplied by OSAL and checked out from the OSAL repository as one commit.
  • The implementation is in a ".c" file supplied by CFE and checked out from the CFE repository as a //different// commit.

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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

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

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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}}}

  • [changeset:eefba4bf] Integrates UT assert with the OSAL "BSP" code. Note that this uses //only// the BSP, not the rest of OSAL, for unit test. The BSP provides start up functions (i.e. the "main" abstraction) and UT-specific text/console output routines. This eliminates most {{{#ifdef}}} statements within the test code for support of certain platforms as well as compile time flags for verbosity level as it is all handled by the UT BSP.

  • [changeset:31defa0a] ports all of the existing OSAL "black-box" style tests (the ones under {{{src/tests}}}) to use the UT assert framework. Error counting and test case reporting is now done via a {{{UtAssert}}} call in all of these tests.

  • [changeset:177d954] introduces a new "stub helper" extensions to UT assert that consolidate many of the repetitive operations found in stub functions. This includes a generic way of setting a "deferred" return code (i.e. return X after Y calls) or a forced return code, passing a data buffer (i.e. for read/write stubs), or similar operations. It does this //without// requiring the instantiation of a separate {{{UT_SetRtn_t}}} object so the stub code becomes much cleaner and more manageable.

  • [changeset:23670ac] Adds the generic OSAL stub functions (which are used to test //other// code like CFE) to the OSAL repository under {{{src/ut-stubs}}} directory. These stubs use the helper routines added in the previous commit. This also serves as an example of how those stub helper functions can make the implementations much simpler and more straightforward compared to the prior implementation within CFE ({{{ut_osapi_stubs.c}}}).

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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
Copy link
Contributor Author

skliper commented Sep 30, 2019

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:

  • [changeset:10b7816]
  • [changeset:d6c581c]
  • [changeset:9533915]
  • [changeset:8be943b] (this also adds the osfileapi stubs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant