Floehopper

Nasty Ruby Bug Affecting Test::Unit

2008-04-05T00:10:06+00:00

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]`

Prefer Commit Notes over Comments

2008-03-29T19:05:00+00:00

My colleague, Chris, recently posted about what makes a good commit note. His main point is that a good commit note should explain why the changeset exists rather than what it contains (which should be more self-evident). I agree with this and (as Chris mentions) it’s of particular benefit when you have to do some software archeology. I’d go a step further and say that, for me, the best commit notes express the business reason for the changeset. If as a developer you are struggling to explain the business case for a particular change, (imho) you should try to find out before committing – otherwise how do you know the changeset is delivering value to the business?

In a previous post about preferring tests over comments, I expressed similar sentiments about using comments and tests to explain why rather than what. Chris’ post prompted me to re-read that old post and I noticed that it didn’t explain why I prefer tests over comments. The reason is that comments have a nasty habit of becoming out-of-date and getting left lying around to confuse the unwary, whereas you are forced to keep tests up-to-date (assuming they are part of a continuous integration build). For similar reasons I also think that commit notes are better than code comments, because they are forever associated with a snapshot of the code at the time they were written – leaving less scope for confusion.

Java Rehabilitation Clinic

2008-03-17T09:23:26+00:00

Not long after Ben persuaded me to join the fledgling Reevoo, we got our first big ReevooMark partners (Dixons & Currys) live. To celebrate this event and my having left Java behind at Thoughtworks, Ben bought me a Java Rehabilitation Clinic mug. The mug has recently developed a crack. I wonder if it’s trying to tell me it’s time to learn another language…

Java Rehabilitation Clinic mug

Remote Pair Programming

2008-03-13T09:48:22+00:00

A while ago, Chris mentioned that we’ve been trying out a few ideas for making remote pair-programming easier. We’ve been doing quite a bit more remote pairing recently. Most of the time we just use a combination of VNC (I’ve been using Vine Server) and voice over Skype (using a headset really helps).

But something else we’ve used successfully is multi-user GNU screen sessions. The advantage of this technique is that much less bandwidth is required and the terminal is much more responsive. I thought I’d post the magic incantation we’ve been using (on OSX), in case anyone else finds it useful…

  # on the server
  sudo chmod u+s /usr/bin/screen

  # first user connects to server over ssh and runs the following
  screen -S <session_name>
  ctrl-A :multiuser on
  ctrl-A :acladd <client_username>

  # second user connects to server over ssh and runs the following
  screen -x <server_username>/<session_name>

End of the Road?

2008-02-22T15:18:00+00:00

A road sign from up-state New York

The photo was taken by a friend of my sister on a recent trip to the States.

Mocking in Java using Mocha

2008-02-17T18:10:01+00:00

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

More GNER/NXEC Advance Single tickets shenanigans

2008-01-04T22:37:22+00:00

As is so often the case, today I wanted to change the date of some of my GNER Advance Single tickets. I’ve never written down the phone number of the GNER website support team, so I looked at a couple of the old GNER URLs to see if I could find it.

Skip ahead to The Nub of the Matter if you’re not very interested in the gory details.

From Pillar to Post

www.gner.co.uk - redirects to NXEC website and gives no phone number

www.gnertickets.co.uk - gives the number 08457225111

So I rang 08457225111. It was the NXEC customer support number. I waited on hold for a few minutes and pressed a number of option buttons as is the way of things. Eventually I spoke to a human being and asked whether they could change my tickets. They said they couldn’t, but that I should ring 08700101127. So I did. This turned out to be the Virgin customer support number. After the obligatory wait and button pushing, I spoke to a human who told me I had been given the wrong number. I was given a new number – 08457225225 (option #2, followed by option #4).

This got me back to NXEC and which was decidedly annoying. They then gave me another number (08700101296) which they said was for “The Train Line” who apparently used to run the GNER website. However when I got through to them, they denied all knowledge and gave me yet another number – 08703665990. This finally got me through to GNER – hooray!

I was worried that there would be complications, because the new tickets would have to be issued by NXEC, whereas the old tickets were issued by GNER. But I was pleasantly surprised to be able to change my GNER Advance Single tickets with no more pain than normal (which included giving my credit card details three times – once for each ticket – doh!). In fact the rep I spoke to was remarkably efficient.

So there goes another pointless hour of my life…

The Nub of the Matter

If you want to change tickets purchased through:

the old GNER website – then ring 08703665990
the new (but rather transient) GNER website – then ring 08547225111
the NXEC website (looks rather similar to the new GNER website) – then ring 08547225111

A Brighter Future?

On a more optimistic note, it looks like the new NXEC website allows you to change Advance Single tickets on-line up to 5 days before departure. If it works that’ll be great :-)

Mock Object Injection

2007-11-29T12:17:48+00:00

A few months back, in my Introduction to Mock Objects talk at LRUG, I talked about “Mock Object Injection”. At the time I described a number of different ways of replacing a production object with a Mock Object using Mocha. I remember that at the meeting, James Adam (who has since joined the team at Reevoo) asked me why I didn’t like the Any Instance Stub Injection technique.

I’m not sure I gave him a very convincing response and I’ve been meaning for ages to have a better go at explaining what I think are the pros and cons of each of the techniques I mentioned. Here’s the list of techniques with the ones I like best at the top. I still haven’t done a very good job, but I’d be interested to hear what other people think so that I can try and improve my understanding.

Constructor Injection

The ClassUnderTest allows its dependencies to be passed in as parameters to its constructor. A mock object is passed in as a replacement for the “real” collaborator. It may be convenient to specify the production collaborator as a default parameter value.

Advantages

The dependencies of the ClassUnderTest are explicit.

Disadvantages

Can’t think of any at the moment.

  class ClassUnderTest
    def initialize(dependency = Collaborator.new)
      @dependency = dependency
    end
    def do_something
      # use @dependency
    end
  end

  collaborator = mock('collaborator')
  # constructor parameter injection
  instance_under_test = ClassUnderTest.new(collaborator)
  instance_under_test.do_something

Parameter Injection

The ClassUnderTest allows its dependencies to be passed in as parameters to the method under test. A mock object is passed in as a replacement for the “real” collaborator. It may be convenient to specify the production collaborator as a default parameter value.

Advantages

The dependencies of the method under test are explicit.

Disadvantages

Can’t think of any at the moment.

  class ClassUnderTest
    def do_something(local_dependency = Collaborator.new)
      # use local_dependency
    end
  end

  collaborator = mock('collaborator')
  instance_under_test = ClassUnderTest.new
  # method parameter injection
  instance_under_test.do_something(collaborator)

Stubbed New Method Injection

Use Mocha’s Object#stubs to temporarily replace Collaborator#new with a stub implementation that returns a mock object.

Advantages

Better than Any Instance Stub Injection, because you can have more control over different instances of Collaborator.

Disadvantages

Dependencies of the ClassUnderTest are hidden and not explicit.

  class ClassUnderTest
    def initialize
      @dependency = Collaborator.new
    end
    def do_something
      # use @dependency
    end
  end

  collaborator = mock('collaborator')
  # stubbed new method injection
  Collaborator.stubs(:new).returns(collaborator)
  instance_under_test = ClassUnderTest.new
  instance_under_test.do_something

Writer Method Injection

Use an attribute writer method to replace the “real” collaborator with a mock object.

Disadvantages

The ClassUnderTest has to unnecessarily expose a way to modify its internal state. The test is coupled to the implementation of the ClassUnderTest.

  class ClassUnderTest
    attr_writer :dependency
    def initialize
      @dependency = Collaborator.new
    end
    def do_something
      # use @dependency
    end
  end

  collaborator = mock('collaborator')
  instance_under_test = ClassUnderTest.new
  # writer method injection
  instance_under_test.dependency = collaborator
  instance_under_test.do_something

Stubbed Private Method Injection

Use partial mocking to temporarily replace a private builder method with a stubbed version of the method.

Disadvantages

The test is coupled to the implementation of the ClassUnderTest. The partial mocking of the instance_under_test means that the test is not testing a pristine instance of the ClassUnderTest, but a modified one. It also means that the boundaries between test code and production code are less clear.

  class ClassUnderTest
    def do_something
      local_dependency = build_collaborator()
      # use local_dependency
    end
    private
    def build_collaborator
      Collaborator.new
    end
  end

  collaborator = mock('collaborator')
  instance_under_test = ClassUnderTest.new
  # stubbed private method injection
  instance_under_test.stubs(:build_collaborator).returns(collaborator)
  instance_under_test.do_something

Any Instance Stub Injection

Use Mocha’s Class#any_instance method to temporarily replace the method on a collaborator with a stub method.

Disadvantages

The stubbed method is applied to all instances of the collaborating class. If the instance_under_test interacts with the stubbed method on more than one instance of the collaborating class, it isn’t possible to specify different behaviour for the stubbed method on each instance. Even if the instance_under_test only interacts with the stubbed method on one instance of the collaborating class, the test is specifying more stubbed behaviour than strictly necessary which could lead to false positives.

  class ClassUnderTest
    def do_something
      local_dependency = Collaborator.new
      return local_dependency.do_stuff
    end
  end

  # any_instance stub injection
  Collaborator.any_instance.stubs(:do_stuff).return('something useful')
  instance_under_test = ClassUnderTest.new
  instance_under_test.do_something

Instance Variable Set Injection

Use Object#instance_variable_set to replace the reference to a collaborator with a mock object.

Disadvantages

The test is coupled to the implementation of the ClassUnderTest. In particular the test is coupled to the supposedly private instance variable. In my opinion, it would be more honest to expose the instance variable by adding an attribute writer and using Writer Method Injection.

  class ClassUnderTest
    def initialize
      @dependency = Collaborator.new
    end
    def do_something
      # use @dependency
    end
  end

  collaborator = mock('collaborator')
  instance_under_test = ClassUnderTest.new
  # instance_variable_set injection
  instance_under_test.instance_variable_set(:@dependency, collaborator)
  instance_under_test.do_something

Changing GNER Advance Single tickets (reply)

2007-11-08T20:02:18+00:00

I finally heard back to my complaint from GNER Customer Service yesterday (7th Nov). Until then I had not even had an acknowledgment (automated or not) that they’d received my email. In their response, they acknowledge that it is possible to change tickets purchased online at a ticket office (although you may lose any “web discount”).

There are some slightly woolly words in the middle, but I’m pleased they do have the good grace to explicitly apologise for the fact I was “wrongly advised” by the Durham Travel Centre.

I’m pleased to have had a response and it’s reassuring to hear that they are taking action to improve the situation. I’m surprised how much better I feel to have had a response.

For completeness, I’ve posted the response on getsatisfaction.com.

Here’s the full text of their response with employee names removed…

Mr Mead,

Thank you for your email.

When making an alteration to an online booking at the station you will not receive the web discount. The web discount, is only applicable to bookings done over the internet, from beginning to end.

If you wish to make an alteration to your online booking, you can do this at a GNER station, however, you do need to take the full ticket into the station with you. When making an alteration to an online booking at the station, you will loose your online discount, because this ticket is not being booked online.

Our staff usually do an excellent job in meeting the needs of our passengers but I know that there are occasions when incidents occur which cause dissatisfaction. We ask our staff to uphold company policies wherever practical in the interest of fairness and consistency. Unfortunately, this does mean that the ability of the staff to exercise discretion and waive certain policies is very limited.

I am concerned that you were wrongly advised that online tickets could not be altered at the station, this was incorrect and for this I apologise.

I will of course forward your comments to the manager responsible for Mr X, who can address this matter directly with him.

If you have any further questions regarding ghtis matter, please contact me again.

Kind Regards

Customer Relations

Implementation Patterns

2007-11-07T23:37:00+00:00

I’m looking forward to reading Kent Beck’s new book Implementation Patterns which has been reviewed on InfoQ. I’ve had it on pre-order at Amazon and it’s just been dispatched. I always meant to read his Smalltalk equivalent Smalltalk Best Practice Patterns, but never quite got round to it.