Sharing the Inheritance Hierarchy

Posted by Jamis on June 07, 2011 @ 03:32 PM

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!

Posted in Essays and Rants

Comments

Have something to add? Click here to leave a comment.

07 Jun 2011

1. Matt Jones said...

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).

2. trans said...

In some cases:

super if defined?(super)

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…

08 Jun 2011

3. David Chelimsky said...

You could just use RSpec :)

But, in case you don’t: https://github.com/seattlerb/minitest/pull/21

4. Jamis said...

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. ;)

5. Erich Timkar said...

I’m guessing he contractually prevented from using Rspec :).

10 Jun 2011

6. Hemant said...

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

11 Jun 2011

7. Jamis said...

@Hemant, that’s probably true, but is beside the point. The point is summarized in the last paragraph of this article.

12 Jul 2011

8. Babil said...

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

: (leave url/email »)
: