Hi Nicholas Thanks for taking care of updating the documentation. I also have a few comments below. Thanks Xavier
On Fri, Jul 25, 2025 at 10:54 PM Mark Michelson via dev < ovs-dev@openvswitch.org> wrote: > Thanks for compiling this, Nick. I've been through the documentation and > I have some suggestions. See inline. > > On 7/21/25 10:43 AM, Nicholas Hubbard wrote: > > OVN is currently lacking documentation for how to develop tests. This > commit > > adds a file "Documentation/topics/test-development.rst" that provides > both a > > high-level overview of the OVN test-suite, along with documentation for > the > > most commonly used macros and functions. > > > > Reported-at: https://issues.redhat.com/browse/FDP-1548 > > Signed-off-by: Nicholas Hubbard <nhubb...@redhat.com> > > --- > > Documentation/automake.mk | 1 + > > Documentation/topics/index.rst | 1 + > > Documentation/topics/test-development.rst | 326 ++++++++++++++++++++++ > > 3 files changed, 328 insertions(+) > > create mode 100644 Documentation/topics/test-development.rst > > > > diff --git a/Documentation/automake.mk b/Documentation/automake.mk > > index a70c294b9..6bc88c24b 100644 > > --- a/Documentation/automake.mk > > +++ b/Documentation/automake.mk > > @@ -25,6 +25,7 @@ DOC_SOURCE = \ > > Documentation/tutorials/ovn-interconnection.rst \ > > Documentation/topics/index.rst \ > > Documentation/topics/testing.rst \ > > + Documentation/topics/test-development.rst \ > > Documentation/topics/high-availability.rst \ > > Documentation/topics/integration.rst \ > > Documentation/topics/ovn-news-2.8.rst \ > > diff --git a/Documentation/topics/index.rst > b/Documentation/topics/index.rst > > index 55bb919c0..896014dc3 100644 > > --- a/Documentation/topics/index.rst > > +++ b/Documentation/topics/index.rst > > @@ -42,6 +42,7 @@ OVN > > ovn-news-2.8 > > vif-plug-providers/index > > testing > > + test-development > > > > .. list-table:: > > > > diff --git a/Documentation/topics/test-development.rst > b/Documentation/topics/test-development.rst > > new file mode 100644 > > index 000000000..ff5d90eb8 > > --- /dev/null > > +++ b/Documentation/topics/test-development.rst > > @@ -0,0 +1,326 @@ > > +.. > > + Licensed under the Apache License, Version 2.0 (the "License"); > you may > > + not use this file except in compliance with the License. You may > obtain > > + a copy of the License at > > + > > + http://www.apache.org/licenses/LICENSE-2.0 > > + > > + Unless required by applicable law or agreed to in writing, > software > > + distributed under the License is distributed on an "AS IS" BASIS, > WITHOUT > > + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > See the > > + License for the specific language governing permissions and > limitations > > + under the License. > > + > > + Convention for heading levels in OVN documentation: > > + > > + ======= Heading 0 (reserved for the title in a document) > > + ------- Heading 1 > > + ~~~~~~~ Heading 2 > > + +++++++ Heading 3 > > + ''''''' Heading 4 > > + > > + Avoid deeper levels because they do not render well. > > + > > +================ > > +Test Development > > +================ > > + > > +This document provides information relevant to writing tests for OVN. > The > > +documentation for executing tests exists in :doc:`/topics/testing`. > > + > > +OVN uses `Autotest < > https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Using-Autotest.html#Using-Autotest > >`_ > > +for generating its tests. The top-level of the test code is located in > the > > +file ``tests/testsuite.at``. This file is expanded into a shell script > that > > +runs all of the OVN tests. Please refer to the Autotest documentation > linked > > +above for more information regarding Autotest, as the rest of this > document > > +assumes a general understanding of it. > > + > > +OVN Test Suite Overview > > +----------------------- > > + > > +All test code is located in the ``tests/`` directory at the root of the > OVN > > +repository. Generally, files suffixed with ``-macros.at`` contain > macros and > > +shell functions to aid in writing tests. Files suffixed with just > ``.at`` > > +contain the code for running actual tests. > > + > > +By convention macros are denoted with all uppercase letters, while > functions > > +use lowercase letters. > > + > > +The most used and important macros/functions are documented here. All of > > +these macros/functions are implicitly available from all test files when > > +running the testsuite via ``make check``. > > + > > +Macros and Functions > > +-------------------- > > + > > +Verification > > +~~~~~~~~~~~~ > > + > > +check COMMAND... > > +++++++++++++++++ > > + > > +Function to run COMMAND and check that it succeeds without any output. > This is > > +the primary function for actually running a test, as this function wraps > > +``AT_CHECK``. > > I think the final sentence can be removed here. > Maybe worth adding that "check COMMAND" also logs the command and that most ovn-nbctl and ovn-sbctl commands *must* be run within "check", so that the return code is checked (only "find", "list", "show", "get" and "dump-flows" arguments do not need a check). > > > + > > +OVN_CHECK_PACKETS([PCAP], [EXPECTED]) > > ++++++++++++++++++++++++++++++++++++++ > > + > > +Macro to compare packets read from PCAP, in pcap format, to those read > from > > +EXPECTED, which is a text file containing packets as hex strings, one > per line. > > +If PCAP contains fewer packets than EXPECTED, it waits up to 10 seconds > for > > +more packets to appear. > Default wait time is not 10 but 30 seconds, and can be changed through the OVS_CTL_TIMEOUT environment variable. To make a clear differentiation with the following CHECK_PACKETS macros, I would add that "reception of any extra or duplicate packets cause the test to fail". > > +OVN_CHECK_PACKETS_CONTAIN([PCAP], [EXPECTED]) > > ++++++++++++++++++++++++++++++++++++++++++++++ > > + > > +Macro to check packets read from PCAP contain data from EXPECTED. > I would add that it waits until all expected packets are received. It ignores extra packets (duplicate and others). There are two other useful CHECK_PACKETS commands, which slightly differ from the previous ones, which might be worth documenting. OVN_CHECK_PACKETS_UNIQ waits until all expected packets are received. It ignores duplicate receive packets, but extra (non duplicate) packets cause the test to fail. OVN_CHECK_PACKETS_REMOVE_BROADCAST waits until the expected number of packets (after removing bcasts) are received. Then it compares expected and received packets. Any (non broadcast) extra or duplicate packets cause the test to fail. > > + > > +check_uuid COMMAND > > +++++++++++++++++++ > > + > > +Function to run COMMAND and check that it does not print anything else > than > > +uuid as output. It also fails if the output is empty. > > + > > +Daemon/Sandbox Management > > +~~~~~~~~~~~~~~~~~~~~~~~~~ > > + > > +ovn_start [--backup-northd=none|paused] [AZ] > > +++++++++++++++++++++++++++++++++++++++++++++ > > This does not document what the "AZ" parameter is used for. The "AZ" > parameter is an arbitrary name for an availability zone, and if it is > specified, then ovn_start() will also start the ovn-ic daemon. > > > + > > +Creates and initializes ovn-sb and ovn-nb databases and starts their > > +ovsdb-server instance, sets appropriate environment variables so that > ovn-sbctl > > +and ovn-nbctl use them by default, and starts ovn-northd running > against them. > > + > > +Normally this starts only an active northd and no backup northd. The > following > > +options are accepted to adjust that: > > + > > +``--backup-northd`` Start a backup northd. > > +``--backup-northd=paused`` Start the backup northd in the paused state. > > +``--use-tcp-to-sb`` Use tcp to connect to sb. > > This option should probably indicate what TCP is being used as an > alternative to. Something like "Use TCP instead of SSL to connect to the > southbound database." > > > + > > +ovn_attach NETWORK BRIDGE IP [MASKLEN] [ENCAP] > > +++++++++++++++++++++++++++++++++++++++++++++++ > > + > > +First, this function attaches BRIDGE to interconnection network > NETWORK, just > > The NETWORK referenced here is one created by the net_add() function. To > make the document a bit easier to read, I would document the net_add() > function before ovn_attach(). > > > +like ``net_attach NETWORK BRIDGE``. Second, it configures (simulated) IP > > This references net_attach but net_attach is not defined in this > document. I think the easiest thing to do is to remove "just like > ``net_attach NETWORK BRIDGE``" from the sentence. > > > +address IP (with network mask length MASKLEN, which defaults to 24) on > BRIDGE. > > +Finally, it configures the Open vSwitch database to work with OVN and > starts > > +ovn-controller. > > + > > +sim_add SANDBOX > > ++++++++++++++++ > > + > > +Function to start a new simulated Open vSwitch instance named SANDBOX. > Files > > +related to the instance, such as logs, databases, sockets, and > pidfiles, are > > +created in a subdirectory of the main test directory also named > > +SANDBOX. Afterward, the ``as`` command (see below) can be used to run > Open > > +vSwitch utilities in the context of the new sandbox. > > Instead of saying "Open vSwitch utilities" I'd just say "commands" since > we also use the as() function to run ovn-appctl in the context of a > sandbox, as well as other miscellaneous commands. > > > + > > +The new sandbox starts out without any bridges. Use ovs-vsctl in the > context of > > +the new sandbox to create a bridge, e.g.:: > > + > > + sim_add hv0 # Create sandbox hv0. > > + as hv0 # Set hv0 as default sandbox. > > + ovs-vsctl add-br br0 # Add bridge br0 inside hv0. > > + > > +or:: > > + > > + sim_add hv0 > > + as hv0 ovs-vsctl add-br br0 > > + > > +as [OVS_DIR] COMMAND > > +++++++++++++++++++++ > > + > > +``as $1`` sets the ``OVS_*DIR`` environment variables to point to > $ovs_base/$1. > > Nit: In OVN, the as() command sets both the OVS_*DIR and the OVN_*DIR > environment variables. > > > + > > +``as $1 COMMAND...`` sets those variables in a subshell and invokes > COMMAND > > +there. > > + > > +net_add NETWORK > > ++++++++++++++++ > > + > > +Function to create a new interconnection network named NETWORK. > > + > > +wait_for_ports_up [PORT...] > > ++++++++++++++++++++++++++++ > > + > > +With arguments, this function waits for specified Logical_Switch_Ports > to come > > +up. Without arguments, waits for all "plain" and router > Logical_Switch_Ports to > > +come up. > > + > > +DUMP_FLOWS(sbox, output_file) > > ++++++++++++++++++++++++++++++ > > DUMP_FLOWS() is only used directly by one test case in the testsuite. > Otherwise, it is used indirectly by other macros. Therefore, I think > it's not worth documenting in this file. > > > + > > +Dump openflows to ``output_file`` for ``sbox``. > > + > > +PARSE_LISTENING_PORT LOGFILE VARIABLE() > > Most other macros documented in this file place the parameters in > parentheses. So this should probably be: > > PARSE_LISTENING_PORT(LOGFILE, VARIABLE) > > > ++++++++++++++++++++++++++++++++++++++++ > > + > > +Macro that parses the TCP or SSL/TLS port on which a server is > listening from > > +LOGFILE, given that the server was told to listen on a kernel-chosen > port, and > > +assigns the port number to shell VARIABLE. You should specify the > listening > > +remote as ptcp:0:127.0.0.1 or pssl:0:127.0.0.1, or the equivalent with > [::1] > > +instead of 127.0.0.1. Here's an example of how to use this with > ovsdb-server:: > > + > > + ovsdb-server --log-file --remote=ptcp:0:127.0.0.1 ... > > + PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT]) > > + > > +Now $TCP_PORT holds the listening port. > > + > > +OVN_POPULATE_ARP() > > +++++++++++++++++++ > > + > > +Macro to pre-populate the ARP tables of all of the OVN instances that > have been > > +started with ```ovn_attach()``. That means that packets sent from one > > +hypervisor to another never get dropped or delayed by ARP resolution, > which > > +makes testing easier. > > + > > > +OVS_VSWITCHD_START([vsctl-args],[vsctl-output],[=override],[vswitchd-aux-args]) > > OVN tests don't call this macro directly. Instead, they call > OVS_TRAFFIC_VSWITCHD_START(), which calls this macro. So it's probably a > better idea to document that macro instead of this one. > > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > + > > +Macro to creates a database and start ovsdb-server, start ovs-vswitchd > > +connected to that database, call ovs-vsctl to create a bridge named br0 > with > > +predictable settings, passing 'vsctl-args' as additional commands to > ovs-vsctl. > > +If 'vsctl-args' causes ovs-vsctl to provide output (e.g. because it > includes > > +"create" commands) then 'vsctl-output' specifies the expected output > after > > +filtering through uuidfilt. > > + > > +If a test needs to use "system" devices (as dummies), then specify > > +``=override`` (literally) as the third argument. Otherwise, system > devices > > +won't work at all (which makes sense because tests should not access a > system's > > +real Ethernet devices). > > + > > +OVS_VSWITCHD_STOP([ALLOWLIST]) > > +++++++++++++++++++++++++++++++ > > Similar to the previous macro, OVN tests call > OVS_TRAFFIC_VSWITCHD_STOP() instead of OVS_VSWITCHD_STOP() > > > + > > +Macro to gracefully stops ovs-vswitchd and ovsdb-server, checking their > log > > +files for messages with severity WARN or higher and signaling an error > if any > > +is present. The optional ALLOWLIST may contain shell-quoted "sed" > commands to > > +delete any warnings that are actually expected, e.g.:: > > + > > + OVS_VSWITCHD_STOP(["/expected error/d"]) > > + > > +OVS_APP_EXIT_AND_WAIT(DAEMON) > > ++++++++++++++++++++++++++++++ > > + > > +Ask the daemon named DAEMON to exit, via ``ovs-appctl``, and then wait > for it > > +to exit. > > + > > +Macro to dump openflows to ``output_file`` for ``sbox``. > > I think the above sentence is a leftover from DUMP_FLOWS() and should be > removed. > > > + > > +OVN_CLEANUP(sim [, sim ...]) > > +++++++++++++++++++++++++++++ > > + > > +Macro to gracefully terminate all OVN daemons, including those in > specified > > +sandbox instances. > It also checks the log file for messages with severity WARN or higher and signals an error if any is present. Optional arguments may contain "acceptable" error messages. Before terminating the daemons, it also issues recomputes on ovn-controllers in listed sandboxes, and checks whether the "related ports" and the openflows before and after recompute are the same. Optional arguments may also contain acceptable "related_ports" differences, datapaths and tables on which flow differences are considered as acceptable. > > + > > +OVN_CLEANUP_SBOX(sbox [...]) > > +++++++++++++++++++++++ > > + > > +Macro to gracefully terminate OVN daemons in the specified sandbox > instance. > > +The sandbox name ``vtep`` is treated as a special case, and is assumed > to have > > +ovn-controller-vtep and ovs-vtep daemons running instead of > ovn-controller. > > + > As for OVN_CLEANUP, it also checks the log file for messages with severity WARN or higher and signals an error if any is present, issues recomputes on ovn-controllers in listed sandbox, and checks whether the "related ports" and the openflows before and after recompute are the same. Optional arguments may also contain acceptable errors, "related_ports" differences, datapaths and tables on which flow differences are considered as acceptable. > > +OVN_CLEANUP_CONTROLLER(sbox[...]) > > +++++++++++++++++++++++++++++ > > + > > +Macro to gracefully terminate ovn-controller in the specified sandbox > > +instance. The sandbox name ``vtep`` is treated as a special case, and is > > +assumed to have ovn-controller-vtep and ovs-vtep daemons running > instead of > > +ovn-controller. > As for previous macros, it issues recomputes on ovn-controllers in listed sandbox, and checks whether the "related ports" and the openflows before and after recompute are the same. Optional arguments may also contain acceptable "related_ports" differences, datapaths and tables on which flow differences are considered as acceptable. > > + > > +OVN_CLEANUP_IC([az ...]) > > +++++++++++++++++++++++++ > > + > > +Macro to gracefully terminate all interconnection DBs and daemons in the > > +specified AZs, if any. > > + > > +Test Management > > +~~~~~~~~~~~~~~~ > > + > > +OVN_FOR_EACH_NORTHD(TEST) > > ++++++++++++++++++++++++++ > > + > > +Macro to wrap an arbitrary TEST. Defines versions of the TEST with all > > +combinations of northd, parallelization enabled and conditional > monitoring > > +on/off. Normally the first statement in TEST is a call to ``AT_SETUP``. > > + > > +OVN_FOR_EACH_NORTHD_NO_HV(TEST) > > ++++++++++++++++++++++++++++++++ > > + > > +Macro to wrap an arbitrary TEST. Defines versions of the TEST with all > > +combinations of northd and parallelization enabled. To be used when the > > +ovn-controller configuration is not relevant. Normally the first > statement in > > +TEST is a call to ``AT_SETUP``. > > + > > +on_exit COMMAND > > ++++++++++++++++ > > + > > +Function to add the shell COMMAND to a collection that is executed when > the > > +current test completes, as a cleanup action. The most common use is to > kill a > > +daemon started by the test. This is important to prevent tests that > start > > +daemons from hanging at exit. > > + > > +Cleanup commands are executed in the reverse order of calls to this > function. > > + > > +OVN_NBCTL(NBCTL_COMMAND) > > +++++++++++++++++++++++++ > > + > > +Macro to add NBCTL_COMMAND to list of commands to be run by the > > +``RUN_OVN_NBCTL`` macro. > > + > > +RUN_OVN_NBCTL() > > ++++++++++++++++ > > + > > +Macro to execute a list of commands built by the ``OVN_NBCTL`` macro. > > It should be noted that the list of commands is executed in a single > `ovn-nbctl()` invocation. > > > + > > +OVS_VSCTL(VSCTL_COMMAND) > > +++++++++++++++++++++++++ > > + > > +Macro to add VSCTL_COMMAND to list of commands to be run by > ``RUN_OVS_VSCTL``. > > + > > +RUN_OVS_VSCTL() > > ++++++++++++++++ > > + > > +Macro to execute the list of commands built by the ``OVS_VSCTL`` macro. > > Same note here as with RUN_OVN_NBCTL(). > > > + > > +ovs_setenv > > +++++++++++ > > ovs_setenv is never used directly by OVN tests. The most common way > ovs_setenv is used is by calling the as() function, which uses > ovs_setenv under the hood. It's probably not worth documenting > ovs_setenv in this file. > > > + > > +With no parameter or an empty parameter, this function sets the OVS_*DIR > > +environment variables to point to $ovs_base, the base directory in > which the > > +test is running. > > + > > +With a parameter, sets them to $ovs_base/$1. > > + > > +OVS_WAIT_FOR_OUTPUT(COMMAND, EXIT-STATUS, STDOUT, STDERR) > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > + > > +Executes shell COMMAND in a loop until it exits with status > EXIT-STATUS, prints > > +STDOUT on stdout, and prints STDERR on stderr. If this doesn't happen > within a > > +reasonable time limit, then the test fails. > > + > > +There is a ``OVS_WAIT_FOR_OUTPUT_UNQUOTED`` version of this macro that > expands > > s/There is a/There is an/ > > > +shell ``$variables``, ``$(command)``, and so on. The plain version > does not > > + > > +OVS_WAIT_UNTIL(COMMAND[, IF-FAILED]) > > +++++++++++++++++++++++++++++++++++++ > > + > > +Macro that executes shell COMMAND in a loop until it returns zero > return code. > > +If COMMAND does not return zero code within reasonable time limit, then > the > > +test fails. In that case, runs IF-FAILED before exiting. > > + > > +OVS_WAIT_WHILE(COMMAND[, IF-FAILED]) > > +++++++++++++++++++++++++++++++++++++ > > + > > +Macro that executes shell COMMAND in a loop until it returns non-zero > return > > +code. If COMMAND does not return non-zero code within reasonable time > limit, > > +then the test fails. In that case, runs IF-FAILED before exiting. > > + > > +OVS_WAIT_UNTIL_EQUAL(COMMAND, OUTPUT) > > ++++++++++++++++++++++++++++++++++++++ > > + > > +Macro that executes shell COMMAND in a loop until it returns zero and > the > > +output equals OUTPUT. If COMMAND does not return zero or a desired > output > > +within a reasonable time limit, fails the test. > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
