Open Stack

Hi Matthew,



> -----Original Message-----

> From: Matthew Treinish [mailto:mtreinish at kortar.org]

> Sent: Saturday, May 10, 2014 12:29 AM

> To: OpenStack Development Mailing List (not for usage questions)

> Subject: Re: [openstack-dev] [qa] Checking for return codes in tempest
client

> calls

>

> On Thu, May 08, 2014 at 09:50:03AM -0400, David Kranz wrote:

> > On 05/07/2014 10:48 AM, Ken'ichi Ohmichi wrote:

> > >Hi Sean,

> > >

> > >2014-05-07 23:28 GMT+09:00 Sean Dague <sean at dague.net>:

> > >>On 05/07/2014 10:23 AM, Ken'ichi Ohmichi wrote:

> > >>>Hi David,

> > >>>

> > >>>2014-05-07 22:53 GMT+09:00 David Kranz <dkranz at redhat.com>:

> > >>>>I just looked at a patch https://review.openstack.org/#/c/90310/3

> > >>>>which was given a -1 due to not checking that every call to

> > >>>>list_hosts returns 200. I realized that we don't have a shared

> > >>>>understanding or policy about this. We need to make sure that each

> > >>>>api is tested to return the right response, but many tests need to

> > >>>>call multiple apis in support of the one they are actually

> > >>>>testing. It seems silly to have the caller check the response of

> > >>>>every api call. Currently there are many, if not the majority of,

> > >>>>cases where api calls are made without checking the response code.

> > >>>>I see a few

> > >>>>possibilities:

> > >>>>

> > >>>>1. Move all response code checking to the tempest clients. They

> > >>>>are already checking for failure codes and are now doing

> > >>>>validation of json response and headers as well. Callers would

> > >>>>only do an explicit check if there were multiple success codes

> possible.

> > >>>>

> > >>>>2. Have a clear policy of when callers should check response codes

> > >>>>and apply it.

> > >>>>

> > >>>>I think the first approach has a lot of advantages. Thoughts?

> > >>>Thanks for proposing this, I also prefer the first approach.

> > >>>We will be able to remove a lot of status code checks if going on

> > >>>this direction.

> > >>>It is necessary for bp/nova-api-test-inheritance tasks also.

> > >>>Current https://review.openstack.org/#/c/92536/ removes status code

> > >>>checks because some Nova v2/v3 APIs return different codes and the

> > >>>codes are already checked in client side.

> > >>>

> > >>>but it is necessary to create a lot of patch for covering all API
tests.

> > >>>So for now, I feel it is OK to skip status code checks in API tests

> > >>>only if client side checks are already implemented.

> > >>>After implementing all client validations, we can remove them of

> > >>>API tests.

> > >>Do we still have instances where we want to make a call that we know

> > >>will fail and not through the exception?

> > >>

> > >>I agree there is a certain clarity in putting this down in the rest

> > >>client. I just haven't figured out if it's going to break some

> > >>behavior that we currently expect.

> > >If a server returns unexpected status code, Tempest fails with client

> > >validations like the following sample:

> > >

> > >Traceback (most recent call last):

> > >   File

> > >"/opt/stack/tempest/tempest/api/compute/servers/test_servers.py",

> > >line 36, in test_create_server_with_admin_password

> > >     resp, server = self.create_test_server(adminPass='testpassword')

> > >   File "/opt/stack/tempest/tempest/api/compute/base.py", line 211,

> > >in create_test_server

> > >     name, image_id, flavor, **kwargs)

> > >   File

> >

> >"/opt/stack/tempest/tempest/services/compute/json/servers_client.py",

> > >line 95, in create_server

> > >     self.validate_response(schema.create_server, resp, body)

> > >   File "/opt/stack/tempest/tempest/common/rest_client.py", line 596,

> > >in validate_response

> > >     raise exceptions.InvalidHttpSuccessCode(msg)

> > >InvalidHttpSuccessCode: The success code is different than the

> > >expected one

> > >Details: The status code(202) is different than the expected

> > >one([200])

> > >

> > >

> > >Thanks

> > >Ken'ichi Ohmichi

> > >

> > >_______________________________________________

> > >OpenStack-dev mailing list

> > >OpenStack-dev at lists.openstack.org

> > >http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

> > Note that there are currently two different methods on RestClient that

> > do this sort of thing. Your stacktrace shows "validate_response" which

> > expects to be passed a schema. The other is "expected_success" which

> > takes the expected response code and is only used by the image

> > clients.

> > Both of these will need to stay around since not all APIs have defined

> > schemas but the expected_success method should probably be changed to

> > accept a list of valid success responses rather than just one as it

> > does at present.

>

> So expected_success() is just a better way of doing something like:

>

> assert.Equals(resp.status, 200)

>

> There isn't anything specific about the images clients with it.

> validate_response() should just call expected_success(), which I pushed
out

> here:

> https://review.openstack.org/93035



There can be possibility to have multiple success return code (Nova Server
Ext events API return 200 & 207 as success code). Currently there is no
such API schema but we need to consider this case. In validate_response(),
it was handled and we should expand the expected_success() also for the
same. I have put this in review comment also.



>

>

> >

> > I hope we can get agreement to move response checking to the client.

> > There was no opposition when we started doing this in nova to check

> > schema. Does any one see a reason to not do this? It would both

> > simplify the code and make sure responses are checked in all cases.

> >

> > Sean, do you have a concrete example of what you are concerned about

> > here? Moving the check from the value returned by a client call to

> > inside the client code should not have any visible effect unless the

> > value was actually wrong but not checked by the caller. But this would

> > be a bug that was just found if a test started failing.

> >

>

> Please draft a spec/bp for doing this, we can sort out the implementation

> details in the spec review. There is definitely some overlap with the

> jsonschema work though so we need to think about how to best integrate

> the 2 efforts. For example, for projects that don't use jsonschema yet
does it

> make sense to start using jsonschema files like we do for nova tests to
veriy

> the status codes. Just so we can have a common path for doing this. I
think

> there may be value in doing it that way. We can discuss it more during the

> jsonschema summit session.

>

>

> -Matt Treinish



-- 
Thanks & Regards
Ghanshyam Mann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140510/3b4aedc1/attachment.html>