Skip to content

Shouldn't rely on result value returned by DRb to be implicitly convertible to integer

gitlab-qa-bot requested to merge github/fork/jscheid/master into master

Created by: jscheid

Hi there,

this fixes a problem that occurs when using rspec 2.6.1 together with spork 0.9.0.rc (and possibly other versions). I am mystified why apparently nobody else is running into this issue, but I can reproduce it like so:

  • Run spork 0.9.0.rc in rspec mode (on a sporkified rails 3 app in my case)
  • Run latest rspec with the --drb flag on any old test

Everything works fine, but a "TypeError: can't convert DRb::DRbObject into Integer" will be raised at the very end. Not a big problem but definitely irritating as it makes it harder to see rspec's "X examples, Y failures" output.

I've had a look at both rspec and spork code and here's what I think happens. Spork runs the tests in a forked child process and marshals/unmarshals the test result (an integer in rspec's case) via an IO object to bridge the parent/child process boundary, then passes the IO object back to rspec proxied by a DRb::DRbObject. The combination of DRbObject and IO object prevents an implicit conversion to integer, causing the TypeError to be raised.

At first I was thinking of fixing this issue in spork code, however this is tricky due to spork's generic architecture. Casting the IO object to an integer or even just a string on the spork side of things feels like it's taking away from spork's genericalness. For example, as far as I can tell it would preclude other spork clients from lazy-loading or incrementally loading a large result value.

In comparison, this fix appears to be straightforward and unintrusive, and in line with Postel's Law. Do you agree?

I thought about including a test case but due to the problem occurring at the shell/rspec process boundary under only very specific circumstances I found it hard to come up with a concise one that doesn't complicate things unnecessarily. If you insist on having a test, could somebody give me a hand with it?

Merge request reports