Skip to content

Instantly share code, notes, and snippets.

@jamesladd
Last active August 29, 2015 14:15
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save jamesladd/09741a50a79f293ff21b to your computer and use it in GitHub Desktop.
Save jamesladd/09741a50a79f293ff21b to your computer and use it in GitHub Desktop.
Original ansi.rb requiring more than 12 test to prove to_ansi method works as expected vs new version requiring 3 tests to prove to_ansi method works as expected.
# - ORIGINAL - Requires 12 test cases the provide to_ansi does what it should
# 1. Test empty commands map returns ""
# 2. 11 Tests to test each if/elsif.
#
class Clear; end
class HideCursor; end
class ShowCursor; end
class SetPos < Value.new(:row, :column); end
class FG < Value.new(:red, :green, :blue); end
class BG < Value.new(:red, :green, :blue); end
class Default; end
class Inverse; end
class Reset; end
def to_ansi(commands)
commands.map do |command|
if command == Clear then escape "2J"
elsif command == HideCursor then escape "?25l"
elsif command == ShowCursor then escape "?25h"
elsif command == SetPos then escape "#{line + 1};#{column + 1}H"
elsif command.is_a? FG then foreground(command)
elsif command.is_a? BG then background(command)
elsif command == Default then escape "39?49m"
elsif command == Inverse then escape("7m")
elsif command == Reset then escape("0m")
elsif command.is_a? String then command
else raise RuntimeError.new("BUG: didn't understand ANSI command #{command.inspect}")
end
end.join("")
end
# - NEW - Requires 3 test cases to prove to_ansi does what it should.
# 1. Test empty commands map returns ""
# 2. Test command is handled (ie: mock handler etc)
# 3. Test command is not handled (ie: RuntimeError is raised)
#
# handlers - a hash of Handler instances with a default handler
# for missing keys to handle case where command is not recognized.
# (Raises RuntimeError.)
#
# Personally I'd break the 'handlers' line out into a method.
#
# type_of is a helper to determine the class of the command or in
# the case of a String instance return String. Maybe there is a method
# elsewhere to help w this.
#
handlers = Hash.new default_handler
handlers[Clear] = ClearHandler.new
# add other initialisations for other handlers here.
handlers[Reset] = ResetHandler.new
def to_ansi(commands)
commands.map do |command|
handlers[type_of(command)].execute command
end.join("")
end
@garybernhardt
Copy link

I assume that your 12 estimated tests for my version include a test for to_ansi([]), plus each of the 11 if branches. Your proposed version contains none of that code, and therefore none of those tests. If you actually write the classes for each of the 11 ANSI commands, all 11 of those little expressions (escape "2J", escape "?25l", etc.) will be reintroduced and will need to be tested.

You now have 11 handler tests plus the three tests that you propose for to_ansi itself, making 14. The ANSI translation rules will be spread across a minimum of six times the vertical space of my to_ansi function, since each single-line if/elsif becomes a newline plus a class line, a def line, the trivial body, and two ends. You also have to define the type_of method, which didn't exist before.

If you actually write this code out, you'll find that it's six or seven times the length of mine, has an identical external interface, explodes the logic such that it doesn't fit on a laptop screen at once, and requires at least as many tests to fully cover all 11 cases (and even more tests than my code if you include your extra 3 tests).

There are 11 independent paths. They cannot be merged. (OK, unless you construct a commands array that includes multiple commands that are covered in one test case, but that applies equally to either solution). Simply moving the code to other classes doesn't reduce the number of test cases.

I've suggested multiple times that you actually write the full code and answer, for yourself, how to cover it in fewer than 11 tests. That's still my suggestion. Write all of those little handler classes out and ask yourself how escape "2J", escape "?25l", etc. get covered. If your answer is "I don't need to cover them because they're basically constants", then you've discovered the reason that I didn't write tests for this code.

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