On 25 July 2016 at 13:56, Bhor, Dinesh <[email protected]> wrote: > > > -----Original Message----- > From: Sean Dague [mailto:[email protected]] > Sent: Monday, July 25, 2016 5:53 PM > To: [email protected] > Subject: Re: [openstack-dev] [Nova] Remove duplicate code using Data Driven > Tests (DDT) > > On 07/25/2016 08:05 AM, Daniel P. Berrange wrote: >> On Mon, Jul 25, 2016 at 07:57:08AM -0400, Sean Dague wrote: >>> On 07/22/2016 11:30 AM, Daniel P. Berrange wrote: >>>> On Thu, Jul 21, 2016 at 07:03:53AM -0700, Matt Riedemann wrote: >>>>> On 7/21/2016 2:03 AM, Bhor, Dinesh wrote: >>>>> >>>>> I agree that it's not a bug. I also agree that it helps in some >>>>> specific types of tests which are doing some kind of input >>>>> validation (like the patch you've proposed) or are simply iterating >>>>> over some list of values (status values on a server instance for example). >>>>> >>>>> Using DDT in Nova has come up before and one of the concerns was >>>>> hiding details in how the tests are run with a library, and if >>>>> there would be a learning curve. Depending on the usage, I >>>>> personally don't have a problem with it. When I used it in manila >>>>> it took a little getting used to but I was basically just looking >>>>> at existing tests and figuring out what they were doing when adding new >>>>> ones. >>>> >>>> I don't think there's significant learning curve there - the way it >>>> lets you annotate the test methods is pretty easy to understand and >>>> the ddt docs spell it out clearly for newbies. We've far worse >>>> things in our code that create a hard learning curve which people >>>> will hit first :-) >>>> >>>> People have essentially been re-inventing ddt in nova tests already >>>> by defining one helper method and them having multiple tests methods >>>> all calling the same helper with a different dataset. So ddt is just >>>> formalizing what we're already doing in many places, with less code >>>> and greater clarity. >>>> >>>>> I definitely think DDT is easier to use/understand than something >>>>> like testscenarios, which we're already using in Nova. >>>> >>>> Yeah, testscenarios feels little over-engineered for what we want >>>> most of the time. >>> >>> Except, DDT is way less clear (and deterministic) about what's going >>> on with the test name munging. Which means failures are harder to >>> track back to individual tests and data load. So debugging the failures is >>> harder. >> >> I'm not sure what you think is unclear - given an annotated test: >> >> @ddt.data({"foo": "test", "availability_zone": "nova1"}, >> {"name": " test ", "availability_zone": "nova1"}, >> {"name": "", "availability_zone": "nova1"}, >> {"name": "x" * 256, "availability_zone": "nova1"}, >> {"name": "test", "availability_zone": "x" * 256}, >> {"name": "test", "availability_zone": " nova1 "}, >> {"name": "test", "availability_zone": ""}, >> {"name": "test", "availability_zone": "nova1", "foo": "bar"}) >> def test_create_invalid_create_aggregate_data(self, value): >> >> It is generated one test for each data item: >> >> test_create_invalid_create_aggregate_data_1 >> test_create_invalid_create_aggregate_data_2 >> test_create_invalid_create_aggregate_data_3 >> test_create_invalid_create_aggregate_data_4 >> test_create_invalid_create_aggregate_data_5 >> test_create_invalid_create_aggregate_data_6 >> test_create_invalid_create_aggregate_data_7 >> test_create_invalid_create_aggregate_data_8 >> >> This seems about as obvious as you can possibly get > > At least when this was attempted to be introduced into Tempest, the naming > was a lot less clear, maybe it got better. But I still think milestone 3 > isn't the time to start a thing like this. > > -Sean > > Hi Sean, > > IMO it is possible to have a descriptive name to test cases using DDT. > > For ex., > > @ddt.data(annotated('missing_name', {"foo": "test", "availability_zone": > "nova1"}), > annotated('name_greater_than_255_characters', {"name": "x" * 256, > "availability_zone": "nova1"})) > def test_create_invalid_aggregate_data(self, value): > > > it generates following test names: > > test_create_invalid_aggregate_data_2_name_greater_than_255_characters > test_create_invalid_aggregate_data_1_missing_name > > I think with this it is easy to identify which test scenario has failed. > > Same is implemented in openstack/zaqar. > https://github.com/openstack/zaqar/blob/master/zaqar/tests/functional/wsgi/v1/test_queues.py#L87
That descriptive name does help with some of my fears (although I wish it were simpler, maybe just using kwargs in data). I am +1 sdague's not now. I know we normally allow unit test things, but we have such a huge backlog, full of really useful changes we should merge instead, I would rather we all looked at those. Thanks, johnthetubaguy __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
