On Mon, Jul 19, 2010 at 12:10 PM, Justin Harris jharris@redhat.com wrote:
Upon doing some work in the cucumber tests, it struck me that there were a lot of inconsistencies in the way tests were being put together, as well as confusion in the semantics of a lot of the Given/When/Then blocks. So I have a few proposals to make around cucumber patterns (Feel free to disagree/call me an idiot):
* We drop the usage of the word 'I' entirely - we have too make different actors and types to really know who 'I' is. Is it the user, or the consumer they just registered? If a user registers two consumers, then 'I' can refer to different consumers depending on the context.
This sounds good, aside from the pain that I suspect will be experienced when we try to refactor all those existing regexps to match this new format. (not fun in the past when I took at stab at cleaning up some of the random capitalization)
* Refer to each actor or entity by a name - this name can then look a global hash table for that type to get the reference. Here is an example:
Given an owner "Rackspace" exists And a user "bill" exists under owner "Rackspace" When user "bill" registers a consumer "virt11" Then consumer "virt11" has a type of "system"
The main idea is that the entities are identified more explicitly, which IMO makes the implementation easier as well as the test a bit more readable to understand everything that is involved in the scenario.
* We do not couple the features files to the ruby files under step_definitions - instead I propose that we have a ruby file that deals with a particular entity. So there would be an entitilement.rb, pool.rb, etc. Not entirely sure if this will work very cleanly, but the cucumber author has specifically said that coupling features and step_definitions is an anti-pattern to be avoided, and it makes sense that this practice kind of kills DRY.
Agreed. Feature files for features, step definitions for re-usable code specific to a particular object type. We probably should be more diligent about re-implementing the same thing in multiple step definitions.
* Each step_definition would be responsible for keeping track of entities created on the candlepin instance and will clean up after itself in an After block - these all execute after a test has completed, but I'm not sure if this will introduce ordering issues if a lot of these need to be executed.
This plays into the earlier point about referencing entities by name, we should flesh this out a little more. There needs to be an established way to create objects in "Given" and reference them in "When" and "Then" explicitly. (stuffing into global variables is quite clearly error prone)
So to do this we were talking on IRC about assigning every object we create with a key. So taking your example:
Given an owner "Rackspace" exists
This could result in @owners['Rackspace'] pointing to the struct returned from Candlepin. Most significantly, you could do @owners['Rackspace']['id'].
Or a larger example from IRC:
Given owner "someowner" has a pool "somepool" of quantity 1 for product "someproduct"
This is getting pretty irritatingly verbose, but I at least can tell what's going on and how to pass data around a little bit.
If this system were in place, we end up with a global After block that would know how to iterate what's in those hashes (@owners @users @products @pools @subscriptions) and destroy everything in there.
I think that if we can all get on the same page around some best practices that work for candlepin, it will (hopefully) make writing functional tests less painful for everyone. So let me know if you guys think this is a good idea or not...
As before, my opinion is strongly that we are essentially re-inventing variables and namespaces because cuke has chopped out most everything we've ever known about organizing and re-using test code with classes, inheritance, fixtures, methods, and return values. I still can't identify any group benefiting from those feature definitions (afaik), and as such I really think we should adopt something that's a bit more programmer friendly (rspec?), which can still be used in a manner consistent with BDD.
That said, if we can get some of these practices sorted out it would help a lot, at least for myself. My primary problem each time I start to write cuke tests is locating the code to re-use, figuring out the "right" way to pass data around, and more often than not spotting the many ways to break so many of the step definitions.
We should probably be doing some serious code reviews too, at least until we have some of the cuke practices more established.
All in all good suggestions, I think they'll help quite a bit. I can help with some of the legwork, how can we start getting it done?
Cheers,
Devan