Friday, February 16, 2007

TDD: Removing test noise

Consider the following expectations:
expect 4 do
Math.plus(2,2)
end

expect ActiveRecord::Base.to_recieve(:execute).with("insert ...") do
Person.save
end
In the past I've written about the absence of mocks in xUnit frameworks. I still believe this is a limitation and it leads to the following additional limitation: You have to set up behavioral expectations in a different way than you set up state based assertions. A side effect of this is that it often leads to tests that contain both behavioral and state based requirements, which are generally brittle. I believe that having two ways to express a test requirement increases noise in tests. The above example attempts to address the disconnect.

There's another spot where test noise can creep in: the name of the test. This idea is very controversial because it goes against what can work best. The problem is, what can work best, rarely does.

For example, when you encounter a test name such as def test_should_send_email_when_the_order_is_placed, how often does the test actually verify that an email is sent following an order being placed? Furthermore, how often do you even find test names that are that descriptive?

In my experience, I generally find tests such as def test_can_save. This is annoying, but it's better than when I find def test_should_verify_that_the_service_is_called_to_verify_a_credit_card and the actual body of the test does something entirely different. This generally happens on larger code bases that have been in development for a bit of time. When the description was originally written it was (hopefully) valid; however, at some point the test broke when a developer was trying to check in. The developer came to the test, didn't understand it and did what they had to do to make it work again. This cycle continued a few times until I found it (also when the tests broke). By the time I found it, no one had a full picture of what the test should be doing and I often end up deleting the test since even the original author has no idea what it's doing with all the patches that other developers have put on it.

The above scenario describes a larger problem: lack of communication. When the tests break the team members should talk to each other to ensure that the test continues to provide value; however, this simply doesn't happen. You could argue that the developers need to be beaten with a stick; however, I prefer a solution that plays to what developers desire: the ability to change the tests themselves.

While the above example does highlight a lack of communication, it isn't simply in the form of team members talking to each other. The test itself did not clearly communicate it's intent and was hard for the maintainers to comprehend.

The original example takes into account that the test name is rarely valuable and replaces it with what is important, the expectation of the test. By reducing the noise in the test maintainability is increased as well as the likelihood that the test will continue to provide value.

If you are a fan of Dave Astels one assertion per test guideline you will notice that the original example also guides you to follow the guideline. However, one assertion per test is simply a guideline so I expect you could also add additional expectations in the block where necessary.

In the Validatable work I've been doing lately I started using:
expect [expected value] do
....
end
This is working well, but I haven't taken the time to implement the mock syntax. I expect that using Mocha it would be fairly easy; however, I think the long term solution is to create a new testing(expectation?) framework.

4 comments:

  1. Anonymous11:16 AM

    "expect" is commonly used with mocks (think mocha - obj.expects...), so using it for state assertions becomes confusing IMO.

    I do like the idea of wrapping parts of specs/test methods in blocks in order to clarify what they are doing. One syntax that Dan North and I batted around was this:

    specify "should start boiler" do
    expect boiler do |b|
    b.start
    end
    coffee_maker.start
    ensure_that b do |b|
    b.should be_on
    end
    end

    The idea being that #expect puts objects passed as arguments into a record mode (setting mock expectations) and #ensure_that adds #should to objects passed to it.

    Mixing #ensure_that with #should doesn't read that well, but the idea of supporting mock and verification syntax directly on objects without having to pollute Object in the process has some merit.

    Anyhow - the important thing is overloading "expect". Of course, we've done the same thing in RSpec by talking about "expectations" instead of "assertions". Perhaps that needs some re-thinking as well.

    WDYT?

    ReplyDelete
  2. Anonymous3:13 PM

    David,
    Expect is commonly used with mocks; however, each word that I thought might fit well is commonly used with other ideas, such as "verify" and "assert".

    I'm less concerned what the current thought might be and am looking at the problem from a "if the slate were blank what would I do" approach.

    I do wish I'd played with this syntax long ago so I could have potentially contributed to RSpec.

    Though, we may be looking at the same problem with different focusses. From my conversation's with Dan North, BDD seems to be something that testers and BA's could do, where I'm much more concerned with writing expectations and code that is descriptive, yet concise.

    Of course, it would be really nice if we could solve both problems. In RSpec's current state, I'd guess that the description of specify is just as valuable as test names. It would be interesting to see if you could find a syntax that would allow you to do something such as:

    Boiler.any_instance.on.should be.true do
    coffee_maker.start
    end

    and

    4.should result_from do
    Math.plus 2, 2
    end

    I've not used RSpec, so feel free to tweak the example to reality; however, I think it represents my desire.

    ReplyDelete
  3. In the case of a classic expectation, I think your second example is as clean as it gets:

    expect ActiveRecord::Base.to_recieve(:execute).with("insert ...") do
    Person.save
    end

    ReplyDelete
  4. Anonymous4:56 AM

    Jay - I'm excited to tell you that, yes, you can do things like that with the upcoming release of RSpec.

    Here's the second one (using regular Ruby addition):

    http://pastie.caboo.se/40999

    This particular one is a bit odd and requires explanation so I included it in the paste.

    ReplyDelete

Note: Only a member of this blog may post a comment.