A few weeks ago, I submitted a pull request to the minitest project. Ryan Davis (a.k.a. zenspider) merged my pull request, but also cleaned up the tests I had written. As someone relatively new to Ruby, I found his cleanup very instructive, so I’d like to share it with you. I understood everything he did, but I didn’t think of doing it that way myself. Being a Smalltalker, I understand quite a bit of “the Ruby way”, but Ruby has a number of unique features as well. I feel like I’ve learned the language and its features pretty well, but I still have much to learn about how and when to use those features to full advantage.
MiniTest::Unit, there is a hook message named
record that is
sent to record the results of each test case as it is run. Minitest
itself doesn’t use this method, but subclasses in other projects do.
In my case, I also use
hook into the test runner in RubyMine, and it uses
I found that
record wasn’t being sent when there was a failure or
exception in a
teardown method. When using a mocking framework,
teardown is typically where mock verification is done, and I
found that I wasn’t seeing failure messages.
record didn’t have any test coverage, so I first wrote some tests
around the original behavior of the code, then changed a test to expose
the broken behavior, and finally fixed the bug.
I followed a pattern I saw in another part of the test suite and made a new test suite:
setup, I create a new, anonymous subclass of
record, storing the error into a hash that is keyed
off of the method name. I provide a getter to return the hash
for later use in test assertions. I tell minitest to use my new class
as it’s test runner.
I also wrote a little helper method to run the test case and yield the recording to a block where assertions could be made:
@tu is defined in
MetaMetaMetaTestCase and is the test suite to be
With that, I wrote a number of tests that looked like this:
In each test, I define an anonymous subclass of
TestCase with any
teardown method needed for that test. Then, in a
with_recording block, I make the necessary assertions.
Overall, I didn’t think these tests were too bad; a bit wordier than I liked, but they followed a pattern used elsewhere in the test suite.
Here’s what Ryan refactored to:
I can see the pieces of what I’d written in his refactored version, but the actual tests themselves are so much cleaner. There is no extra noise in those tests.
As part of merging my pull request, he decided to change the semantics
record a bit (for the better), which is why he maps method names
to arrays instead of to single instances, but that’s not relevant
Here are some of the things I find interesting about his solution:
He rolled all of the common boilerplate code into an assertion method, so no
He defined the
recordingmethods as per-instance methods on
@turather than creating a new, anonymous subclass. Understanding what is going on here is one of the keys to understanding Ruby’s object model. This is one of the significant differences between Ruby and Smalltalk. Were I to do this again, I think this technique is a little deeper in the bag of tricks than I’d choose to go in this situation. I’d probably still create a new subclass here. Still, it’s an interesting technique and worth learning and thinking about.
He defines the anonymous subclass of
TestCasein the assertion method, using the block from the test to define the necessary test and
teardownmethods. This is brilliant, in my opinion. I’m not sure I ever would have thought of doing it this way, and yet seeing it, it makes perfect sense.
assert_equalon the class of the recorded exception, which is actually a more accurate test than the
assert_instance_ofthat I’d used.
This was definitely educational for me, and I hope it is for you as well.
Thanks to Ryan Davis for the opportunity to learn from his code, for merging the pull request, and for giving me permission to blog about the experience.
I had already scheduled this post for this week, and over the weekend saw a similar post by Clay Allsopp: How A Pull Request Rocked My World.
I’d never really thought about the educational value of pull requests before. Here are two examples of that, one from the submitter side, and one from the receiver side. Pull requests are a great way to learn about writing better code in a real-world context.