My replies are in-line. On Sun, Jan 27, 2013 at 7:49 PM, John Carr <[email protected]> wrote:
> Actually, the tests for route53/elb are about as good as the other load > balancers and dns drivers. And the branch I mention wouldn't have caught > this particular problem anyway, well at least not at this stage. I didn't > mean to imply that I thought more work was needed before Route53/ELB could > be unleashed. > Yeah, I probably didn't express myself correctly (didn't want to imply that your tests / code is worse than other drivers and tests). Your latest patch fixes the blocking issue in ELB and Route53 driver so it's good to be included in the next release. > > I wanted to be further along before I shared this branch, but as i've gone > and brought it up.. here is what I have been thinking about: > > I started working on a branch to look at the tests for libcloud as a whole > because I think we can do better. I'm starting with ELB because i'm more > familiar with it and because there aren't many load balancer > implementations. > > There were 2 core issues I wanted to address: > > * Generally speaking, project wide the tests actually do a poor job of > validating the parameters we send to the server (i.e. to the mock). I found > a couple of checks here and there, but IMO nowhere near enough (this is why > I brought the branch up). As an example, I can change the rackspace dns > drivers in ways that would break deleting zones and records, but without > breaking the tests. One is simply to change the method from DELETE to PUT, > POST, GET, whatever. Like wise I can break EC2's destroy_node function > without causing the tests to fail (by not actually passing the InstanceId > list into params). These are contrived examples, but it doesn't take much > to break a driver. Indeed, a real world example is that the generic > response refactor broke the ELB driver and at the time I was too confident > in the tests. > I agree, we have been doing a better job with having tests for edge cases lately, but there is still a lot of work to be done here. > > * As an implementor, I find myself unsure if I have implemented the > interfaces properly. Sure the arguments are kind of the same and I can > definitely make my new class work, but that doesn't mean it behaves enough > like the other implementations to not just be a PITA for the libcloud > users. Hypothetically for example, what if one implementation of > DNSDriver.create_record took 'www' and another took 'www.example.com' and > another took 'www.example.com.'. Our tests don't prevent that, but I > think they should strive to. > Great point about the driver contributor point of the view. This should be solved in two ways: 1. Documentation which explains the right inputs and how to contribute compute / dns / storage / lb driver 2. Tests which ensure that the code is correct (like you have said above) > > My goal ATM is to write one "gold standard" test for the load balancer > core features. If a load balancer can't do something another can then we > either need to add a feature flag for it or block it for all load > balancers. If as a new contributor I came to add a new load balancer i > would sub class the core tests and then write a Driver and Mock that makes > those tests pass. This will require mocks that do more that return hard > coded data. > > I've pushed my progress so far to GitHub in the Jc2k/test-proposal branch, > but it's very early on and i've only looked at the ELB mock so far. Next I > will do the same for another load balancer and then merge the common tests > together to create TestBaseLoadBalancer. > Sounds great! > > Obviously if this works, i'd want to push it beyond the load balancers. > But baby steps. > > ETA is 'a while' - very busy atm. > > John > > > On 28 Jan 2013, at 00:52, Tomaz Muraus <[email protected]> wrote: > > What is the ETA for this patch (improved tests)? > > We do our best to avoid half working / broken code in stable releases > which means that depending on the patch EA, we have the following options: > > 1. Wait for the patch, start a new voting thread for 0.12.1 which includes > this patch > > 2. Don't expose ELB driver in the next release > > Both of this options also mean canceling an existing 0.12.0 voting thread > and starting a new thread for a new release (0.12.1) which will include one > of the changes mentioned above. > > In any case, thanks for the heads up. > > P.S. When you are submitting a patch in the future, please try to generate > it against trunk branch (I will manually backport it to 0.12.x). > > On Sun, Jan 27, 2013 at 3:15 PM, John Carr <[email protected]> wrote: > >> I checked out the 0.12 branch, ran the normal tests and (for the drivers >> i submitted) then hacked the example scripts to run against AWS. >> >> Have submitted patches for the bits that fell out. >> >> I have some vastly improved tests for ELB that im working on, but they >> won't be ready for 0.12.0. >> >> John >> >> On 27 Jan 2013, at 17:04, Jaume Devesa <[email protected]> wrote: >> >> > And Abiquo compute driver! ;) >> > >> > +1 >> > >> > >> > On 27 January 2013 08:16, Tomaz Muraus <[email protected]> wrote: >> > >> >> Hello all, >> >> >> >> It has been a while since we released the last version in November. A >> bunch >> >> of changes have piled up in trunk so I decided to create a new release >> >> (0.12.0 - r1439036) which includes many new features, bug-fixes and >> >> improvements. It contains everything from trunk except the "datacenter" >> >> related changes. >> >> >> >> Datacenter changes are partly backward incompatible and still work in >> >> progress so I have decided to exclude them from this release. They >> should >> >> be finished in the near future (yay!) and included in the next major >> >> release. >> >> >> >> I have manually reverted those changes (it was a pretty time consuming >> >> PITA) so I would appreciate if everyone can spend some extra time >> testing >> >> those changes and make sure I haven't broke anything in the drivers >> listed >> >> bellow. I did spend quite a lot of time testing it myself, but it's >> >> possible that I've missed something. >> >> >> >> - Rackspace compute drivers >> >> - CloudFiles storage drivers >> >> - EC2 compute drivers >> >> >> >> Release artifacts can be found at >> >> http://people.apache.org/~tomaz/libcloud/. >> >> >> >> Release highlights: >> >> >> >> - New more efficient generator based approach for iterating over >> paginated >> >> collections. >> >> >> >> - Old ENUM style provided constants have been replaced with a string >> >> version. This allows users to dynamically register new providers using >> new >> >> set_driver method. >> >> >> >> - New generator based method for iterating over containers >> >> (iterate_containers). >> >> >> >> - Support for multipart uploads in the Amazon S3 storage driver. >> >> >> >> - New load balancer driver for Amazon Load Balancing (ELB) service. >> >> >> >> Full change log can be found at >> >> >> >> >> https://svn.apache.org/viewvc/libcloud/tags/0.12.0/CHANGES?revision=r1439036&view=markup >> >> >> >> Please test the release and post your votes. >> >> >> >> +/- 1 >> >> [ ] Release Apache Libcloud 0.12.0 >> >> >> >> Vote closes on Sunday, February 3rd, 2013 at 12:00 PST. >> >> >> >> Thanks, >> >> Tomaz >> >> >> >> > >
