Of the 15 patches in this set, this is the only one that I disagree with.

The problem with this change is that the test writer likely has a specific amount of recomputes they expect to see based on the changes that are applied. With the "-le" change, it means that the test could pass in a situation that should probably actually fail.

Here are a couple of modification ideas:

1) Instead of providing a single number for the expected recompute count, provide a range instead. The test currently does `check_recompute_counter 5 5 5` , but it could be changed to something like `check_recompute_counter 4 5 4 5 4 5`, giving a minimum and maximum number of recomputes.

2) Provide an optional "tolerance" parameter. The test currently does `check_recompute_counter 5 5 5` but it could be something like `check_recompute_counter 5 5 5 -1` to indicate that the number could actually be 5 - 1 and we'd still consider that a pass.

What do you think?

On 9/18/23 12:47, Xavier Simonart wrote:
We might get less recomputes than expected: e.g. Port_Binding->chassis and
Port_Binding->up might be received by northd within the same idl transaction.

Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
---
  tests/ovn-northd.at | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f51e29b11..abb425a8c 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10010,13 +10010,13 @@ ovn_attach n1 br-phys 192.168.0.11
check_recompute_counter() {
      northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats 
northd recompute)
-    AT_CHECK([test x$northd_recomp = x$1])
+    AT_CHECK([test $northd_recomp -le $1])
lflow_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute)
-    AT_CHECK([test x$lflow_recomp = x$2])
+    AT_CHECK([test $lflow_recomp -le $2])
sync_sb_pb_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats sync_to_sb_pb recompute)
-    AT_CHECK([test x$sync_sb_pb_recomp = x$3])
+    AT_CHECK([test $sync_sb_pb_recomp -le $3])
  }
check ovn-nbctl --wait=hv ls-add ls0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to