[ovs-dev] tests: Make "check CoPP config" more reliable.

At one point in the test, we expect that the output of `ovn-nbctl
copp-list` will produce:

arp: meter0
igmp: meter0

However, the output is created by walking a hashmap, and the ordering of
the items in the hashmap appears not to always be the same. On many
builds, I see the test fail, with the following error:

./ovn-northd.at:3287: ovn-nbctl copp-list copp0

On Tue, Mar 29, 2022 at 4:18 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> At one point in the test, we expect that the output of `ovn-nbctl
> copp-list` will produce:
>
> arp: meter0
> igmp: meter0
>
> However, the output is created by walking a hashmap, and the ordering of
> the items in the hashmap appears not to always be the same. On many
> builds, I see the test fail, with the following error:
>
> ./ovn-northd.at:3287: ovn-nbctl copp-list copp0
> --- -   2022-03-29 16:01:54.738558818 -0400
> +++ /builddir/build/BUILD/ovn-22.03.0/tests/testsuite.dir/at-groups/897/stdout  2022-03-29 16:01:54.730616276 -0400
> @@ -1,3 +1,3 @@
> -arp: meter0
>  igmp: meter0
> +arp: meter0
>

Hi Mark,

The above few lines seem to be confusing the git to apply this patch properly.

Take a look at this
https://patchwork.ozlabs.org/project/ovn/patch/20220329201800.462820-1-mmichels@redhat.com/

I manually deleted the above few lines and then I was able to apply
the patch locally.

Please fix this issue before applying.

Acked-by: Numan Siddique <numans@ovn.org>

Numan

> The data is correct, but the test is being too strict about the expected
> ordering of the results.
>
> This commit fixes the issue by running the results through `sort` to
> ensure the ordering will be consistent.
>
> Signed-off-by: Mark Michelson <mmichels@redhat.com>
> ---
>  tests/ovn-northd.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 17d4f31b3..8b3e867c7 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3447,7 +3447,7 @@ ls_copp_uuid=$(fetch_column nb:Logical_Switch copp)
>  AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"])
>
>  check ovn-nbctl --wait=hv copp-add $copp_uuid igmp meter0
> -AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
> +AT_CHECK([ovn-nbctl copp-list copp0 | sort], [0], [dnl
>  arp: meter0
>  igmp: meter0
>  ])
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: sha1 information is lacking or useless (builddir/build/BUILD/ovn-22.03.0/tests/testsuite.dir/at-groups/897/stdout).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 tests: Make "check CoPP config" more reliable.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

On 3/29/22 16:38, Numan Siddique wrote:
> On Tue, Mar 29, 2022 at 4:18 PM Mark Michelson <mmichels@redhat.com> wrote:
>>
>> At one point in the test, we expect that the output of `ovn-nbctl
>> copp-list` will produce:
>>
>> arp: meter0
>> igmp: meter0
>>
>> However, the output is created by walking a hashmap, and the ordering of
>> the items in the hashmap appears not to always be the same. On many
>> builds, I see the test fail, with the following error:
>>
>> ./ovn-northd.at:3287: ovn-nbctl copp-list copp0
>> --- -   2022-03-29 16:01:54.738558818 -0400
>> +++ /builddir/build/BUILD/ovn-22.03.0/tests/testsuite.dir/at-groups/897/stdout  2022-03-29 16:01:54.730616276 -0400
>> @@ -1,3 +1,3 @@
>> -arp: meter0
>>   igmp: meter0
>> +arp: meter0
>>
> 
> Hi Mark,
> 
> The above few lines seem to be confusing the git to apply this patch properly.
> 
> Take a look at this
> https://patchwork.ozlabs.org/project/ovn/patch/20220329201800.462820-1-mmichels@redhat.com/
> 
> I manually deleted the above few lines and then I was able to apply
> the patch locally.
> 
> Please fix this issue before applying.
> 
> Acked-by: Numan Siddique <numans@ovn.org>
> 
> Numan

Thanks, Numan. I fixed the commit message and pushed the change to main 
and branch-22.03.

> 
>> The data is correct, but the test is being too strict about the expected
>> ordering of the results.
>>
>> This commit fixes the issue by running the results through `sort` to
>> ensure the ordering will be consistent.
>>
>> Signed-off-by: Mark Michelson <mmichels@redhat.com>
>> ---
>>   tests/ovn-northd.at | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index 17d4f31b3..8b3e867c7 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -3447,7 +3447,7 @@ ls_copp_uuid=$(fetch_column nb:Logical_Switch copp)
>>   AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"])
>>
>>   check ovn-nbctl --wait=hv copp-add $copp_uuid igmp meter0
>> -AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
>> +AT_CHECK([ovn-nbctl copp-list copp0 | sort], [0], [dnl
>>   arp: meter0
>>   igmp: meter0
>>   ])
>> --
>> 2.31.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>