Skip to content

Test Generators, and Other Thoughts on Style.

2011/08/17

For a time, I paired with someone who convinced me that whitespace between example blocks was superflous and so wound up with a bunch of specs like:

it "should define ACTIVITY_LEVELS" do
  Game::ACTIVITY_LEVELS.should == %w{ Low Medium High }
end
it "should define FORMATIONS" do
  Game::FORMATIONS.should == %w{ Circle Lines Scattered None }
end
it "should define LOCATIONS" do
  Game::LOCATIONS.should == %w{ Indoor Outdoor }
end

which now bothers me; whitespace separation between example blocks feels more idiomatic. In fact, someone asked me after seeing those specs “are you in a fight with whitespace?” I am not.

Also, I don’t like “should” in descriptions. Too wishy-washy sounding, and again, superflous. Speaking of superflous, spaces between curly brackets and their contents… they are it.

Really, does reading the first example require that much more effort than the second?

{ :some_key => "some value" }

vs

{:some_key => "some value"}

Call me crazy but it saves two keystrokes and just looks better to me without the spaces. Style call. YMMV.

Another thought on idiomatic tests for equality in Rspec examples suggests using the appropriate “eq” and “eql” matchers instead of the equality operator (“==”) so that’s made its way into muscle memory lately as well.

So, given the above, were I to refactor or rewrite those specs today, they would look like this:

it "defines ACTIVITY_LEVELS" do
  Game::ACTIVITY_LEVELS.should eql(%w{Low Medium High})
end

it "defines FORMATIONS" do
  Game::FORMATIONS.should eql(%w{Circle Lines Scattered None})
end

it "defines LOCATIONS" do
  Game::LOCATIONS.should eql(%w{Indoor Outdoor})
end

Looks a bit cleaner to me, probably good enough.

However, since the description isn’t really giving us any brilliant insights about the behaviour the specs are verifying, those examples could probably be factored into specify blocks:

specify {Game::ACTIVITY_LEVELS.should eql(%w{Low Medium High})}
specify {Game::FORMATIONS.should eql(%w{Circle Lines Scattered None})}
specify {Game::LOCATIONS.should eql(%w{Indoor Outdoor})}

It’s terse, but still pretty readable and obvious to most people who are familiar with Rspec (and we know our tools, right, so that’s not an issue.)

That is probably a reasonable stopping point, providing it makes sense to leave those as explicit examples. However, were we to add a few more examples, it might be sensible to consider rolling them into a test generator. It can be a useful pattern, especially if you are testing the initialization of many attributes for e.g. an API call, or some similar thing.

While this may not be the ideal example, here’s what that might look like in the context of the above specs:

constants = {:activity_levels => %w{Low Medium High},
             :durations => ["5 to 15", "15 plus", "below 5"],
             :formations => %w{Indoor Outdoor}}

constants.each {|key,val| specify {Game.const_get("#{key.upcase}").should eql(val)}}

In this case it actually adds a line, and again these particular specs are simple enough that leaving them as three individual calls to specify is not a huge deal, but imagine you were testing something like the mapping and initialization of an object from the result of an api call;

result_hash = {:some_key => "some value",
               :some_other_key => "some other value",
               :another_key => "another value",
               :yet_another_key => "yet another value",
               :more_key_fun => "more value fun",
               :moar_object => "moar stuff"}

it "maps some_key to some value" do
  SomeApiWrapper.new_from_api_result(result_hash).some_key.should eql("some value")
end

it "maps some_other_key to some other value" do
  SomeApiWrapper.new_from_api_result(result_hash).some_other_key.should eql("some value")
end

… and so on. Again, taking into account that this is a made-up example, consider that explicitly specifying that all 6 of the expected keys are properly initialized would require 18 lines of code, plus whitespace. Yes, you could build the object and assert that the result of the method you’re testing returns an equivalent object, but sometimes that isn’t as easy as it sounds, or you only need to test certain attributes… Point being, if you’re writing specs like those, you might consider using a generator, like this:

result_hash = {:some_key => "some value",
               :some_other_key => "some other value",
               :another_key => "another value",
               :yet_another_key => "yet another value",
               :more_key_fun => "more value fun",
               :moar_object => "moar stuff"}

result_hash.each do |key,value|
  it "maps #{key} to #{value}" do
    SomeApiWrapper.new_from_api_result(result_hash).send("#{some_key}").should eql("some value")
  end
end

And save ourselves some typing, some reading, and some duplicationey stuff. Since we’re kind of on a roll here, let’s save ourselves some method calls too by moving the actual call out of the example:

result_hash = {:some_key => "some value",
               :some_other_key => "some other value",
               :another_key => "another value",
               :yet_another_key => "yet another value",
               :more_key_fun => "more value fun",
               :moar_object => "moar stuff"}

actual_result = SomeApiWrapper.new_from_api_result(result_hash)

result_hash.each do |key,value|
  it "maps #{key} to #{value}" do
    actual_result.send("#{some_key}").should eql("some value")
  end
end

Congratulations. Our spec is now precious few microseconds faster. Imagine if that method were more computationally intensive, however. The longer that method call takes, the more time that one little change saves.

The last thing bothering me about that spec is the questionable value of explicitly specifying that it maps some key to some value. We could probably use the “specify” method and clean things up just that much more… this is DEFINITELY a style issue however, because the following is quite terse and may be difficult for some people to read, or understand what the spec code is really doing:

result_hash = {:some_key => "some value",
               :some_other_key => "some other value",
               :another_key => "another value",
               :yet_another_key => "yet another value",
               :more_key_fun => "more value fun",
               :moar_object => "moar stuff"}

actual_result = SomeApiWrapper.new_from_api_result(result_hash)

result_hash.each do |key,value|
  specify {actual_result.send("#{some_key}").should eql("some value")}
end

That looks pretty clean, from here. All of that said, writing for readability is never bad, some duplication in test code is usually acceptable, and use your discretion about how “clever” or dense it makes sense for your specs to be. As long as you’re writing them, you’re that much better off than someone who isn’t.

Again, allow me to reiterate that these are largely matters of preference and style, and are by no means to be taken as anything but. That said, feel free to share your rants, raves, questions or flame war inciting opinions below.

Advertisements

From → Programming

Leave a Comment

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: