On Mon, Mar 14, 2016 at 09:32:34AM -0700, Avneesh Sachdev wrote:
> It looks like we agree that:
>
> - test functions should be easier to write.
>
> - test code should not be part of a production build (whether via
> conditional compilation or separate binaries).
>
> Given the above, why not let the user use the familiar vtysh interface to
> invoke tests?
I'm not saying test code shouldn't be run through the CLI. I do think
dlsym() with a fixed assumed function signature is not a nice way of
doing things.
As I understand it, you're trying to fix 2 things here:
(a) DEFUN + install_element suckage
(b) vtysh rebuilding
I think the better option to fix these is to fix them individually.
(a) could be fixed with some easier to use macro specific for tests
(b) can be fixed by using telnet, or by using a "catch-all" in vtysh
(a "catch-all" could be "test .LINE" which would make vtysh send the
command down to the daemons without caring about the arguments; the
daemon could then implement multiple "test foo A B C" "test bar D E"
commands which would all work)
The following diff will make all "test ..." commands work in all
daemons; the daemons should then use DEFUN_NOSH to add specific
commands (extract.pl need NOT process these).
=> you can add & use test commands without recompiling vtysh
diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index 63b596a..b346d6b 100644
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -785,6 +785,15 @@ vtysh_end (void)
}
DEFUNSH (VTYSH_ALL,
+ vtysh_test_all,
+ vtysh_test_all_cmd,
+ "test .LINE",
+ "test commands\ntest commands\n")
+{
+ return CMD_SUCCESS;
+}
+
+DEFUNSH (VTYSH_ALL,
vtysh_end_all,
vtysh_end_all_cmd,
"end",
@@ -2450,6 +2459,8 @@ vtysh_init_vty (void)
install_element (VTY_NODE, &vtysh_exit_line_vty_cmd);
install_element (VTY_NODE, &vtysh_quit_line_vty_cmd);
+ install_element (ENABLE_NODE, &vtysh_test_all_cmd);
+
/* "end" command. */
install_element (CONFIG_NODE, &vtysh_end_all_cmd);
install_element (ENABLE_NODE, &vtysh_end_all_cmd);
> > My opinion on this is:
> > - the DEV_BUILD macro is a bad idea. Test code should always be
> > compiled, or it will fall into bit-rot. We have that problem with the
> > code in tests/ already, people don't update it when changing some
> > structure and it breaks.
>
> Bit rot should not be a problem if a CI system runs the tests.
Yes, but it's still better if this also happens on every developer's
machine, not only the CI system.
> > There is a different argument for separating out the test code, i.e.
> > not having it in the installed binaries. The programs in tests/ have
> > that automatically since they're independent programs. For extra code
> > bits in the daemons, the nicest way would be to have them in separate
> > files which for example get linked into a second binary (like
> > "zebra/testzebra"). This doesn't work for anything "static" in source
> > files, unfortunately... not sure what the best way there would be.
> >
>
> A CI system can do multiple out-of-source builds: one for production and
> another for testing. Thereafter, the same automation that is used to run
> quagga today can be used create various scenarios and invoke test
> functions. This requires no changes to the makefiles/build system or the
> quagga automation.
True, but if the same can be done without the extra switch that's IMHO
preferrable...
And... I'm not even so sure the test code really needs to be disabled.
There's probably quite a few situations where extra tools are useful on
user installations when a bug is found. Maybe hidden behind a "_debug
enable". (depends on what the test function does, of course)
Cheers,
-David
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
