Followup: TDD and Ruby Scripts

A few weeks ago, I asked for some advice about using TDD for Ruby replacements of bash scripts.

I received a few responses on the Ruby Rogues Parley Forum.

One suggestion was to not bother writing tests, because my example script has little logic to it and is short enough to throw away and rewrite if it’s wrong. For this particular script, that’s probably true. But this is part of a bigger library, and the example I posted was intended to be simple enough to discuss the principles without getting bogged down in details.

Another response pointed me to a talk by David Copeland at GoGaRuCo 2011: Test-drive the development of your command-line applications. This is a good talk, and worth watching. It definitely helped, but still didn’t deal with some of the issues in my example problem, like how to test the copying of files to a remote server or running commands on a remote server.

Another response pointed me to David Copeland’s book, Build Awesome Command-Line Applications in Ruby. I had just ordered that book a few days earlier, and it’s now in my reading queue. I expect to learn quite a bit from it.

Here’s my original solution to the example problem. It has the redeeming quality of working for its intended purpose, but I’m not totally happy with the tests.

I split the problem into two parts: building the package, and uploading the package. I made little classes, essentially MethodObjects, for each part.

First, the package building part:

BuildPackageTest

class BuildPackageTest < MiniTest::Unit::TestCase
  include FlexMock::TestCase
 
  def setup
    @builder = BuildPackage.new("source", "target")
  end
 
  def test_executes_normal_build_command
    flexmock(@builder).should_receive(:`).once.with("fakeroot dpkg-deb -b source target")
 
    @builder.call
  end
 
  def test_executes_build_command_with_permission_change
    flexmock(File).should_receive(:directory?).once.with("source/special/path").and_return(true)
    flexmock(@builder).should_receive(:`).once.with(%r{chown -R 1000.1000 source/special/path})
 
    @builder.call
  end
 
  def test_extracts_debfile_name
    output = "dpkg-deb: building package `my-package' in `target/my-package_42_all.deb'."
    flexmock(@builder).should_receive(:`).and_return(output)
    @builder.call
    assert_equal("target/my-package_42_all.deb", @builder.final_product)
  end
end

As you can see, I’m not actually testing the effects of the various commands; instead, I’m just testing that the commands look right. Not ideal, but better than nothing. I’m testing the commands by mocking the backtick method on the object under test. I realize that mocking a method on the class under test is a smell, but it seemed better than the alternatives.

Here’s the code to make it pass:

BuildPackage

class BuildPackage
  attr_reader :final_product
 
  def initialize(source_directory, target_directory)
    @source_directory = source_directory
    @target_directory = target_directory
    @final_product = ""
  end
 
  def call
    special_directory = "#@source_directory/special/path"
    command = "dpkg-deb -b #@source_directory #@target_directory"
    if File.directory?(special_directory)
      command = %{bash -c "chown -R 1000.1000 #{special_directory} && #{command}"}
    end
    output = %x{fakeroot #{command}}
    @final_product = output[/dpkg-deb: building package .* in `(.*)'/, 1] || "" if output
  end
end

Here’s the tests for the package upload part:

UploadPackageTest

class UploadPackageTest < MiniTest::Unit::TestCase
  include FlexMock::TestCase
 
  HOST = UploadPackage::REPOSITORY_HOST
 
  def test_uses_reprepro_directly_when_repository_is_local
    upload = UploadPackage.new("#{HOST}.example.com}")
    flexmock(upload).should_receive(:system).once.with(/^reprepro.*filename.*$/)
 
    upload.call("filename")
  end
 
  def test_uses_ssh_when_repository_is_remote
    upload = UploadPackage.new("not-the-repo-host.example.com")
    flexmock(Net::SSH).should_receive(:start).once.with(HOST, "username", Proc)
 
    upload.call("filename")
  end
end

Note that I don’t test the commands that I’m executing in the remote SSH session, because I couldn’t think of a good way to do that at the time I wrote this.

Here’s the code:

UploadPackage

require "net/scp"
require "net/ssh"
 
class UploadPackage
  REPOSITORY_HOST = "repo-host"
 
  def initialize(hostname)
    @local = hostname.downcase.include?(REPOSITORY_HOST)
  end
 
  def call(filename)
    if @local
      upload_locally(filename)
    else
      upload_remotely(filename)
    end
  end
 
  def upload_locally(filename)
    system(repository_command(filename))
  end
 
  def upload_remotely(filename)
    Net::SSH.start(REPOSITORY_HOST, "username") do |session|
      remote_filename = File.join("/tmp", filename)
 
      puts "Uploading #{filename}..."
      session.scp.upload!(filename, remote_filename)
 
      puts "Including #{filename} in the repository"
      session.exec!(repository_command(remote_filename))
 
      session.exec!("rm #{remote_filename}")
    end
  end
 
  def repository_command(filename)
    "reprepro -Vb /path/to/repo includedeb squeeze #{filename}"
  end
end

To use these classes:

Putting It All Together

builder = BuildPackage.new(SOURCE, TARGET)
uploader = UploadPackage.new(`hostname`)
 
builder.call
uploader.call(builder.final_product)

And there’s my less-than-ideal solution to the problem. Now that you’ve seen the code, how would you improve it?

Comments