Open Stack

On Mon, Jul 25, 2016 at 08:22:52AM -0400, Sean Dague wrote:
> 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.

Historically we've allowed patches that improve / adapt unit tests
to be merged at any time that we're not in final bug-fix only freeze
periods. So on this basis, I'm happy to see this accepted now, especially
since the module is already in global requirements, so not a new thing
from an openstack POV

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|