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 <az...@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 <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to