Re: [vpp-dev] ACL Build/Test Issues

[Klement Sekera -X (ksekera - PANTHEON TECHNOLOGIES at Cisco)]

Quoting Jon Loeliger (2017-11-10 23:11:36)

>First, this is draconian for no really good reason.  Second, it should be
>fixed.  Third, I would do that except I am stupid and need a clue where
>or how to fix this situation so the tests are less draconian.  (Can we
>get a "less than 0" test instead of "equal to -1"?)

When writing the logic, it was sufficient (so far) for everybody to just
know what is the correct value to expect (0, -1, ..). If there really is
a situation when an API call can have a random (from the client's view)
return value, I can think of two approaches to deal with this...

1.) introduce a special case "expected_retval=None" which just skips
verifying the return value (in framework) leaving this up to the caller
2.) allow passing a functor instead of the expected value, which
fulfills the role of validator and have the call it, supplying the actual
"retval" from API response message as a parameter to the functor to figure
out whether it's one of the expected ones or nor ...

Thanks,
Klement
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] ACL Build/Test Issues

[Jon Loeliger]

Chris,

On Fri, Nov 10, 2017 at 8:27 PM, Luke, Chris  wrote:

> If you’re wondering where the tests are:
>
>
>
> $ ls test/*acl*
>
> test/test_acl_plugin_conns.py  test/test_acl_plugin_macip.py
>
> test/test_acl_plugin_l2l3.py   test/test_acl_plugin.py
>

Ah, excellent!


> Chris.
>

Thanks!

jdl
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] ACL Build/Test Issues

[Luke, Chris]

Since the tests reside in the same repo and they’re run from the checkout copy 
of the patch, you should be able to simply update the tests in the same patch 
that changes the result codes, without need for backward compatibility.

If you’re wondering where the tests are:

$ ls test/*acl*
test/test_acl_plugin_conns.py  test/test_acl_plugin_macip.py
test/test_acl_plugin_l2l3.py   test/test_acl_plugin.py

Chris.

From: vpp-dev-boun...@lists.fd.io [mailto:vpp-dev-boun...@lists.fd.io] On 
Behalf Of Jon Loeliger
Sent: Friday, November 10, 2017 19:17
To: Andrew Yourtchenko <ayour...@gmail.com>
Cc: vpp-dev <vpp-dev@lists.fd.io>
Subject: Re: [vpp-dev] ACL Build/Test Issues

On Fri, Nov 10, 2017 at 5:54 PM, Andrew Yourtchenko 
<ayour...@gmail.com<mailto:ayour...@gmail.com>> wrote:
Hi Jon,

On 10 Nov 2017, at 23:11, Jon Loeliger 
<j...@netgate.com<mailto:j...@netgate.com>> wrote:
Folks,

Every error from the ACL implementation is -1.  Generically bad.
Without regard for what might be more useful to an upper-layer UI.

When we discussed with the openstack folks the way they are treating errors was 
all as catastrophic, but yes more distinction would be better, so thanks a lot 
for taking care of it!

Happy to try to help. :-)

So I submitted a patch to help this situation some.
https://gerrit.fd.io/r/#/c/9383/

I have built and tested it locally, but it fails the Verify Tests because
it has a test that is expecting a hard-coded -1 return from some tests.
Returning a -6 wasn't good enough.

First, this is draconian for no really good reason.  Second, it should be
fixed.  Third, I would do that except I am stupid and need a clue where
or how to fix this situation so the tests are less draconian.  (Can we
get a "less than 0" test instead of "equal to -1"?)

Yeah. So we would need to first submit new test(s) that pass on both current 
and new code and then the new code itself... wanna take a shot at it  or should 
I ?

I don't even know where I would begin on that front, except
to say the test should maybe be " actual < 0" for now.
Oh, and, it's pretty clear why the Verify failed, despite the Jenkins job
saying it was unable to determine a cause.  Um, double blerg.

Alas, this one I leave to someone else to comment on :)

I hear that!

jdl

___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] ACL Build/Test Issues

[Jon Loeliger]

On Fri, Nov 10, 2017 at 5:54 PM, Andrew Yourtchenko 
wrote:

> Hi Jon,
>
> On 10 Nov 2017, at 23:11, Jon Loeliger  wrote:
>
> Folks,
>
> Every error from the ACL implementation is -1.  Generically bad.
> Without regard for what might be more useful to an upper-layer UI.
>
>
> When we discussed with the openstack folks the way they are treating
> errors was all as catastrophic, but yes more distinction would be better,
> so thanks a lot for taking care of it!
>

Happy to try to help. :-)

So I submitted a patch to help this situation some.
> https://gerrit.fd.io/r/#/c/9383/
>
> I have built and tested it locally, but it fails the Verify Tests because
> it has a test that is expecting a hard-coded -1 return from some tests.
> Returning a -6 wasn't good enough.
>
> First, this is draconian for no really good reason.  Second, it should be
> fixed.  Third, I would do that except I am stupid and need a clue where
> or how to fix this situation so the tests are less draconian.  (Can we
> get a "less than 0" test instead of "equal to -1"?)
>
>
> Yeah. So we would need to first submit new test(s) that pass on both
> current and new code and then the new code itself... wanna take a shot at
> it  or should I ?
>

I don't even know where I would begin on that front, except
to say the test should maybe be " actual < 0" for now.

> Oh, and, it's pretty clear why the Verify failed, despite the Jenkins job
> saying it was unable to determine a cause.  Um, double blerg.
>
>
> Alas, this one I leave to someone else to comment on :)
>

I hear that!

jdl
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

Re: [vpp-dev] ACL Build/Test Issues

[Andrew Yourtchenko]

Hi Jon,

> On 10 Nov 2017, at 23:11, Jon Loeliger  wrote:
> 
> Folks,
> 
> Every error from the ACL implementation is -1.  Generically bad.
> Without regard for what might be more useful to an upper-layer UI.

When we discussed with the openstack folks the way they are treating errors was 
all as catastrophic, but yes more distinction would be better, so thanks a lot 
for taking care of it!

> 
> So I submitted a patch to help this situation some.
> https://gerrit.fd.io/r/#/c/9383/
> 
> I have built and tested it locally, but it fails the Verify Tests because
> it has a test that is expecting a hard-coded -1 return from some tests.
> Returning a -6 wasn't good enough.
> 
> First, this is draconian for no really good reason.  Second, it should be
> fixed.  Third, I would do that except I am stupid and need a clue where
> or how to fix this situation so the tests are less draconian.  (Can we
> get a "less than 0" test instead of "equal to -1"?)

Yeah. So we would need to first submit new test(s) that pass on both current 
and new code and then the new code itself... wanna take a shot at it  or should 
I ?

> 
> Oh, and, it's pretty clear why the Verify failed, despite the Jenkins job
> saying it was unable to determine a cause.  Um, double blerg.

Alas, this one I leave to someone else to comment on :)

—a


> 
> Any help for the weary here?
> 
> Thanks,
> jdl
> 
> 
> 21:21:43 ERROR: ACL create/delete test
> 21:21:43 
> --
> 21:21:43 Traceback (most recent call last):
> 21:21:43   File 
> "/w/workspace/vpp-verify-master-ubuntu1604/test/test_acl_plugin.py", line 
> 563, in test_0001_acl_create
> 21:21:43 tag=":", expected_retval=-1)
> 21:21:43   File 
> "/w/workspace/vpp-verify-master-ubuntu1604/test/vpp_papi_provider.py", line 
> 2483, in acl_add_replace
> 21:21:43 expected_retval=expected_retval)
> 21:21:43   File 
> "/w/workspace/vpp-verify-master-ubuntu1604/test/vpp_papi_provider.py", line 
> 167, in api
> 21:21:43 raise UnexpectedApiReturnValueError(msg)
> 21:21:43 UnexpectedApiReturnValueError: API call failed, expected -1 return 
> value instead of -6 in acl_add_replace_reply(_0=936, context=33, 
> acl_index=432, retval=-6)
> 21:21:43 
> ___
> vpp-dev mailing list
> vpp-dev@lists.fd.io
> https://lists.fd.io/mailman/listinfo/vpp-dev
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev

[vpp-dev] ACL Build/Test Issues

[Jon Loeliger]

Folks,

Every error from the ACL implementation is -1.  Generically bad.
Without regard for what might be more useful to an upper-layer UI.

So I submitted a patch to help this situation some.
https://gerrit.fd.io/r/#/c/9383/

I have built and tested it locally, but it fails the Verify Tests because
it has a test that is expecting a hard-coded -1 return from some tests.
Returning a -6 wasn't good enough.

First, this is draconian for no really good reason.  Second, it should be
fixed.  Third, I would do that except I am stupid and need a clue where
or how to fix this situation so the tests are less draconian.  (Can we
get a "less than 0" test instead of "equal to -1"?)

Oh, and, it's pretty clear why the Verify failed, despite the Jenkins job
saying it was unable to determine a cause.  Um, double blerg.

Any help for the weary here?

Thanks,
jdl


*21:21:43* ERROR: ACL create/delete test*21:21:43*
--*21:21:43*
Traceback (most recent call last):*21:21:43*   File
"/w/workspace/vpp-verify-master-ubuntu1604/test/test_acl_plugin.py",
line 563, in test_0001_acl_create*21:21:43* tag=":",
expected_retval=-1)*21:21:43*   File
"/w/workspace/vpp-verify-master-ubuntu1604/test/vpp_papi_provider.py",
line 2483, in acl_add_replace*21:21:43*
expected_retval=expected_retval)*21:21:43*   File
"/w/workspace/vpp-verify-master-ubuntu1604/test/vpp_papi_provider.py",
line 167, in api*21:21:43* raise
UnexpectedApiReturnValueError(msg)*21:21:43*
UnexpectedApiReturnValueError: API call failed, expected -1 return
value instead of -6 in acl_add_replace_reply(_0=936, context=33,
acl_index=432, retval=-6)*21:21:43*
___
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev