On Mon, Mar 21, 2022 at 1:39 PM Ilya Maximets <[email protected]> wrote:
>
> OVS_WAIT_UNTIL() macro has only 2 arguments and doesn't check
> the output of the command, but bonding tests are trying to use
> it as if it was AT_CHECK macro. That makes checks in bonding
> tests mostly useless, since they are not actually checking
> anything except for command returning zero.
>
> Introducing a new macro OVS_WAIT_UNTIL_EQUAL that will actually
> perform the comparison with the desired output. Using it for
> the bonding tests and fixing all the caught incorrect expected
> outputs along the way.
- Did you consider having the same semantic for OVS_WAIT_UNTIL() as
AT_CHECK(), rather than introduce a new helper?
I think it would be more unit tests developer friendly.
- There might also be some other broken checks, worth followup fix(es).
I caught one in tests/system-route.at:
dnl Check that OVS catches route updates.
OVS_WAIT_UNTIL([ovs-appctl ovs/route/show | grep 'p1-route' | sort], [0], [dnl
Cached: 10.0.0.17/24 dev p1-route SRC 10.0.0.17
Cached: 10.0.0.17/32 dev p1-route SRC 10.0.0.17 local
])
>
> Fixes: b4e50218a0f8 ("bond: Add 'primary' interface concept for active-backup
> mode.")
> Signed-off-by: Ilya Maximets <[email protected]>
Otherwise the patch lgtm.
--
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
