[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openvswitch-dev
Subject:    [ovs-dev] [ovstest 1/2] unit-test: Add ovstest
From:       blp () nicira ! com (Ben Pfaff)
Date:       2014-03-31 16:16:30
Message-ID: 20140331161630.GH29657 () nicira ! com
[Download RAW message or body]

On Sun, Mar 30, 2014 at 06:45:09PM -0700, Andy Zhou wrote:
> Changing one of the files in the Open vSwitch ``lib'' directory
> causes 43 binaries to be relinked, which takes a lot of time even with
> parallel ``make''.  31 of those binaries are in the ``tests''
> directory.  ovs-test attemps to combine most of those binaries into a
> single test program that just takes a subcommand name as its first
> command-line argument.
> 
> The following patch makes use of this infrastructure.
> 
> Signed-off-by: Andy Zhou <azhou at nicira.com>

I like it.

The user interface could be improved a little.  Running without any
arguments at all exits without doing anything and a status of zero, but
I'd prefer that it prints a message on stderr and exits with a nonzero
status.  (Otherwise this could look like a "false pass" if some test has
a bug that passes no arguments to ovstest.)

The --help output is not very helpful, I would provide a little more
context:
--help, 0, 0

The OVSTEST_REGISTER macro includes a cast for the function type.  I
think it would be better to require the function to have the correct
type, that is, to omit the cast:
> +#define OVSTEST_REGISTER(name, function, sub_commands) \
> +    OVS_CONSTRUCTOR(register_##function) { \
> +        ovstest_register(#name, (ovstest_func)function, \
> +                           sub_commands); \
> +    }

Acked-by: Ben Pfaff <blp at nicira.com>

[prev in list] [next in list] [prev in thread] [next in thread]