Experimental Physics and
Industrial Control System

Andrew Johnson <[email protected]> · Wed, 02 Jul 2014 19:14:57 -0000

Review: Needs Fixing

Hi Michael,

This looks much better. There are still a few minor things to clean up and we will eventually need appDevGuide text for the new dbUnitTest API, but I will accept this approach. Some comments:

--------------------------------------------------------------

A patch for the following fixes exists, or I can commit the
changes if you prefer. I tested the result on Linux & VxWorks,
builds Ok for RTEMS:

Build errors: Include guard missing from epicsUnitTest.h;
    dbShutdownTest.c was calling the non-universal strcasecmp()
    instead of epicsStrCaseCmp().

I would prefer that ioc/db/test not depend on std. I modified
    xRecord to make it a working record type, and simplified the
    other test programs so they all use the same new expanded
    dbd file rather than each making their own. I also added
    dbShutdownTest() to epicsRunDbTests().

--------------------------------------------------------------

I would like to see these issues addressed:

I don't like the public name iocBuildNoCA(), if anything the
    iocBuild routine should have "WithCA" in its name instead,
    as eventually merging pvAccess will invalidate a ...NoCA
    name, but that may be going a little too far. How about
    using iocBuildIsolated() instead of ...NoCA (or see
    http://thesaurus.com/browse/isolated for some alternative
    adjectives, Sequestered would also be good but it's even
    longer).

In dbUnitTest.c shouldn't the Type arguments to OP use the
    epics{Int,UInt,Float}<n> names? If not I suspect there
    should at least be a couple more 'unsigned' keywords in
    there.

The dbUnitTest.h API can extend the epicsUnitTest.h one and add
    its own Ok() routines. I'd suggest testIocInitOk() and
    testIocShutdownOk() [note capitalization and name change]
    which should call testOk() directly (with a suitable
    description string) instead of making the user remember to
    invert the return values.

The name testVdbPutField() should probably be testdbVPutField()
    since most of the other routine names start 'testdb'.

The name testGetRecord() doesn't match the other names (use
    testdbRecordPtr() maybe?), and should say "record" in its
    abort message instead of "PV" (usually records are PVs, but
    not all PVs are records...).

The testdbPrepare() routine could call eltc(0) to turn off
    errlog warning messages which otherwise get sent to stderr
    and appear in the test output.

--------------------------------------------------------------

These are minor details which don't have to be fixed now but
should be eventually:

The variable(atExitDebug,int) declaration should not appear in
    dbCore.dbd, but libCom doesn't currently have its own .dbd
    file where it should really belong.

src/ioc/misc/iocInit.c routine names (static):
  * Change the iocBuild_<n>() names to be more descriptive?

dbShutdownTest does not have a checkNoCommonThreads() test, so
    it's not testing the full operation of iocShutdown().

These ioc/db/test programs need converting to use dbUnitTest.h:
    dbChannelTest.c chfPluginTest.c and arrShorthandTest.c

--------------------------------------------------------------

- Andrew

-- 
https://code.launchpad.net/~epics-core/epics-base/ioc-shutdown2/+merge/224213
Your team EPICS Core Developers is subscribed to branch lp:epics-base.

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System