diff mbox

[ovs-dev] tests: add documentation for OVS_WAIT_UNTIL and OVS_WAIT_WHILE macros

Message ID 1446595473-11465-1-git-send-email-aatteka@nicira.com
State Changes Requested
Headers show

Commit Message

Ansis Atteka Nov. 4, 2015, 12:04 a.m. UTC
It is very easy to misuse these macros, because when the COMMAND
returns exit code "0" it is actually considered as if condition
evaluated to "true" and not "false" as some might think.

This patch ensures that this is clearly reflected in documentation.

Signed-off-by: Ansis Atteka <aatteka@nicira.com>
---
 tests/ovs-macros.at | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Ben Pfaff Nov. 4, 2015, 12:07 a.m. UTC | #1
On Tue, Nov 03, 2015 at 04:04:33PM -0800, Ansis Atteka wrote:
> It is very easy to misuse these macros, because when the COMMAND
> returns exit code "0" it is actually considered as if condition
> evaluated to "true" and not "false" as some might think.
> 
> This patch ensures that this is clearly reflected in documentation.
> 
> Signed-off-by: Ansis Atteka <aatteka@nicira.com>

Thanks a lot for improving the documentation!

I think that this documentation is reversed.  That is, OVS_WAIT_UNTIL
waits until the command returns success, that is, an exit code of zero,
and OVS_WAIT_WHILE waits until the command returns failure, that is, a
nonzero exit code.

Also, s/Exectues/Executes/.

> ---
>  tests/ovs-macros.at | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 541b042..e9856b6 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -103,7 +103,21 @@ m4_define([OVS_WAIT],
>       [ovs_wait_cond () { $1
>  }
>  ovs_wait], [0], [ignore], [ignore], [$2])])
> +
> +dnl OVS_WAIT_UNTIL(COMMAND)
> +dnl
> +dnl Exectues shell COMMAND in a loop until it returns
> +dnl non-zero return code.  If COMMAND did not return
> +dnl non-zero code within reasonable time limit, then
> +dnl the test fails.
>  m4_define([OVS_WAIT_UNTIL], [OVS_WAIT([$1], [$2])])
> +
> +dnl OVS_WAIT_WHILE(COMMAND)
> +dnl
> +dnl Exectues shell COMMAND in a loop until it returns
> +dnl zero return code.  If COMMAND did not return
> +dnl zero code within reasonable time limit, then
> +dnl the test fails.
>  m4_define([OVS_WAIT_WHILE],
>    [OVS_WAIT([if $1; then return 1; else return 0; fi], [$2])])
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Ansis Atteka Nov. 4, 2015, 12:15 a.m. UTC | #2
On Tue, Nov 3, 2015 at 4:07 PM, Ben Pfaff <blp@nicira.com> wrote:
> On Tue, Nov 03, 2015 at 04:04:33PM -0800, Ansis Atteka wrote:
>> It is very easy to misuse these macros, because when the COMMAND
>> returns exit code "0" it is actually considered as if condition
>> evaluated to "true" and not "false" as some might think.
>>
>> This patch ensures that this is clearly reflected in documentation.
>>
>> Signed-off-by: Ansis Atteka <aatteka@nicira.com>
>
> Thanks a lot for improving the documentation!
>
> I think that this documentation is reversed.  That is, OVS_WAIT_UNTIL
> waits until the command returns success, that is, an exit code of zero,
> and OVS_WAIT_WHILE waits until the command returns failure, that is, a
> nonzero exit code.

Thanks. Not sure what I was thinking about since the documentations is
indeed reversed for both macros. Will send V2.



>
> Also, s/Exectues/Executes/.
ok
>
>> ---
>>  tests/ovs-macros.at | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
>> index 541b042..e9856b6 100644
>> --- a/tests/ovs-macros.at
>> +++ b/tests/ovs-macros.at
>> @@ -103,7 +103,21 @@ m4_define([OVS_WAIT],
>>       [ovs_wait_cond () { $1
>>  }
>>  ovs_wait], [0], [ignore], [ignore], [$2])])
>> +
>> +dnl OVS_WAIT_UNTIL(COMMAND)
>> +dnl
>> +dnl Exectues shell COMMAND in a loop until it returns
>> +dnl non-zero return code.  If COMMAND did not return
>> +dnl non-zero code within reasonable time limit, then
>> +dnl the test fails.
>>  m4_define([OVS_WAIT_UNTIL], [OVS_WAIT([$1], [$2])])
>> +
>> +dnl OVS_WAIT_WHILE(COMMAND)
>> +dnl
>> +dnl Exectues shell COMMAND in a loop until it returns
>> +dnl zero return code.  If COMMAND did not return
>> +dnl zero code within reasonable time limit, then
>> +dnl the test fails.
>>  m4_define([OVS_WAIT_WHILE],
>>    [OVS_WAIT([if $1; then return 1; else return 0; fi], [$2])])
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 541b042..e9856b6 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -103,7 +103,21 @@  m4_define([OVS_WAIT],
      [ovs_wait_cond () { $1
 }
 ovs_wait], [0], [ignore], [ignore], [$2])])
+
+dnl OVS_WAIT_UNTIL(COMMAND)
+dnl
+dnl Exectues shell COMMAND in a loop until it returns
+dnl non-zero return code.  If COMMAND did not return
+dnl non-zero code within reasonable time limit, then
+dnl the test fails.
 m4_define([OVS_WAIT_UNTIL], [OVS_WAIT([$1], [$2])])
+
+dnl OVS_WAIT_WHILE(COMMAND)
+dnl
+dnl Exectues shell COMMAND in a loop until it returns
+dnl zero return code.  If COMMAND did not return
+dnl zero code within reasonable time limit, then
+dnl the test fails.
 m4_define([OVS_WAIT_WHILE],
   [OVS_WAIT([if $1; then return 1; else return 0; fi], [$2])])