← Back to Upcase

Writing specs for file input


(Jon Seidel) #1

I paired yesterday on an interview and we started working on refactoring my code to use a reader class for the input to the coding challenge. That quickly got to the point where we were constantly writing little test files and I said that at this point, I’d write a utility class that would quickly create a test file that could be used for the spec. Something like this:

def create_test_word_list(words)
   File.open(TEST_FILE, 'w') do |file|
     words.each { |w| file << w + "\n" }
  end
end

The interviewer said that he didn’t think this would really be a unit test and we proceded to dive into a full-scale mocking exercise that wound up looking something like this:

  it "raises an exception if the file isn't there" do
    file_methods_that_never_find_a_file = double( "File", :exists? => false )
    expect{
    dr = DigraphReader.new('blah',file_methods_that_never_find_a_file)
    }.to raise_exception "Missing file"
  end

  class FakeFileMethods
    def initialize( file_to_open )
      @file_to_open = file_to_open
    end

    def open(file_name)
      yield @file_to_open
    end

    def exists?(file_name)
      true
    end
  end

  class FakeFile
    def initialize(*contents)
      @contents = contents
    end
    def each_line
      @contents.each do |line|
        yield line
      end
    end
  end

  it "doesn't yield for an empty file" do
    fake_file = FakeFile.new()
    file_methods_for_an_empty_file = FakeFileMethods.new(fake_file)

    dr =  DigraphReader.new('blah', file_methods_for_an_empty_file)
    dr.read_graph_input do |f,t,d|
      raise "didn't expect block to actually be called!"
    end
  end

This seems to me to be an overly-complicated approach to a fairly straight-forward problem. The interviewer pointed out that it wasn’t a unit test because it wasn’t properly isolated from the File system, even though he allowed that mocking could lead to a situation where a method was mocked and used which wasn’t actually part of the File class… like mocking/using ‘read_lines’ when the actual method is ‘read_line’.

My counter-argument: this is an operating facilitiy which can reasonably be expected to be present, otherwise we have much larger issues than a little lack of test isolation. If this were an external service (like access Twitter, for example, all this overhead would make more sense to me.

Your thoughts?