| Message ID | 20230419124422.3576765-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] tests: update OVS_PAUSE_TEST behavior | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On Wed, Apr 19, 2023 at 2:44 PM Xavier Simonart <xsimonar@redhat.com> 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 <xsimonar@redhat.com> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks! Acked-by: Ales Musil <amusil@redhat.com>
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 <amusil@redhat.com> wrote: > > > On Wed, Apr 19, 2023 at 2:44 PM Xavier Simonart <xsimonar@redhat.com> > 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 <xsimonar@redhat.com> >> --- >> 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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Looks good to me, thanks! > > Acked-by: Ales Musil <amusil@redhat.com> > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com IM: amusil > <https://red.ht/sig> >
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 <amusil@redhat.com> wrote: > >> >> >> On Wed, Apr 19, 2023 at 2:44 PM Xavier Simonart <xsimonar@redhat.com> >> 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 <xsimonar@redhat.com> >>> --- >>> 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 >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> >> Looks good to me, thanks! >> >> Acked-by: Ales Musil <amusil@redhat.com> >> >> -- 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
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
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 <xsimonar@redhat.com> --- Documentation/topics/testing.rst | 3 +++ tests/ovs-macros.at | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-)