Skip to content

Instantly share code, notes, and snippets.

@jamesarosen
Last active December 11, 2015 08:09
Show Gist options
  • Star 2 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save jamesarosen/4571228 to your computer and use it in GitHub Desktop.
Save jamesarosen/4571228 to your computer and use it in GitHub Desktop.
Some questions about optional dependencies in Ruby gems.

Some questions about optional dependencies

1. A small ruby library

I'm writing a small Ruby library:

class FizzBuzzEndpoint

  def call(env)
    request = Rack::Request.new(env)
    q = request['q']
    input = Integer(q)
    output = fizz_buzzify(input)
    [ 200, {}, [ output.to_s ] ]
  rescue ArgumentError
    return [ 422, {}, [ "#{q} is not an integer" ] ]
  end
  
  private
  
  def fizz_buzzify(int)
    int % 15 == 0 ? 'fizbuzz' :
      int % 5 == 0 ? 'buzz' :
        int % 3 == 0 ? 'fizz' : int
  end

end

2. Adding a minor concern

I want to add instrumentation so it's easy for consumers of my library to see how the service is getting used. One option would be to pick an instrumentation library and run with it:

def call(env)
  request = Rack::Request.new(env)
  q = request['q']
  input = Integer(q)
  output = fizz_buzzify(input)
  instrument('success', output)
  [ 200, {}, [ output.to_s ] ]
rescue ArgumentError
  instrument('error', q)
  return [ 422, {}, [ "#{q} is not a number" ] ]
end

private

def instrument(status, value, &block)
  ActiveSupport::Notifications.instrument "fizz_buzz.#{status}", value, &block
end

I don't like this for two reasons:

  1. it takes a very small library and adds a large dependency
  2. it forces an implementation on the consumer. This gets worse as many libraries pick different implementations.

The Ruby world doesn't have true interfaces (in the Java sense), but we do have interface libraries for some utilities. For example, the multi_json gem lets library authors require an implementation of JSON parsing and encoding without specifying a particular one. It ships with a default.

Sadly, there isn't an interface gem for every utility.

3. Declaring a hook

A second path would be to define a null implementation and let clients override:

def instrument(status, value, &block)
end

4. An optional implementation

A third is to combine the first and second, making the dependency on ActiveSupport::Notifications soft or optional. The code gets complex enough that it might warrant its own file and class:

# lib/fizz_buzz/instrumentation
module FizzBuzz::Instrumentation

  begin
    require 'securerandom' # ActiveSupport::Notifications requires this, but doesn't declare the dependency
    require 'active_support/notifications'
  
    def instrument(status, value, &block)
      ActiveSupport::Notifications.instrument "fizz_buzz.#{status}", value, &block
    end
  rescue LoadError
    def instrument(status, value, &block)
    end
  end
  
  module_method :instrument
  
end

5. Testing issues

This complexity also makes it harder to test the instrumentation behavior of the library. My first instinct is to do the following:

  1. add the appraisal gem to my gemspec
  2. add an appraisal that includes activesupport and one that doesn't
  3. add a test class that tests the ActiveSupport::Notification.instrument calls if and only if that library is available
  4. add a test class that tests the availability of the hook (and possibly that the default implementation is null) if and only if activesupport is not available

6. An alternate testing path

I really like my tests to be fast. It's so important to me that in this library I wrote a test to test that the tests run in under a second. The two appraisals plus activesupport bother me.

As Corey Haines says, TDD means listening to your tests. When they're slow or hard to write, it means something is wrong with your code. I don't doubt that, though I don't see the solution here. Instead, I'll pose a new testing plan:

class ActiveSupportUnavailableTest < Test::Unit::TestCase

  def setup
    Kernel.stubs(:require).with('active_support/notifications').throws(LoadError.new)
    load 'fizz_buzz/instrumentation'
  end

  def test_null_implementation_when_active_support_is_unavailable
    assert FizzBuzz::Instrumentation.respond_to?(:instrument)
  end
  
end

class ActiveSupportAvailableTest < Test::Unit::TestCase

  module FakeActiveSupport
    module Notifications
    end
  end

  def setup
    Kernel.stubs(:require).with('active_support/notifications')
    Object.send :const_set, :ActiveSupport, FakeActiveSupport
    load 'fizz_buzz/instrumentation'
  end
  
  def teardown
    Object.send :const_set, :ActiveSupport, nil
  end
  
  def test_adds_fizz_buzz_prefix
    FakeActiveSupport::Notifications.expects(:instrument).with do |args|
      args.first == 'fizz_buzz.foo'
    end
    FizzBuzz::Instrumentation.instrument 'foo', 12
  end
  
  def test_sends_data
    FakeActiveSupport::Notifications.expects(:instrument).with do |args|
      args.second == 'bar'
    end
    FizzBuzz::Instrumentation.instrument 'foo', 'bar'
  end
  
end

I don't feel settled on any of this. I feel the library does need the instrumentation, but I really want to keep it small. (It's not a framework, after all.) Can I reasonably provide only the null implementation and ask clients to monkey-patch or subclass to get the behavior? Is the "right" solution to create a multi_instrumentation gem on which this gem depends?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment