Re: [openstack-dev] [nova] Question about redundant API samples tests for microversions
On 06/20/2016 08:24 AM, Sean Dague wrote: Honestly, I feel like subclassing tests is almost always an anti-pattern. It looks like you are saving code up front, but it massively couples things in ways that become super hard to deal with in the future. Test code doesn't need to be normalized to within an inch of it's life. Amen. -jay __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Question about redundant API samples tests for microversions
On 06/17/2016 04:16 PM, Matt Riedemann wrote: > I was reviewing this today: > > https://review.openstack.org/#/c/326940/ > > And I said to myself, 'self, do we really need to subclass the API > samples functional tests for this microversion given this change doesn't > modify the request/response body, it's only adding paging support?'. > > https://review.openstack.org/#/c/326940/6/nova/tests/functional/api_sample_tests/test_hypervisors.py > > > The only change here is listing hypervisors, and being able to page on > those if the microversion is high enough. So the API samples don't > change at all, they are just running against a different microversion. Agree. If the samples are the same, I think we shouldn't have that extra set of tests, and just test the interesting surface. I think part of the confusion in the code is also that the subclassing to run tests with different scenarios pattern exists a lot of places, and we use testscenarios explicitly other places. > > The same goes for the REST API unit tests really: > > https://review.openstack.org/#/c/326940/6/nova/tests/unit/api/openstack/compute/test_hypervisors.py > > > I'm not sure if the test subclassing is just done like this for new > microversions because it's convenient or if it's because of regression > testing - knowing that we aren't changing a bunch of other REST methods > in the process, so the subclassed tests aren't testing anything > different from the microversion that came before them. > > The thing I don't like about the test subclassing is all of the > redundant testing that goes on, and people might add tests to the parent > class not realizing it's subclassed and thus duplicating test cases with > no functional change. > > Am I just having some Friday crazies? Ultimately this doesn't hurt > anything really but thought I'd ask. Honestly, I feel like subclassing tests is almost always an anti-pattern. It looks like you are saving code up front, but it massively couples things in ways that become super hard to deal with in the future. Test code doesn't need to be normalized to within an inch of it's life. -Sean -- Sean Dague http://dague.net __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Question about redundant API samples tests for microversions
> -Original Message- > From: Matt Riedemann [mailto:mrie...@linux.vnet.ibm.com] > Sent: 18 June 2016 05:16 > To: OpenStack Development Mailing List (not for usage questions) > <openstack-dev@lists.openstack.org> > Subject: [openstack-dev] [nova] Question about redundant API samples > tests for microversions > > I was reviewing this today: > > https://review.openstack.org/#/c/326940/ > > And I said to myself, 'self, do we really need to subclass the API samples > functional tests for this microversion given this change doesn't modify the > request/response body, it's only adding paging support?'. > > https://review.openstack.org/#/c/326940/6/nova/tests/functional/api_sam > ple_tests/test_hypervisors.py > > The only change here is listing hypervisors, and being able to page on those > if > the microversion is high enough. So the API samples don't change at all, they > are just running against a different microversion. > > The same goes for the REST API unit tests really: > > https://review.openstack.org/#/c/326940/6/nova/tests/unit/api/openstack/ > compute/test_hypervisors.py > > I'm not sure if the test subclassing is just done like this for new > microversions > because it's convenient or if it's because of regression testing - knowing > that > we aren't changing a bunch of other REST methods in the process, so the > subclassed tests aren't testing anything different from the microversion that > came before them. > > The thing I don't like about the test subclassing is all of the redundant > testing > that goes on, and people might add tests to the parent class not realizing > it's > subclassed and thus duplicating test cases with no functional change. > > Am I just having some Friday crazies? Ultimately this doesn't hurt anything > really but thought I'd ask. Yes, we should not run redundant test case till we are trying to test the regression in previous microversion or really testing the changes does not reflect in previous version or something. In this case, we should not run the normal hypervisor tests(present in HypervisorsSampleJsonTests) for this new microversion tests (as those does not give real benefit), if we want to tests the pagination in functional tests then it should be separate specific tests. But if we want to tests paging in functional tests, I wonder we end up with having duplicate sample template and sample files. Otherwise I too like to tests those bits in unit tests and then in Tempest (volume pagination tests are well implemented [1]). Tempest has taken care the redundant tests run by mentioning the max_microversion in existing parent test class [2] which make sure tests gets executed once in case of subclass too. Or other way is not to subclass microversion tests class and let is implement and run its own specific tests [3]. Those can be implemented case by case. 1 .. https://github.com/openstack/tempest/blob/master/tempest/api/volume/v2/test_volumes_list.py#L184-L193 2 .. https://github.com/openstack/tempest/blob/master/tempest/api/compute/keypairs/test_keypairs.py#L22 3 .. https://github.com/openstack/tempest/blob/master/tempest/api/compute/admin/test_keypairs_v210.py > > -- > > Thanks, > > Matt Riedemann > > > __ > > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: OpenStack-dev- > requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev Thanks Ghanshyam Mann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Question about redundant API samples tests for microversions
On Fri, Jun 17, 2016, at 04:16 PM, Matt Riedemann wrote: > I was reviewing this today: > > https://review.openstack.org/#/c/326940/ > > And I said to myself, 'self, do we really need to subclass the API > samples functional tests for this microversion given this change doesn't > modify the request/response body, it's only adding paging support?'. > > https://review.openstack.org/#/c/326940/6/nova/tests/functional/api_sample_tests/test_hypervisors.py > > The only change here is listing hypervisors, and being able to page on > those if the microversion is high enough. So the API samples don't > change at all, they are just running against a different microversion. > > The same goes for the REST API unit tests really: > > https://review.openstack.org/#/c/326940/6/nova/tests/unit/api/openstack/compute/test_hypervisors.py > > I'm not sure if the test subclassing is just done like this for new > microversions because it's convenient or if it's because of regression > testing - knowing that we aren't changing a bunch of other REST methods > in the process, so the subclassed tests aren't testing anything > different from the microversion that came before them. > > The thing I don't like about the test subclassing is all of the > redundant testing that goes on, and people might add tests to the parent > class not realizing it's subclassed and thus duplicating test cases with > no functional change. I agree that the naive subclassing is wasteful. I would rather see tests that purposely check the changes not just duplicate things at a different microversion. If there's a concern about regressions I think a better approach would be to have a check against 'latest' which would catch accidental changes, not check unchanged request/reponses against microversions where they aren't intended to change. For my own curiosity I checked the timings and the duplicated tests took about 20 seconds to run. That could quickly add up to the point where a significant amount of time is spent on unnecessary testing. > > Am I just having some Friday crazies? Ultimately this doesn't hurt > anything really but thought I'd ask. > > -- > > Thanks, > > Matt Riedemann > > > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: > openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [nova] Question about redundant API samples tests for microversions
I was reviewing this today: https://review.openstack.org/#/c/326940/ And I said to myself, 'self, do we really need to subclass the API samples functional tests for this microversion given this change doesn't modify the request/response body, it's only adding paging support?'. https://review.openstack.org/#/c/326940/6/nova/tests/functional/api_sample_tests/test_hypervisors.py The only change here is listing hypervisors, and being able to page on those if the microversion is high enough. So the API samples don't change at all, they are just running against a different microversion. The same goes for the REST API unit tests really: https://review.openstack.org/#/c/326940/6/nova/tests/unit/api/openstack/compute/test_hypervisors.py I'm not sure if the test subclassing is just done like this for new microversions because it's convenient or if it's because of regression testing - knowing that we aren't changing a bunch of other REST methods in the process, so the subclassed tests aren't testing anything different from the microversion that came before them. The thing I don't like about the test subclassing is all of the redundant testing that goes on, and people might add tests to the parent class not realizing it's subclassed and thus duplicating test cases with no functional change. Am I just having some Friday crazies? Ultimately this doesn't hurt anything really but thought I'd ask. -- Thanks, Matt Riedemann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
