Thanks Ben for the review.
I will push with following incremental changes.
The first two issues, argument check and --help took a bit more code
which I don't feel comfortable check-in without review. I will
incorporate them into the follow on patch series.
------------------------------------
index 300a0d9..6d10a5d 100644
--- a/tests/ovstest.h
+++ b/tests/ovstest.h
@@ -73,13 +73,12 @@ ovstest_register(const char *test_name, ovstest_func f,
* // not have sub commands. Otherwise, its command
* // array can replace the NULL here.
*
- * OVSTEST_REGISTER(my-test, my_test_main, NULL);
+ * OVSTEST_REGISTER("my-test", my_test_main, NULL);
*/
#define OVSTEST_REGISTER(name, function, sub_commands) \
OVS_CONSTRUCTOR(register_##function) { \
- ovstest_register(#name, (ovstest_func)function, \
- sub_commands); \
+ ovstest_register(name, function, sub_commands); \
}
#endif
diff --git a/tests/test-heap.c b/tests/test-heap.c
index dfb0981..3e0940c 100644
--- a/tests/test-heap.c
+++ b/tests/test-heap.c
@@ -485,4 +485,4 @@ test_heap_main(int argc, char *argv[])
run_command(argc - 1, argv + 1, commands);
}
-OVSTEST_REGISTER(test-heap, test_heap_main, commands);
+OVSTEST_REGISTER("test-heap", test_heap_main, commands);
On Mon, Mar 31, 2014 at 9:16 AM, Ben Pfaff <b...@nicira.com> wrote:
> 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
