On 4/20/23 18:48, Xavier Simonart wrote: > CoPP test failed for this patch, but this is in a flaky test - the failure > is independent of this patch. > A patch for the CoPP test will be proposed later, independently of this > patch > > On Thu, Apr 20, 2023 at 8:04 AM Ales Musil <[email protected]> wrote: > >> >> >> On Wed, Apr 19, 2023 at 2:44 PM Xavier Simonart <[email protected]> >> wrote: >> >>> 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. >>> >>> When OVS_PAUSE_TEST=1 and the test succeeds, system is still waiting for >>> CTRL-D >>> to resume. However, there is no added value to this behavior, as cleanup >>> is already >>> complete (the only potential added value could be to keep the logs, which >>> can be >>> achieved using -d option). >>> >>> This patch causes OVS_PAUSE_TEST=1 to wait for CTRL-D before cleanup only >>> for failed >>> tests. For successful tests, the test completes as if no OVS_PAUSE_TEST=1. >>> This new behavior helps in running the same test in loop, with >>> OVS_PAUSE_TEST=1, >>> and stopping when it fails, and keep environment in error state. >>> This is useful in trying to reproduce some flaky tests. >>> >>> Note that the same macro exists in OVS tree. It could be updated there as >>> well >>> if/once the patch is accepted. ovs-macros are however already slighly >>> different in OVS >>> and OVN subtrees. >>> >>> Signed-off-by: Xavier Simonart <[email protected]> >>> --- >>> Documentation/topics/testing.rst | 3 +++ >>> tests/ovs-macros.at | 3 ++- >>> 2 files changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/topics/testing.rst >>> b/Documentation/topics/testing.rst >>> index db265344a..14dbaa2cb 100644 >>> --- a/Documentation/topics/testing.rst >>> +++ b/Documentation/topics/testing.rst >>> @@ -113,6 +113,9 @@ And from another window, one can execute ovs-xxx >>> commands like:: >>> >>> Once done with investigation, press ENTER to perform cleanup operation. >>> >>> +OVS_PAUSE_TEST=1 only pauses failed tests when run with '-v' option. >>> +Tests run without '-v', or successful tests, are not paused. >>> + >>> .. _testing-coverage: >>> >>> Coverage >>> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at >>> index 36b58b5ae..d3e6c7ab5 100644 >>> --- a/tests/ovs-macros.at >>> +++ b/tests/ovs-macros.at >>> @@ -68,7 +68,8 @@ ovs_pause() { >>> } >>> >>> ovs_on_exit () { >>> - if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then >>> + rv=$? >>> + if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ] && [ $rv != 0 >>> ]; then >>> trap '' INT >>> ovs_pause >>> fi >>> -- >>> 2.31.1 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> >> Looks good to me, thanks! >> >> Acked-by: Ales Musil <[email protected]> >> >> --
Thanks, Xavier and Ales! It makes sense to me so I applied this to the main branch. Ilya, what do you think about picking this change up in OVS too? Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
