ActiveTest: Examination, Part II: Abstracting Without Basis

Posted by Mathew Abonyi Mon, 01 Jan 2007 04:37:50 GMT

My first mistake started the following cascade effect:

  • move all assert_x_success and assert_x_failure to class methods
  • begin defining any test case as a class method
  • extract the definition of an instance method to the superclass level (ActiveTest::Controller)
  • extract those superclass methods to abstract methods (ActiveTest::Subject)
  • fragment assert conditions to be passed up the hierarchy (define_behavioural)
  • extract fragments to the class level (assert_restful_get_success, method_missing, etc.)
  • rinse & repeat for each Subject

Rather than inheriting from a single class with all its assertions self-contained, I now inherited from ActiveTest::Base -> Subject -> Controller. Along the way there were sprinkled methods on the class and eigenclass level which helped define the test cases (ActiveTest::Subject.define_behavioral) or provide extra assertions. I had extracted so much that it was a nightmare to understand what was actually happening (a rare problem when writing in Ruby).

An Extraction Case Study from 0.1.0 Beta (abridged)

module ActiveTest
  class Controller < Subject

    class << self

      def succeeds_on(action, options = {})
        define_behavioral(:succeed_on, action, options) do
          send("call_#{action}_action", options)
          send("assert_#{action}_success")
        end
      end

    end

    ...

    def assert_restful_get_success(action, options = {})
      assert_response :success
      assert_template action.to_s
    end

    ...

    def method_missing(method, *args)
      options = args.last.is_a?(Hash) ? args.dup.pop : {}
      if method.to_s =~ /^assert_(.*)_(success|failure)$/
        action, sf = $1, $2
        if action !~ /get|post|put|delete/
          request_method = self.class.determine_request_method(action)
          send("assert_restful_#{request_method}_#{sf}", action, options)
        end
      elsif method.to_s =~ /^call_(.*)_action$/
        request_method = self.class.determine_request_method(action = $1)
        send("call_request_method", request_method, action, options)
      else
        super
      end
    end

    def call_request_method(request_method, action, options = {})
      options = expand_options(options)
      send(request_method, action, options[:parameters])
    end

  end

end

As you can see, I left out quite a few secondary methods which are being called. The first problem here is in the design of defining a behaviour: it gives too many hooks to the user that allow them to customise. If I am trying to address common cases, this should have been a red flag to a bull—I was blind. The result was a ton of method_missing calls because I extracted every call into a dynamic method, including the assertions in a dynamically defined test case (twice, in fact). Awful.

What Was I Thinking?

When I first began writing ActiveTest, I focused solely on making test cases a one-line business. ActiveTest contained just a simple class method on ActiveTest::Subject to define any kind of test case and a number of classes inheriting from it that defined common groups of assertions. Then, I abstracted the entire process into defining ‘behaviours’, approaching each test case as a behaviour request-response pair (in fact, it is a context if you use BDD terminology). I went through a number of phases looking for the cleanest way to implement this idea of behaviours (I aped RSpec, Test::Rails and others). None of them seemed appropriate, so my own metalanguage was written that looked clean (but only when I did not provide parameters). I thought the metaprogramming of tests with a descriptive class method call would be self-explanatory enough.[1]

Take away from this one single maxim, by which you should live and die when you refactor: do not abstract without real-life need. If you don’t base your abstraction on something real, you are theorizing—and we all know that the problem with theories is that they have not been tested.

Coming Up Next: Code Bloat…

Footnotes

1 Recently I encountered a situation where parameters were impossible to pass. This unforeseen situation (where I needed RFuzz:http://rfuzz.rubyforge.org/) contributed to my realisation that ActiveTest’s current design is useless for anything moderately dynamic or complex.

Posted in , , ,  | no comments | 4 trackbacks

ActiveTest: Examination, Part I: Removing Transparency

Posted by Mathew Abonyi Mon, 01 Jan 2007 03:34:58 GMT

The first abstraction I made was to change assert_get_success and its equivalents (get_failure, post_success, etc.) into an abstract idea of common test cases. Obviously every application is going to be treated differently, but to make a suite of tools to remove the most common tests seemed like a useful thing to extract—it’s DRY.

Consequently, I had the following in Test::Unit::TestCase:

  class << self
    def test_should_index_items
      define_method(:test_should_index_items) do
        assert_get_success :index
      end
    end
  end

I then abstracted it a little:

  class << self
    def test_should_get_with_success(action = :index, parameters = {})
      define_method("test_should_#{action}_item") do
        assert_get_success action, parameters
      end
    end
  end

This is basic DRYing up. However, I have at this point already lost sight of the simplest benefit of writing out test cases with Test::Unit: documentation. If someone else uses this, the only thing they know is that their GET succeeds… but in what way? I began documenting what each method does and extracting more and more complex cases which may be ‘common’. Documenting these methods hid the general fault, which was making the entire testing process opaque to the user. Tests started to look like this:

class TestSomething < ActiveTest::Base
  test_should_get_with_success :index
  test_should_get_with_success :show, :id => 1
  test_should_get_with_success :new
  test_should_get_with_success :edit, :id => 1
end

So my first mistake was removing transparency without thinking about what I was beginning to lose. This is quite project-specific. In most cases, making methods opaque and useful is a good thing, but in the case of ActiveTest, it needs to be flexible and transparent. It is hard to beat a series of asserts.[1]

This mistake is probably my least offensive, because some, including myself, may like this feature just to remove all those irritating test_should_index_items, especially when you are writing new controllers left and right.

What Was I Thinking?

I thought that the documenting aspect of testing was purely outlining what everything should do, rather than describing what everything should do. My original idea was to make it easier to document and test at the same time by outlining the cases in each suite. Just in terms of line ratios, I now had a 5-line method which let me write a test case in 1 line. Not bad. It quickly paid for itself because it catered to :index, :new:, :show and :edit. However, I lost all the expressiveness of showing what is being tested.

Coming Up Next: Abstracting Without Basis…

Footnotes

1 It can be done. In fact, the next generation of Active Test, while rather different, frees up the tester to think completely about the behaviour of his code. I will trade in the brevity of test cases for the flexibility of defining them (for anything, whether Rails application, plugin, gem or 60-line file).

Posted in , , ,  | no comments | 1 trackback

Older posts: 1 2