Hi Matthew,
> -----Original Message----- > From: Matthew Treinish [mailto:[email protected]] > 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 <[email protected]>: > > >>On 05/07/2014 10:23 AM, Ken'ichi Ohmichi wrote: > > >>>Hi David, > > >>> > > >>>2014-05-07 22:53 GMT+09:00 David Kranz <[email protected]>: > > >>>>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 > > >[email protected] > > >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
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
