Getting Testy: Abstraction
This post is part of an ongoing series about Unit Testing.
Review
In my last post, I talked about testing pure functions using the Coin Changer code kata. Here are the tests we ended up with:
RSpec.describe CoinChanger do
subject(:coin_changer) { CoinChanger.new }
[ # description amount expected
[ "no coins", 0, {} ],
[ "one penny", 1, { 1 => 1 } ],
[ "two pennies", 2, { 1 => 2 } ],
[ "one nickel", 5, { 5 => 1 } ],
[ "one nickel and three pennies", 8, { 5 => 1, 1 => 3 } ],
[ "one dime", 10, { 10 => 1 } ],
[ "a mix of coins", 394, { 100 => 3, 50 => 1, 25 => 1, 10 => 1, 5 => 1, 1 => 4 } ]
].each do |description, amount, expected|
it "returns #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
Handling Different Sets of Coin Denominations
The one feature we haven’t tested yet is the ability to pass in a different set of coin denominations. There are a couple of ways we could do that:
-
Pass the coin denominations directly into the
exchangemethod. This would allowexchangeto remain a pure function that is unaffected by anything outside of itself. However, every caller must then know the set of coin denominations. -
Construct the
CoinChangeron the list of coin denominations and then use them from within theexchangemethod. This means that callers only need aCoinChangerand don’t need to know the particular coin denominations. However, we would need to create a newCoinChangerevery time we wanted to use a new set of coin denominations.
If we were using this kata for practice, we’d try each of the options
in turn and see how the design works out either way. In real-world
code, we’d look at how the rest of the application is likely to use
the CoinChanger and make a decision based on the wider context.
If we choose option #1, the tests are relatively easy. We just add a
coins column to our test table above and add new rows to cover the
cases of interest.
Option #2 introduces some interesting issues, so let’s go that direction.
I’m pretty confident that the code we’ve written so far will handle other sets of denominations that have similar properties to the USD coins we’ve been using so far. However, there are two special cases I can think of:
-
Canada has eliminated the penny from circulation; thus, we need to round to the nearest nickel when calculating change.
-
There are possible sets of coin denominations that will cause our implementation to fail if we’re not careful. For example, if we have coin denominations of 10, 7, and 1 and we want to make change for 14, we should return two 7-cent coins, not a 10 and four 1’s. Remember that we are supposed to return the fewest number of coins. A typical algorithm will return too many coins.
Here are the tests we might end up with after handling these cases:
RSpec.describe CoinChanger do
describe "American coins" do
subject(:coin_changer) { CoinChanger.new([100, 50, 25, 10, 5, 1]) }
[ # description amount expected
[ "no coins", 0, {} ],
[ "one penny", 1, { 1 => 1 } ],
[ "two pennies", 2, { 1 => 2 } ],
[ "one nickel", 5, { 5 => 1 } ],
[ "one nickel and three pennies", 8, { 5 => 1, 1 => 3 } ],
[ "one dime", 10, { 10 => 1 } ],
[ "a mix of coins", 394, { 100 => 3, 50 => 1, 25 => 1, 10 => 1, 5 => 1, 1 => 4 } ]
].each do |description, amount, expected|
it "returns #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
describe "Rounding to the smallest denomination" do
subject(:coin_changer) { CoinChanger.new([10, 5]) }
[ # description amount expected
[ "one nickel", 7, { 5 => 1 } ],
[ "one dime", 8, { 10 => 1 } ]
].each do |description, amount, expected|
it "rounds to #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
describe "Minimizing the number of coins" do
subject(:coin_changer) { CoinChanger.new([10, 7, 1]) }
[ # description amount expected
[ "one dime and three pennies", 13, { 10 => 1, 1 => 3 } ],
[ "two seven-cent pieces", 14, { 7 => 2 } ],
[ "two seven-cent pieces and two pennies", 16, { 7 => 2, 1 => 2 } ],
[ "one dime and one seven-cent piece", 17, { 10 => 1, 7 => 1 } ]
].each do |description, amount, expected|
it "returns #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
end
These specs are quite reasonable, and many people would suggest leaving them as-is. But there are two bits of duplication and a missed opportunity for communication that I’d like to consider.
Extracting the Test Blocks
Notice that each of the three describe blocks is structured exactly
the same. There’s an array of test cases followed by a block that
defines it blocks for each test case. The it blocks are nearly
identical. In the middle describe block, the description is slightly
different than in the other two cases.
There are a couple of ways we could eliminate the duplicated it
blocks. First, we could extract a method and pass in the array of
test cases:
RSpec.describe CoinChanger do
def self.generate_specs(cases)
cases.each do |description, amount, expected|
it "returns #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
describe "American coins" do
subject(:coin_changer) { CoinChanger.new([100, 50, 25, 10, 5, 1]) }
generate_specs(
[ # description amount expected
[ "no coins", 0, {} ],
[ "one penny", 1, { 1 => 1 } ],
[ "two pennies", 2, { 1 => 2 } ],
[ "one nickel", 5, { 5 => 1 } ],
[ "one nickel and three pennies", 8, { 5 => 1, 1 => 3 } ],
[ "one dime", 10, { 10 => 1 } ],
[ "a mix of coins", 394, { 100 => 3, 50 => 1, 25 => 1, 10 => 1, 5 => 1, 1 => 4 } ]
])
end
describe "Rounding to the smallest denomination" do
subject(:coin_changer) { CoinChanger.new([10, 5]) }
generate_specs(
[ # description amount expected
[ "one nickel", 7, { 5 => 1 } ],
[ "one dime", 8, { 10 => 1 } ]
])
end
describe "Minimizing the number of coins" do
subject(:coin_changer) { CoinChanger.new([10, 7, 1]) }
generate_specs(
[ # description amount expected
[ "one dime and three pennies", 13, { 10 => 1, 1 => 3 } ],
[ "two seven-cent pieces", 14, { 7 => 2 } ],
[ "two seven-cent pieces and two pennies", 16, { 7 => 2, 1 => 2 } ],
[ "one dime and one seven-cent piece", 17, { 10 => 1, 7 => 1 } ]
])
end
end
Second, we could extract the test block into a method returning a lambda:
RSpec.describe CoinChanger do
def self.test_block
-> (description, amount, expected) do
it "returns #{description} for #{amount} cents" do
expect(coin_changer.exchange(amount)).to eq(expected)
end
end
end
describe "American coins" do
subject(:coin_changer) { CoinChanger.new([100, 50, 25, 10, 5, 1]) }
[ # description amount expected
[ "no coins", 0, {} ],
[ "one penny", 1, { 1 => 1 } ],
[ "two pennies", 2, { 1 => 2 } ],
[ "one nickel", 5, { 5 => 1 } ],
[ "one nickel and three pennies", 8, { 5 => 1, 1 => 3 } ],
[ "one dime", 10, { 10 => 1 } ],
[ "a mix of coins", 394, { 100 => 3, 50 => 1, 25 => 1, 10 => 1, 5 => 1, 1 => 4 } ]
].each(&test_block)
end
describe "Rounding to the smallest denomination" do
subject(:coin_changer) { CoinChanger.new([10, 5]) }
[ # description amount expected
[ "one nickel", 7, { 5 => 1 } ],
[ "one dime", 8, { 10 => 1 } ]
].each(&test_block)
end
describe "Minimizing the number of coins" do
subject(:coin_changer) { CoinChanger.new([10, 7, 1]) }
[ # description amount expected
[ "one dime and three pennies", 13, { 10 => 1, 1 => 3 } ],
[ "two seven-cent pieces", 14, { 7 => 2 } ],
[ "two seven-cent pieces and two pennies", 16, { 7 => 2, 1 => 2 } ],
[ "one dime and one seven-cent piece", 17, { 10 => 1, 7 => 1 } ]
].each(&test_block)
end
end
Both of these work and both eliminate the duplication. Notice that
we lost the somewhat customized messages for the middle describe
block. Given that the key concept (rounding) is communicated by the
describe, that’s not too big a loss and I’m willing to live with it.
It is unfortunate that both methods require the self. prefix but,
since we’re executing the methods outside the context of an it
block, we need to do that. Similarly, the methods have to be at the
top of the spec so that they’re available when RSpec evaluates the
calls to the methods. I’d have preferred to hide them away at the
bottom.
Overall, I prefer the second approach (extracting the test block)
because it keeps the test case table front-and-center in each test.
That’s where I want the reader’s focus; the test block shows up as
almost an afterthought, and that’s exactly what I want. In the first
approach, the reader encounters the generate_specs call first and
has to start thinking about that while trying to process the test case
table.
By extracting the duplication, we’ve had to get a little more clever with the code. This can be a disadvantage. In this case, I think it makes the tests more readable. I don’t have to study the three duplicate test blocks to see if there’s anything special about any of them; I know they’re identical. I have one place to look to see details about how the test cases are executed, and one place to change if I later refactor the code.
All in all, I think this extraction is a net win.
Extracting the Coin Denominations
The second bit of duplication is in the subject blocks. We are
creating a CoinChanger three different times. Since we’re just
calling new, it doesn’t feel like serious duplication but if we ever
wanted to change the way we create a CoinChanger, there are three
places to change.
These subject blocks are also missing an opportunity to communicate
a little bit more information to the reader. What is different about
the three top-level describe blocks? It’s really just the
denominations of the coins. That difference is somewhat obscured by
the extra noise of the CoinChanger.new calls.
Let’s see about fixing both problems:
RSpec.describe CoinChanger do
subject(:coin_changer) { CoinChanger.new(coins) }
def self.test_block
-> (description, amount, expected) do
it "returns #{description} for #{amount} cents" do
expect(coin_changer.change_for(amount)).to eq(expected)
end
end
end
describe "American coins" do
let(:coins) { [100, 50, 25, 10, 5, 1] }
[ # description amount expected
[ "no coins", 0, {} ],
[ "one penny", 1, { 1 => 1 } ],
[ "two pennies", 2, { 1 => 2 } ],
[ "one nickel", 5, { 5 => 1 } ],
[ "one nickel and three pennies", 8, { 5 => 1, 1 => 3 } ],
[ "one dime", 10, { 10 => 1 } ],
[ "a mix of coins", 394, { 100 => 3, 50 => 1, 25 => 1, 10 => 1, 5 => 1, 1 => 4 } ]
].each(&test_block)
end
describe "Rounding to the smallest denomination" do
let(:coins) { [10, 5] }
[ # description amount expected
[ "one nickel", 7, { 5 => 1 } ],
[ "one dime", 8, { 10 => 1 } ]
].each(&test_block)
end
describe "Minimizing the number of coins" do
let(:coins) { [10, 7, 1] }
[ # description amount expected
[ "one dime and three pennies", 13, { 10 => 1, 1 => 3 } ],
[ "two seven-cent pieces", 14, { 7 => 2 } ],
[ "two seven-cent pieces and two pennies", 16, { 7 => 2, 1 => 2 } ],
[ "one dime and one seven-cent piece", 17, { 10 => 1, 7 => 1 } ]
].each(&test_block)
end
end
Now there is one subject defined for the entire test suite, and a new
instance is constructed for each individual spec using the coins
specified for that spec’s containing describe block. The
let(:coins) definition at the top of each describe block
communicates exactly what’s special about that context.
On the other hand, the subject definition at the top of the spec is
a bit confusing, as it refers to coins and we have no clue where
that’s coming from. It feels a bit like “spooky action at a
distance.” The bigger the spec grows, the more of a problem this is.
I might argue that a spec that is too big to handle this approach
might be testing a class that has too many responsibilities and that I
should focus my efforts on that problem instead.
Conclusion
As I mentioned earlier, there are many people who would argue that these new abstractions are not worth it. They’d argue that it makes the tests harder to understand at a glance.
If I’ve chosen the wrong abstractions, then they have a very valid point. When creating abstractions in any code, it’s important to choose abstractions that are helpful; many times, we choose the wrong ones and it makes our code more confusing to those coming after us.
I think that the abstractions I’ve chosen help to communicate the important parts of the tests while making the less important details a little bit harder to access.
I really want my tests to focus on what I’m testing, not how I’m testing it. So if I can make the “what” front-and-center and abstract away the “how”, I think it makes the tests easier to read and reason about.
Getting rid of some duplication is a nice side-benefit, but should not be the main goal. Communication should be the goal, and duplication removal can often be a way of achieving that communication.
What do you think about these abstractions? Do they help? Or have I gone too far?