Hi Rosemarie, thanks for the update. Acked-by: Mark Michelson <[email protected]>
I plan to make one small edit when merging this. See below: On Wed, Aug 12, 2026 at 6:26 PM Rosemarie O'Riorden via dev <[email protected]> wrote: > > Fix various typos, inaccuracies, and missing information in "Testing" > and "Test Development". > > Reported-at: https://redhat.atlassian.net/browse/FDP-3051 > Assisted-by: Claude Opus 4.6, Claude Code > Signed-off-by: Rosemarie O'Riorden <[email protected]> > --- > v2 -> v3: > - Update title and description. > - Fix ovn_start args as Mark recommended. > - Make "+++"s under titles the correct length. > - Add "no_recompute_check" option to OVN_CLEANUP_SBOX > - Remove list of supported versions of OVN. > --- > Documentation/topics/test-development.rst | 74 +++++++++++------------ > Documentation/topics/testing.rst | 12 ++-- > Documentation/tutorials/ovn-sandbox.rst | 2 +- > 3 files changed, 41 insertions(+), 47 deletions(-) > > diff --git a/Documentation/topics/test-development.rst > b/Documentation/topics/test-development.rst > index 823957983..0dbd76bec 100644 > --- a/Documentation/topics/test-development.rst > +++ b/Documentation/topics/test-development.rst > @@ -61,7 +61,7 @@ check COMMAND... > > Function to run COMMAND and check that it succeeds without any output. Also > logs the COMMAND. Note that most ``ovn-nbctl`` and ``ovn-sbctl`` must be run > -withing ``check`` so that the return status is checked. > +within ``check`` so that the return status is checked. > > OVN_CHECK_PACKETS([PCAP], [EXPECTED]) > +++++++++++++++++++++++++++++++++++++ > @@ -102,8 +102,8 @@ uuid as output. It also fails if the output is empty. > Daemon/Sandbox Management > ~~~~~~~~~~~~~~~~~~~~~~~~~ > > -ovn_start [--backup-northd=none|paused] [AZ] > -++++++++++++++++++++++++++++++++++++++++++++ > +ovn_start [--backup-northd[=paused] | --use-tcp-to-sb] [AZ] > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > Creates and initializes ovn-sb and ovn-nb databases and starts their > ovsdb-server instance, sets appropriate environment variables so that > ovn-sbctl > @@ -129,7 +129,8 @@ ovn_attach NETWORK BRIDGE IP [MASKLEN] [ENCAP] > > First, this function attaches BRIDGE to interconnection network NETWORK. > Second, it configures (simulated) address IP (with network mask length > MASKLEN, > -which defaults to 24) on BRIDGE. Finally, it configures the Open vSwitch > +which defaults to 24) on BRIDGE. ENCAP specifies the tunnel encapsulation > types > +(defaults to ``geneve,vxlan``). Finally, it configures the Open vSwitch > database to work with OVN and starts ovn-controller. > > sim_add SANDBOX > @@ -156,7 +157,7 @@ or:: > as [OVS_DIR] COMMAND > ++++++++++++++++++++ > > -``as $1`` sets the ``OVS_*DIR`` and ``OVN_*DIR*`` environment variables to > +``as $1`` sets the ``OVS_*DIR`` and ``OVN_*DIR`` environment variables to > point to $ovs_base/$1. > > ``as $1 COMMAND...`` sets those variables in a subshell and invokes COMMAND > @@ -187,36 +188,30 @@ 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 > +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_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) > -+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output]) > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > -Macro to create a database and starts ovsdb-server, starts ovs-vswitchd > -connected to that database, calls 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). > +Macro to create a database and start ovsdb-server, start ovs-vswitchd > connected > +to that database, and create a bridge named br0 with predictable settings. > +Additional ovs-vsctl commands can be passed via 'vsctl-args'. If those > commands > +produce output (e.g. "create" commands), 'vsctl-output' specifies the > expected > +output after filtering through uuidfilt. > > OVS_TRAFFIC_VSWITCHD_STOP([WHITELIST], [extra_cmds]) > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > -Macro to gracefully stops ovs-vswitchd and ovsdb-server, checking their log > +Macro to gracefully stop 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 WHITELIST may contain shell-quoted "sed" commands to > delete any warnings that are actually expected, e.g.:: > > OVS_TRAFFIC_VSWITCHD_STOP(["/expected error/d"]) > > -'extra_cmds' are shell commands to be executed afte OVS_VSWITCHD_STOP() is > +'extra_cmds' are shell commands to be executed after OVS_VSWITCHD_STOP() is > invoked. They can be used to perform additional cleanups such as name space > removal. > > @@ -240,35 +235,34 @@ 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) > -++++++++++++++++++++++ > +OVN_CLEANUP_SBOX(sbox[, error[, related_ports[, ignored_dp[, > ignored_tables[, no_recompute_check]]]]]) > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 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. > > 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. > +higher and signals an error if any is present. The 'error' argument may > contain > +sed commands to delete 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. > +the sandbox, and checks whether the related ports and the openflows before > +and after recompute are the same. The 'related_ports', 'ignored_dp', and > +'ignored_tables' arguments specify acceptable differences. > > -OVN_CLEANUP_CONTROLLER(sbox) > -++++++++++++++++++++++++++++ > +OVN_CLEANUP_CONTROLLER(hv[, sbox[, related_ports[, ignored_dp[, > ignored_tables[, no_recompute_check]]]]]) > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 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. > +instance. The 'hv' argument names the hypervisor; when it equals ``vtep``, > the > +macro stops ovn-controller-vtep and ovs-vtep instead of ovn-controller. The > +'sbox' argument names the sandbox directory (defaults to 'hv' when omitted). This is a bit misleading. The 'sbox' argument does not have any sort of default value in OVN_CLEANUP_CONTROLLER. However, calling OVN_CLEANUP results in calling OVN_CLEANUP_CONTROLLER with both the 'hv' and 'sbox' arguments set to the same value. I plan to remove the parenthetical at the end mentioning default values. Interestingly, there are many places throughout our system tests where we directly call OVN_CLEANUP_CONTROLLER with only a single argument. This results in many cases where we call as() with no argument. I wonder if this could result in recompute checks not working as expected. That's potentially a separate issue though. > > -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. > +Unless 'no_recompute_check' is ``True``, issues recomputes on ovn-controller > +and checks whether the related ports and the openflows before and after > +recompute are the same. The 'related_ports', 'ignored_dp', and > +'ignored_tables' arguments specify acceptable differences. > > OVN_CLEANUP_IC([az ...]) > ++++++++++++++++++++++++ > @@ -314,7 +308,7 @@ RUN_OVN_NBCTL() > +++++++++++++++ > > Macro to execute a list of commands built by the ``OVN_NBCTL`` macro. The > list > -of commands is executed in a single invocation of ``ovn-nbctl`` > +of commands is executed in a single invocation of ``ovn-nbctl``. > > OVS_VSCTL(VSCTL_COMMAND) > ++++++++++++++++++++++++ > @@ -335,7 +329,7 @@ STDOUT on stdout, and prints STDERR on stderr. If this > doesn't happen within a > reasonable time limit, then the test fails. > > There is an ``OVS_WAIT_FOR_OUTPUT_UNQUOTED`` version of this macro that > expands > -shell ``$variables``, ``$(command)``, and so on. The plain version does not > +shell ``$variables``, ``$(command)``, and so on. The plain version does not. > > OVS_WAIT_UNTIL(COMMAND[, IF-FAILED]) > ++++++++++++++++++++++++++++++++++++ > diff --git a/Documentation/topics/testing.rst > b/Documentation/topics/testing.rst > index 579422ca0..951133c29 100644 > --- a/Documentation/topics/testing.rst > +++ b/Documentation/topics/testing.rst > @@ -36,7 +36,7 @@ validating basic functionality of OVN. Before running any > of the tests > described here, you must bootstrap, configure and build OVN as > described in :doc:`/intro/install/general`. You do not need to install > OVN, Open vSwitch or to build or load the kernel module to run these test > -suites.You do not need supervisor privilege to run these test suites. > +suites. You do not need superuser privilege to run these test suites. > > Unit Tests > ~~~~~~~~~~ > @@ -78,7 +78,7 @@ To see a complete list of test options, run:: > $ make check TESTSUITEFLAGS=--help > > The results of a testing run are reported in ``tests/testsuite.log``. Report > -report test failures as bugs and include the ``testsuite.log`` in your > report. > +test failures as bugs and include the ``testsuite.log`` in your report. > > .. note:: > Sometimes a few tests may fail on some runs but not others. This is > usually a > @@ -266,7 +266,7 @@ cached. In order to force the tests to rebuild all these > objects, run:: > A typical workflow for a developer trying to improve the performance of OVN > would be the following: > > -0. Optional: Modify/add a performance test to buld the topology that you are > +0. Optional: Modify/add a performance test to build the topology that you are > benchmarking, if required. > 1. Run ``make check-perf TESTSUITEFLAGS="--rebuild"`` to generate cached > databases (and complete a test run). The results of each test run are > @@ -283,7 +283,7 @@ would be the following: > 2. Run ``make check-perf`` to measure the performance metric that you are > benchmarking against. If you are only using one test, you can specify the > test to run by adding the test number to the ``make`` command. > - (e.g. ``make check-perf TESTSUITEFLAGS="--rebuild <test number>"``) > + (e.g. ``make check-perf TESTSUITEFLAGS="<test number>"``) > 3. Modify OVN code to implement the change that you believe will improve the > performance. > 4. Go to Step 2. to continue making improvements. > @@ -447,12 +447,12 @@ CI Integration > Upgrade tests run automatically in GitHub Actions: > > *On Schedule (Weekly)* > - - Tests all supported versions (24.03, 24.09, 25.03, 25.09) > + - Tests all supported versions. > > Implementation Details > ++++++++++++++++++++++ > > -Test are run locally through ``check-upgrade`` Makefile target. > +Tests are run locally through ``check-upgrade`` Makefile target. > The flow for make check-upgrade is: > > - Makefile > diff --git a/Documentation/tutorials/ovn-sandbox.rst > b/Documentation/tutorials/ovn-sandbox.rst > index 4acc22cdd..ff519c986 100644 > --- a/Documentation/tutorials/ovn-sandbox.rst > +++ b/Documentation/tutorials/ovn-sandbox.rst > @@ -47,7 +47,7 @@ Running the sandbox does the following steps to the > environment: > 1. Creates the ``OVN_Northbound`` and ``OVN_Southbound`` databases as > described in > `ovn-nb(5)`_ and `ovn-sb(5)`_. > > -2. Creates a backup server for ``OVN_Southbond`` database. Sandbox launch > +2. Creates a backup server for ``OVN_Southbound`` database. Sandbox launch > screen provides the instructions on accessing the backup database. > However > access to the backup server is not required to go through the tutorial. > > -- > 2.55.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
