| Message ID | 20250710153345.104954-4-xsimonar@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Multiple unit/system tests fixes. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 7/10/25 11:33 AM, Xavier Simonart via dev wrote: > Multiple tests call ovn commands such as ovn-nbctl or ovn-sbctl or ovs > commands such as ovs-ofctl or ovs-dpctl within on_exit. > Avoid issuing those commands when ovn/ovs has been stopped. > When tests succeed, this was causing extra unnecessary error messages (when > a test succeeds, ovn and ovs are stopped before on_exit is executed), which made > debugging/understanding tests more difficult. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > --- > tests/ovs-macros.at | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > index 25b34791a..e344d22c8 100644 > --- a/tests/ovs-macros.at > +++ b/tests/ovs-macros.at > @@ -382,7 +382,19 @@ dnl Cleanup commands are executed in the reverse order of calls to this > dnl function. > m4_divert_text([PREPARE_TESTS], [dnl > on_exit () { > - (echo "$1"; cat cleanup) > cleanup.tmp > + if [ echo "$1" | grep -qe '^ovn-nbctl' ] ; then > + (echo "test -e ovn-nb/ovn-nb.sock && $1"; cat cleanup;) > cleanup.tmp > + else > + if [ echo "$1" | grep -qe '^ovn-sbctl' ]; then > + (echo "test -e ovn-sb/ovn-sb.sock && $1"; cat cleanup) > cleanup.tmp > + else > + if [ echo "$1" | grep -qe '^ovs-ofctl' -qe '^ovs-dpctl' ]; then > + (echo "test -e $OVS_RUNDIR/ovs-vswitchd.pid && $1"; cat cleanup) > cleanup.tmp > + else > + (echo "$1"; cat cleanup) > cleanup.tmp > + fi > + fi > + fi My bash knowledge isn't great. Is there a reason why we have all these nested if-else statements instead of using `elif` ? > mv cleanup.tmp cleanup > } > ])
Hi Mark Thanks for the review and comments. I'll send v2. Thanks Xavier On Mon, Jul 21, 2025 at 10:49 PM Mark Michelson <mmichels@redhat.com> wrote: > On 7/10/25 11:33 AM, Xavier Simonart via dev wrote: > > Multiple tests call ovn commands such as ovn-nbctl or ovn-sbctl or ovs > > commands such as ovs-ofctl or ovs-dpctl within on_exit. > > Avoid issuing those commands when ovn/ovs has been stopped. > > When tests succeed, this was causing extra unnecessary error messages > (when > > a test succeeds, ovn and ovs are stopped before on_exit is executed), > which made > > debugging/understanding tests more difficult. > > > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > --- > > tests/ovs-macros.at | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at > > index 25b34791a..e344d22c8 100644 > > --- a/tests/ovs-macros.at > > +++ b/tests/ovs-macros.at > > @@ -382,7 +382,19 @@ dnl Cleanup commands are executed in the reverse > order of calls to this > > dnl function. > > m4_divert_text([PREPARE_TESTS], [dnl > > on_exit () { > > - (echo "$1"; cat cleanup) > cleanup.tmp > > + if [ echo "$1" | grep -qe '^ovn-nbctl' ] ; then > > + (echo "test -e ovn-nb/ovn-nb.sock && $1"; cat cleanup;) > > cleanup.tmp > > + else > > + if [ echo "$1" | grep -qe '^ovn-sbctl' ]; then > > + (echo "test -e ovn-sb/ovn-sb.sock && $1"; cat cleanup) > > cleanup.tmp > > + else > > + if [ echo "$1" | grep -qe '^ovs-ofctl' -qe '^ovs-dpctl' ]; then > > + (echo "test -e $OVS_RUNDIR/ovs-vswitchd.pid && $1"; cat > cleanup) > cleanup.tmp > > + else > > + (echo "$1"; cat cleanup) > cleanup.tmp > > + fi > > + fi > > + fi > > My bash knowledge isn't great. Is there a reason why we have all these > nested if-else statements instead of using `elif` ? > > > mv cleanup.tmp cleanup > > } > > ]) > >
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 25b34791a..e344d22c8 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -382,7 +382,19 @@ dnl Cleanup commands are executed in the reverse order of calls to this dnl function. m4_divert_text([PREPARE_TESTS], [dnl on_exit () { - (echo "$1"; cat cleanup) > cleanup.tmp + if [ echo "$1" | grep -qe '^ovn-nbctl' ] ; then + (echo "test -e ovn-nb/ovn-nb.sock && $1"; cat cleanup;) > cleanup.tmp + else + if [ echo "$1" | grep -qe '^ovn-sbctl' ]; then + (echo "test -e ovn-sb/ovn-sb.sock && $1"; cat cleanup) > cleanup.tmp + else + if [ echo "$1" | grep -qe '^ovs-ofctl' -qe '^ovs-dpctl' ]; then + (echo "test -e $OVS_RUNDIR/ovs-vswitchd.pid && $1"; cat cleanup) > cleanup.tmp + else + (echo "$1"; cat cleanup) > cleanup.tmp + fi + fi + fi mv cleanup.tmp cleanup } ])
Multiple tests call ovn commands such as ovn-nbctl or ovn-sbctl or ovs commands such as ovs-ofctl or ovs-dpctl within on_exit. Avoid issuing those commands when ovn/ovs has been stopped. When tests succeed, this was causing extra unnecessary error messages (when a test succeeds, ovn and ovs are stopped before on_exit is executed), which made debugging/understanding tests more difficult. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- tests/ovs-macros.at | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)