7 June 2011 — Part rant, part exhortation—the dangers of naively implementing Ruby's inheritance hierarchy callbacks — 2-minute read
I’ve been working more closely with Ruby 1.9 and Rails 3 lately, and while in general it’s been smooth going, there was one particularly disappointing road bump today.
Consider the following code, from a Rails functional test:
1 2 3 4 5 6 7 8 9 10 11 12 |
class SessionControllerTest < ActionController::TestCase tests SessionController # ... end class LoginTest < SessionControllerTest # ... end class LogoutTest < SessionControllerTest # ... end |
This works great with ruby 1.8.7. The tests call sets the SessionController as the controller under test, and the subclasses gain access to that via the “inheritable attributes” feature of ActiveSupport.
Sadly, this does not work in ruby 1.9. Those tests have errors now, saying that the controller instance variable needs to be set in the setup method, because the inheritable attribute of the parent is no longer being inherited.
Some digging and experimenting helped me pare this down to a simpler example:
1 2 3 4 5 6 7 8 9 10 11 |
require 'minitest/unit' require './config/application' class A < MiniTest::Unit::TestCase write_inheritable_attribute "hi", 5 end p A.read_inheritable_attribute("hi") class B < A end p B.read_inheritable_attribute("hi") |
If you run this example with ruby 1.9, it will print “5”, and then “nil”. And the most telling bit: if you comment out the superclass from A’s definition, the program will then print “5” and “5”. It was obviously something that MiniTest was doing.
Another 30 minutes later, I had my answer. MiniTest::Unit::TestCase was defining an inherited callback method, to be invoked every time it was subclassed:
1 2 3 |
def self.inherited klass # :nodoc: @@test_suites[klass] = true end |
ActiveSupport, too, is defining a version of the inherited method, monkeypatching the Class object directly so that subclasses can get a copy of their parent’s inheritable attribute collection. And because MiniTest is descended from Class, MiniTest’s version was overriding ActiveSupport’s version, causing the inheritable attributes to never be copied on inheritance.
Frustrating!
All it takes is the addition of one line—just a single word!—to “fix” MiniTest’s version:
1 2 3 4 |
def self.inherited klass # :nodoc: @@test_suites[klass] = true super # <-- THIS RIGHT HERE end |
You can argue that ActiveSupport shouldn’t be monkeypatching system classes like that. Maybe, maybe not. The point is, it is, and it actually does it in a pretty “good neighbor” way. It saves a reference to any prior “inherited” definition, and calls it from within its version, chaining the calls together.
Sadly, MiniTest’s assumption that it would be the only consumer of the inherited hook in its ancestor chain totally kills ActiveSupport’s attempts at working together. I’ve had to resort to calling the tests helper in each test subclass, explicitly. Not a huge deal, but I’d sure love to have back the two hours I spent debugging this.
The lesson? Always be a good neighbor. Never assume you are the only kid on the playset. Call super when you override a method. Create a call chain when you replace a method in situ. Think of the children!