On 3/22/22 13:35, Ilya Maximets wrote:
> On 3/21/22 17:58, Ilya Maximets wrote:
>> On 3/21/22 15:45, David Marchand wrote:
>>> 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.
>>
>> Might make sense, I'll look into that.
>> Changes in the macro will be heavier though.
> 
> I'm finding myself writing a fairly complex shell code with lots
> of checks for existence of arguments and lots of different cases.
> AT_CHECK is not that simple.
> 
> For now I'm inclined to keep the new macro and just add argument
> validation in v2 for both new and old, so they will explicitly
> fail on misuse.  What do you think?

In the meantime I sent v2 here:
  
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to