Sharing the Inheritance Hierarchy
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!
Reader Comments
This is even MOAR fun when it’s libraries hooking into inherited on ActiveRecord::Base – some really bizarro things happen (notably because AR loses track of its subclasses).
7 Jun 2011
In some cases:
And thank you! You reminded me that I have to fix this in one of my projects too. Now if I could just remember where it was…
7 Jun 2011
You could just use RSpec :)
But, in case you don’t: https://github.com/seattlerb/minitest/pull/21
8 Jun 2011
David, point taken: thanks for doing what I ought to have done first! I really didn’t mean to complain about about minitest (well okay, maybe I did a little, two hours of lost time was a bit frustrating), mostly I just wanted to “raise awareness” of the anti-pattern. But my point would have been stronger if I’d included a patch. Thanks for fixing that for me. ;)
8 Jun 2011
I’m guessing he contractually prevented from using Rspec :).
8 Jun 2011
But since write_inheritable_attribute and read_inheritable_attribute has been deprecated in favour of class_attribute methods, prolly you are better off, not using them at all? :P
10 Jun 2011
@Hemant, that’s probably true, but is beside the point. The point is summarized in the last paragraph of this article.
11 Jun 2011
Ehm, I am very sorry about the OT but I can’t find a proper section to post this message in your blog… Due to an hardware failure in my computer, I lost your wonderful dungeon generator program. The web page to download it seems to be broken. I have seen that your dungeon generator is nicely implemented in another website, but I would like to have an off-site version to carry around with my laptop. There is any place where can I download your dungeon generator? I would be happy even with the old third edition version… Thank you very much
12 Jul 2011