Nasty Ruby Bug Affecting Test::Unit

Posted by James Mead Sat, 05 Apr 2008 00:10:06 GMT

Introduction

Some time ago, while I was pair-programming with him, Chris alerted me to a Ruby bug he’d come across which was interfering with the diagnosis of a bug in our application. Since then I’ve tried to find out more about it, but couldn’t find much, so I’ve done a bit of investigation and thought I’d post it here in case it’s useful to anyone else. The bug has long since been fixed, but I’m sure there are still people our there with the affected versions of Ruby 1.8.6.

Ruby bug

As far as I understand it, the bug is in Ruby’s Kernel.at_exit hook. A call to Kernel.exit(false) should cause the process to exit with an exit status of 1 indicating the process did not complete successfully. The bug means that calling Kernel.exit(false) from within Kernel.at_exit incorrectly causes the process to exit with an exit status of 0.

The most relevant bug report is #9300 and the most relevant mailing list thread is made up of:- [ruby-core:10746], [ruby-core:10748], [ruby-core:10760].

The fix seems to be in changeset 12126…
r12126 | nobu | 2007-03-23 16:53:42 +0000 (Fri, 23 Mar 2007) | 9 lines

* eval.c (ruby_cleanup): exit by SystemExit and SignalException in END
  block.  [ruby-core:10609]

* test/ruby/test_beginendblock.rb (test_should_propagate_exit_code):
  test for exit in END block.  [ruby-core:10760]

* test/ruby/test_beginendblock.rb (test_should_propagate_signaled):
  test for signal in END block.

Implications for Test::Unit & Rake::TestTask

The bug has some important consequences. Test::Unit makes use of this mechanism to report test failures. Unfortunately, the bug means that a Test::Unit process will always return an exit status of 0 even when there have been test failures.

From test/unit.rb:
at_exit do
  unless $! || Test::Unit.run?
    exit Test::Unit::AutoRunner.run
  end
end

This in turn means that a Rake::TestTask process will also always return an exit status of 0 even when there have been test failures. This is significant, because many continuous integration systems rely on Rake::TestTask processes returning an exit status of 1 to indicate that there have been test failures. Thus you will get false positive passing builds – not good.

Affected versions of Ruby

I’ve built and installed a number of versions of Ruby and run tests on them to try to establish which ones are affected. Although they aren’t comprehensive, here are the results…

affected? version
N ruby 1.8.4 (2005-12-24) [i686-darwin8.10.3]
N ruby 1.8.5 (2006-08-25) [i686-darwin8.10.3]
N ruby 1.8.5 (2007-03-16 patchlevel 37) [i686-darwin8.10.3]
N ruby 1.8.5 (2008-03-03 patchlevel 115) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-02-17 patchlevel 0) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-03-13 patchlevel 0) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-03-16 patchlevel 2) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-03-19 patchlevel 4) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-05-22 patchlevel 5) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-05-22 patchlevel 6) [i686-darwin8.10.3]
Y ruby 1.8.6 (2007-05-22 patchlevel 7) [i686-darwin8.10.3]
N ruby 1.8.6 (2007-05-22 patchlevel 8) [i686-darwin8.10.3]
N ruby 1.8.6 (2007-05-23 patchlevel 9) [i686-darwin8.10.3]
N ruby 1.8.6 (2007-05-23 patchlevel 10) [i686-darwin8.10.3]
N ruby 1.8.6 (2007-08-22 patchlevel 50) [i686-darwin8.10.3]
N ruby 1.9.0 (2007-11-28 patchlevel 0) [i686-darwin8.10.3]

Tags , , , , , , , , ,  | no comments

Mocking in Java using Mocha

Posted by James Mead Sun, 17 Feb 2008 18:10:01 GMT

Ola Bini one of the JRuby guys has released the JtestR tool which allows you to write tests for Java code in Ruby! Ola has bundled a number of Ruby libraries – Mocha, RSpec, Dust, Test::Unit & ActiveSupport – together with JRuby to allow you to write Ruby test cases that test Java code.

He has a couple of examples in the Mock documentation of how to use Mocha...

The first one demonstrates using Mocha to mock an interface (Map).

  import java.util.Map
  import java.util.Iterator
  import java.util.Set
  import java.util.HashMap

  functional_tests do 
    test "that a new HashMap can be created based on another map" do 
      map = Map.new

      map.expects(:size).returns(0)

      iter = Iterator.new
      iter.expects(:hasNext).returns(false)

      set = Set.new
      set.expects(:iterator).returns(iter)

      map.expects(:entrySet).returns(set)

      assert_equals 0, HashMap.new(map).size
    end
  end

The second example demonstrates using Mocha to setup expectations on a real (non-mock) instance (HashMap)...

  import java.util.Iterator
  import java.util.Set
  import java.util.HashMap

  functional_tests do 
    test "that a new HashMap can be created based on another map" do 
      map = mock(HashMap)

      map.expects(:size).returns(0)

      iter = Iterator.new
      iter.expects(:hasNext).returns(false)

      set = Set.new
      set.expects(:iterator).returns(iter)

      map.expects(:entrySet).returns(set)

      assert_equals 0, HashMap.new(map).size
    end
  end

Tags , , , ,  | no comments

Testing tidbits

Posted by James Mead Tue, 25 Sep 2007 22:03:00 GMT

A couple of useful nuggets came up while I was pair programming with James today.

Firstly, James had an interesting new use of the Mocha parameter matcher anything, which reminded me of Joe Walnes and his Flexible JUnit assertions with assertThat().

Normally anything is used to determine what parameters are matched by an expectation…

  Product.expects(:do_stuff).with('name', anything)

But James was passing anything into a method to indicate that the value of that parameter was irrelevant in the context of the test. The kind of thing you might already do using nil...

  Product.do_stuff('name', anything)

I wonder if this idea could be extended by having a kind of holy hand-grenade object which “blows up” by raising an AssertionFailedError when any method is invoked on it…?

Secondly, we were running all our tests using the default rake task when we discovered a test that was hanging. The standard rake test task output wasn’t very helpful in identifying which test was stuck (just rows of dots)...

Started
.....................................................................
.....................................................................
.....................................................................
......................................................

Interrupting the process and examining the stack trace didn’t help either (the relevant stack frames had been collapsed)...

   ... 24 levels...
  from /opt/local/lib/ruby/1.8/test/unit/autorunner.rb:216:in `run'
  from /opt/local/lib/ruby/1.8/test/unit/autorunner.rb:12:in `run'
  from /opt/local/lib/ruby/1.8/test/unit.rb:278
  from /opt/local/lib/ruby/gems/1.8/gems/rake-0.7.3/lib/rake/...

So we set the output to verbose which outputs the name of each test as it starts. From this we could work out which was the offending test…

  TESTOPTS="-v" rake

  test_should_do_something(NaughtyTest): .
  test_should_do_something_else(NaughtyTest): .
  test_should_do_something_ideally_without_hanging(NaughtyTest): .

Tags , , , , , , ,  | 2 comments

Prefer Tests Over Comments

Posted by James Mead Thu, 10 May 2007 15:29:00 GMT

One small a-ha moment in my early programming days was when someone suggested it was better to write a comment explaining why a chunk of code worked the way it did, rather than simply describing how it worked – essentially a direct translation of the code into english sentences.

Since then I’ve become a test-infected, test-driven developer and now I would always choose to write a test in preference to writing a comment. But I think the same lesson still applies…

I’ve recently been making a conscious effort to come up with better test names. The easy option is for the test name to reflect what happens in the test. But it makes the test much more valuable if you can use the test name to explain why the behaviour is the way it is.

Comments are also often used as to-do items, but I think tests can be a better solution. For example, Ben mentions leaving notes for the team in tests.

Tags , , ,  | 1 comment

RSpec adding to the confusion?

Posted by James Mead Tue, 02 Jan 2007 17:39:00 GMT

Update:

I’ve just read Aslak’s article again in the cold light of day. I now realise I have put words in his mouth, for which I apologise.

I posted the article late at night after being annoyed by the line on the caboose blog. In hindsight, I should have waited until morning and read it through again (and then not posted it in it’s current form).

Original Article:

In a recent article Aslak Hellesoy seemed to imply one of the differences between RSpec and Test::Unit was that RSpec you could do interaction-based testing using Mock Objects. I don’t think this implication was intentional, but this lack of clarity isn’t helpful to those just getting to grips with automated testing.

As I’m sure Aslak will acknowledge, interaction-based testing has been around a lot longer than RSpec and BDD. Most Ruby mocking frameworks (e.g. FlexMock, Mocha) were originally designed to work with Test::Unit. And as Nat Pryce recently explained ,”Mock Objects is a Technique Not a Technology”.

There is a similarly confusing statement in a recent post from courtenay (of the caboose).

The counter-intuitive thing for you test::unit types is that you set up the expectations before calling the method.

...thus perpetuating the myth that you can’t do interaction-based testing without RSpec.

Today, Martin Fowler published an update to his excellent article “Mocks Aren’t Stubs”. I think he has added clarity by breaking

the old dichotomy of state based testing and interaction based testing into the state/behavior verification dichotomy and the classical/mockist TDD dichotomy.

However, I fear people might incorrectly associate “behavior verification” with BDD.

In a comment on Aslak’s article, David Chelimsky comments that

It helps at the very least by promoting this very conversation. How does it hurt?

I don’t think it hurts to have a debate (about whether BDD is more usefu than TDD), but we should avoid sowing confusion about what makes BDD different from TDD.

Aslak talks about the “BDD buzzword”. Quoting from Wikipedia...

Buzzwords are typically intended to impress one’s audience with the pretense of knowledge. For this reason, they are often universal. They typically make sentences difficult to dispute, on account of their cloudy meaning.

I vote for more clarity and less cloudy meaning.

Tags , , , , , , , ,  | 9 comments

Tell Don't Ask

Posted by James Mead Sat, 23 Dec 2006 20:04:00 GMT

Following on from a couple of my previous posts, here are some more comments on the Making a Mockery of ActiveRecord article…

Something that may be adding unnecessary complexity to the test is that the Message#generate method knows the structure of objects for which it does not have direct references. For example, it looks like the List and Person instances are supplied, just so that the generate method can navigate to the subscriptions in order to call create. This breaks the Law of Demeter.

A first step to improving the design would be to have a List#create_subscriptions method which would be called by Message#generate. We could then simplify the unit test as follows…

  def test_should_create_email_messages_and_subscriptions_when_include_subscribers_is_true
    list = mock()
    campaign = Campaign.new(:people => [])
    message = Message.new(:campaign => campaign)
    message.stubs(:lists).returns([list])
    message.include_subscribers = true

    list.expects(:create_subscriptions)

    message.generate
    assert_equal 1, message.email_messages
  end

Obviously we’d need to add a separate unit test for the List#create_subscriptions method, but that would also be much simpler and easier to understand. In this way we have decoupled Message from the structure of List and are just relying on its behaviour. Nat Pryce & Steve Freeman call this the Tell Don’t Ask style.

Finally, I prefer not to have much more than one assertion per test (and don’t forget verifying an expectation is effectively an assertion), so I’d probably split the test into two – one to verify create_subscriptions was called and one to check the email_messages were created.

Tags , , , , , ,  | 8 comments

Stub Queries and Expect Commands

Posted by James Mead Sat, 23 Dec 2006 20:02:00 GMT

In Making a Mockery of ActiveRecord, a single test has multiple expectations set up for accessor methods…

  specify "should create EmailMessages and Subscriptions when include_subscribers is true" do
    @message.include_subscribers = true
    @message.should_receive(:lists).and_return([@list])
    @message.should_receive(:campaign).twice.and_return(@campaign)
    @list.should_receive(:people).and_return([@person])
    @person.should_receive(:campaigns).and_return([])

    @person.should_receive(:subscriptions).and_return(@subscriptions)
    @subscriptions.should_receive(:create).and_return(nil)
    @campaign.should_receive(:people).and_return([])

    message.generate # I think this line is missing from the original test
    @message.should_have(1).email_messages
  end

I’ve found Nat Pryce’s rule of thumb – stub queries and expect commands – to be very useful in reducing the brittleness of tests (see Yoga for Your Unit Tests).

A query in this context is a method which does not change the state of the object on which it is called. The accessor methods definitely fall into this category, so I would stub them, not set expectations for them. The one command method in the above example which merits an expectation is the call to create on @subscriptions.

However, I prefer to use in-memory ActiveRecord objects instead of mocks or stubs wherever possible. So I’d write something more like this (using Test::Unit and Mocha)...

  def test_should_create_email_messages_and_subscriptions_when_include_subscribers_is_true
    subscriptions = mock()
    person = Person.new(:campaigns => [])
    person.stubs(:subscriptions).returns(subscriptions)
    list = List.new(:people => [person])
    campaign = Campaign.new(:people => [])
    message = Message.new(:lists => [list], :campaign => campaign)
    message.include_subscribers = true

    subscriptions.expects(:create)

    message.generate
    assert_equal 1, message.email_messages
  end

I haven’t actually run this – so it’s quite likely there are errors in it, but you should get the general idea. The person.stubs line is necessary to avoid the type checking that Luke Redpath mentions in his comment.

More thoughts here.

Tags , , , , , ,  | 1 comment

Fixtures, mock objects or in-memory ActiveRecord objects?

Posted by James Mead Sat, 23 Dec 2006 20:00:00 GMT

Wilson Bilkovich has posted an article about mocking ActiveRecord objects.

A new method mock_model is defined that builds a mock object which will respond the same way as a real ActiveRecord object. As I understand it, this means he can replace…

  @campaign = mock("campaign")
  @campaign.stub!(:is_a?).and_return(true)
  @campaign.stub!(:new_record?).and_return(false)
  @campaign.stub!(:id).and_return(rand(1000))

with…

  mock_model :campaign

Although I agree with him that using fixtures is not a good idea, why not use a real ActiveRecord object…

  @campaign = Campaign.new

Sometimes due to the way ActiveRecord couples your models to the database, it becomes essential to have a model in the database and not just in memory. In which case why not just do this…

  @campaign = Campaign.create!

I’ve written up a couple more thoughts here and here.

Tags , , , , , ,  | 1 comment

Mocha Adoption

Posted by James Mead Fri, 17 Nov 2006 05:52:00 GMT

It’s great to see that so many people are finding Mocha useful. But it’s particularly rewarding to see people recommend it to others…

Tags , , , ,  | 2 comments

Hellish XML

Posted by James Mead Tue, 17 Oct 2006 03:26:00 GMT

So now you can write test cases for ANT in XML using AntUnit.

You can even write assertions in XML

  <!-- the actual test case -->
  <target name="testTouchCreatesFile">
    <au:assertFileDoesntExist file="${foo}"/>
    <touch file="${foo}"/>
    <au:assertFileExists file="${foo}"/>
  </target>

... but why?

Perhaps the time is ripe for an XML mocking library ;-)

Tags , , , , , , ,  | 1 comment

Older posts: 1 2 3