On Fri, Oct 23, 2020 at 4:32 PM Dumitru Ceara <[email protected]> wrote: > > On 10/23/20 12:46 PM, Mark Gray wrote: > > On 23/10/2020 11:20, Dumitru Ceara wrote: > >> From: Vasu Dasari <[email protected]> > >> > >> Upstream OVS commit: > >> commit c99d14775f78cb38b2109add063f58201ba07652 > >> Author: Vasu Dasari <[email protected]> > >> Date: Mon Jul 15 17:15:01 2019 -0400 > >> > >> ovs-macros: An option to suspend test execution on error > >> > >> Origins for this patch are captured at > >> > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html. > >> > >> Summarizing here, when a test fails, it would be good to pause test > >> execution > >> and let the developer poke around the system to see current status of > >> system. > >> > >> As part of this patch, made a small tweaks to ovs-macros.at, so that > >> when test > >> suite fails, ovs_on_exit() function will be called. And in this > >> function, a check > >> is made to see if an environment variable to OVS_PAUSE_TEST is set. If > >> it is > >> set, then test suite is paused and will continue to wait for user input > >> Ctrl-D. Meanwhile user can poke around the system to see why test case > >> has > >> failed. Once done with investigation, user can press ctrl-d to cleanup > >> the > >> test suite. > >> > >> For example, to re-run test case 139: > >> > >> export OVS_PAUSE_TEST=1 > >> cd tests/system-userspace-testsuite.dir/139 > >> sudo -E ./run > >> > >> When error occurs, above command would display something like this: > >> ===================================================== > >> Set environment variable to use various ovs utilities > >> export > >> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139 > >> Press ENTER to continue: > >> > >> ===================================================== > >> And from another window, one can execute ovs-xxx commands like: > >> export > >> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139 > >> $ ovs-ofctl dump-ports br0 > >> . > >> . > >> > >> To be able to pause while performing `make check`, one can do: > >> $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v' > >> > >> Acked-by: Aaron Conole <[email protected]> > >> Signed-off-by: Vasu Dasari <[email protected]> > >> Signed-off-by: Ben Pfaff <[email protected]> > >> > >> Signed-off-by: Dumitru Ceara <[email protected]> > >> --- > >> Documentation/topics/testing.rst | 24 ++++++++++++++++++++++++ > >> tests/ovs-macros.at | 24 +++++++++++++++++++++++- > >> 2 files changed, 47 insertions(+), 1 deletion(-) > >> > >> diff --git a/Documentation/topics/testing.rst > >> b/Documentation/topics/testing.rst > >> index 242608a..e07cdf8 100644 > >> --- a/Documentation/topics/testing.rst > >> +++ b/Documentation/topics/testing.rst > >> @@ -89,6 +89,30 @@ report test failures as bugs and include the > >> ``testsuite.log`` in your report. > >> > >> $ make check TESTSUITEFLAGS=-j8 RECHECK=yes > >> > >> +Debugging unit tests > >> +++++++++++++++++++++ > >> + > >> +To initiate debugging from artifacts generated from `make check` run, set > >> the > >> +``OVS_PAUSE_TEST`` environment variable to 1. For example, to run test > >> case > >> +139 and pause on error:: > >> + > >> + $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v 139' > >> + > >> +When error occurs, above command would display something like this:: > >> + > >> + Set environment variable to use various ovs utilities > >> + export OVS_RUNDIR=<dir>/ovs/_build-gcc/tests/testsuite.dir/0139 > >> + Press ENTER to continue: > >> + > >> +And from another window, one can execute ovs-xxx commands like:: > >> + > >> + export > >> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/testsuite.dir/0139 > >> + $ ovs-ofctl dump-ports br0 > >> + . > >> + . > >> + > >> +Once done with investigation, press ENTER to perform cleanup operation. > >> + > >> .. _testing-coverage: > >> > >> Coverage > >> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > >> index 3dcf8f9..2370cd2 100644 > >> --- a/tests/ovs-macros.at > >> +++ b/tests/ovs-macros.at > >> @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS]) > >> # directory. > >> ovs_init() { > >> ovs_base=`pwd` > >> - trap '. "$ovs_base/cleanup"' 0 > >> + trap ovs_on_exit 0 > >> : > cleanup > >> ovs_setenv > >> } > >> > >> +# Catch testsuite error condition and cleanup test environment by tearing > >> down > >> +# all interfaces and processes spawned. > >> +# User has an option to leave the test environment in error state so that > >> system > >> +# can be poked around to get more information. User can enable this > >> option by setting > >> +# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to > >> resume the > >> +# cleanup operation. > >> +ovs_pause() { > >> + echo "=====================================================" > >> + echo "Set following environment variable to use various ovs utilities" > >> + echo "export OVS_RUNDIR=$ovs_base" > >> + echo "Press ENTER to continue: " > >> + read > >> +} > >> + > >> +ovs_on_exit () { > >> + if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then > >> + trap '' INT > >> + ovs_pause > >> + fi > >> + . "$ovs_base/cleanup" > >> +} > >> + > >> # With no parameter or an empty parameter, sets the OVS_*DIR > >> # environment variables to point to $ovs_base, the base directory in > >> # which the test is running. > >> > > > > This applied and worked for me. This is useful functionality for > > debugging and now it works as OVS does. > > > > Thanks for the review, Mark! > > > In the future, I think the UI/docs could be improved. For example, I > > think this should be a flag to 'TESTSUITE_FLAGS' rather than an > > environment variable and I don't think the -v flag should be necessary > > for this. For the moment, it is better to get it to the same level as > > OVS so I think it should be applied as-is. > > > > Yes, I'm however not sure what the best solution is; should enhancements go to > the OVS version of ovs-macros.at first or do we consider that at this point > OVN > already forked its own version of ovs-macros.at
Since we have already forked these files, I think these files can be considered independent. However if the enhancements benefit OVS too, I think those patches can be submitted to OVS first. Thanks Numan > > There's a combination of approaches when using OVS utilities within OVN, e.g.: > - we use the ovs-lib (generated from ovs-lib.in) from the OVS repo we build > against > - we have our own copy of ovs-macros.at in OVN. > > I guess this is somehow related to the OVS/OVN build compatibility discussion > [0]. > > Thanks, > Dumitru > > [0] https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376491.html > > > Acked-by: Mark Gray <[email protected]> > > > > _______________________________________________ > 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
