Eliminating duplication from tests

devbanana · April 10, 2009, 6:17am

Hi,

Lately I’ve found myself trying to reduce typing as much as possible, so
when I have a lot of tests that only differ in a few variables, I tend
to
abstract them to reduce typing.

For instance, I have an assert_form_errors method that I call within a
describe block to ensure the proper errors are displaying. I have an
assert_form_fields method to validate that the fields that have errors
are
wrapped in div.error, and the ones that don’t are not (that method
should
probably be renamed now that I think of it, but I’m not sure to what).
That
one is especially useful because it can test all of the fields in one
go.

So is this necessarily bad? I’ve heard it’s not always good to reduce
duplication in tests, but it just seems much preferable than copy/paste.

Also, I fear it is becoming more test-like than behavior, but that’s the
way
my testing is progressing.

Brandon

devbanana · April 10, 2009, 9:11am

On Fri, Apr 10, 2009 at 1:14 AM, Brandon O.
[email protected] wrote:

Hi,

Lately I’ve found myself trying to reduce typing as much as possible, so
when I have a lot of tests that only differ in a few variables, I tend to
abstract them to reduce typing.

abstract or extract? I think you mean extract.

For instance, I have an assert_form_errors method that I call within a
describe block to ensure the proper errors are displaying. I have an
assert_form_fields method to validate that the fields that have errors are
wrapped in div.error, and the ones that don’t are not (that method should
probably be renamed now that I think of it, but I’m not sure to what). That
one is especially useful because it can test all of the fields in one go.

So is this necessarily bad? I’ve heard it’s not always good to reduce
duplication in tests, but it just seems much preferable than copy/paste.

If it’s not always good, then it’s probably not always bad It
really varies from situation to situation. You need to understand the
risks involved.

Can you please post specific examples of the use of these so we don’t
have to talk in generalities?

Also, I fear it is becoming more test-like than behavior, but that’s the way
my testing is progressing.

Of course your “testing” is progressing towards something more
“test-like.” You’re calling it “testing” so you’re probably thinking
of it as “testing.” You’re using words like “ensure” instead of
“specify” and you’ve even named your expectations “assert_xxx” instead
of “expect_xxx.”

TDD is not a testing practice. It is a design practice. The whole
reason BDD was created was to help get that point across, and you are
fighting it every step of the way in your example.

Don’t feel bad about this. If it was easy, we wouldn’t need to talk
about it. It is very subtle stuff. And it’s confusing because the
very nature of a code example changes as soon as the code it specifies
gets written. Consider:

Write an example. There is no code, so the example is a
specification for code that doesn’t exist.
Write code to pass the example. The example is now, in addition
to being a specification for that code, documentation of existing code
and a regression test for that code.

During step 2 the example itself does not change, yet it transforms in
its meaning. The example is a test, but not until after the code to
pass it is written. And that’s even more confusing because we use the
terms pass and fail.

I have no idea if this is helpful Regardless, that is the intent.

Cheers,
David

devbanana · April 10, 2009, 10:46am

On Fri, Apr 10, 2009 at 4:38 AM, Brandon O.
[email protected] wrote:

have to talk in generalities?
" when the request method is #{method.to_s.upcase}" do
end
end
end
end
end # assert_route_recognition

I meant where it’s used, not how it works But now that you’ve shown
the implementation, I can see that this is a macro that generates
examples. So when you’re using it it probably looks like this:

describe WidgetsController do
assert_route_recognition “/widgets/”, :widgets, [:get, :post]
end

In this case, I’d change the name to something like
recognizes_routes_for:

describe WidgetsController do
recognizes_routes_for “/widgets/”, :widgets, [:get, :post]
end

But even that is a bit of a challenge to understand what all the
arguments mean. Another option might be:

describe WidgetsController do
recognizes_routes_for :get, :post, “/widgets/”
end

The controller can be inferred from WidgetsController in the macro,
which you can access from the described_class method:

:controller =>
described_class.to_s.sub(/Controller/,‘’).underscore.to_sym

an example?
I don’t have any examples I was just making a suggestion because
part of the BDD nomenclature is “expectation” instead of “assertion.”
An expectation is about something in the future. An assertion is about
something that already exists.

In this case, since it’s a macro, I’d go with what I suggested above
(recognizes_routes_for).

HTH,
David

devbanana · April 10, 2009, 2:31pm

David,

Thank you very much. Most of my methods that are generating examples are
similar to that, and I hadn’t thought to call them macros.

Anyway I like the name you suggest. I’ll go through my macros and try to
come up with more appropriate names for them.

Oh, and that described_class trick is great; thanks for showing me that.

Brandon

devbanana · April 10, 2009, 9:41am

-----Original Message-----
From: [email protected] [mailto:rspec-users-
[email protected]] On Behalf Of David C.
Sent: Friday, April 10, 2009 3:10 AM
To: rspec-users
Subject: Re: [rspec-users] Eliminating duplication from tests

Can you please post specific examples of the use of these so we don’t
have to talk in generalities?

Sure, here is something I just wrote.

def assert_route_recognition path, controller, methods
  describe "route recognition" do
    [:get, :post, :put, :delete].each do |method|
      if methods.include? method
        it "should generate the params" +
        " {:controller => '#{controller}', :action =>

‘#{methods[method]}’}" +
" when the request method is #{method.to_s.upcase}" do
params_from(method, path).should == {
:controller => controller,
:action => methods[method]
}
end
else
it “should not accept the #{method} method” do
lambda {
params_from(method, path)
}.should raise_error(ActionController::MethodNotAllowed)
end
end
end
end
end # assert_route_recognition

Of course your “testing” is progressing towards something more
“test-like.” You’re calling it “testing” so you’re probably thinking
of it as “testing.” You’re using words like “ensure” instead of
“specify” and you’ve even named your expectations “assert_xxx” instead
of “expect_xxx.”

Interesting. I’m just used to thinking that way I guess. What is wrong
with
ensure instead of specify?

And I’ve never seen that convention with the expect prefix. Can you
provide
an example?

Thank you very much for the help.

Brandon

devbanana · April 10, 2009, 3:27pm

David,

I wanted to ask about another macro I have.

I have something currently called assert_form_fields, that is called
like:

Assert_form_fields :field1, :field2, …

What it does is sets an error for each field, and specifies that the
field
should be wrapped in a div with class ‘error’, and that the other fields
are
not wrapped by such a div.

It does it by calling other methods. For each field I have
assert_[field]has_errors and assert[field]_does_not_have_errors.

So now that I’m trying to rename these, I’m just not sure what to name
this.
What should I keep in mind when trying to write appropriate names for
such
things?

Thanks,
Brandon

devbanana · April 10, 2009, 3:25pm

Brandon O. wrote:

Oh, and that described_class trick is great; thanks for showing me that.

http://c2.com/cgi/wiki?AbstractTest ?

devbanana · April 10, 2009, 3:28pm

Brandon O. wrote:

So is this necessarily bad? I’ve heard it’s not always good to reduce
duplication in tests, but it just seems much preferable than copy/paste.

Refactor production code (while passing the tests after each edit) to
make it as
DRY as possible. This typically means duplicate the behavior you need
to
change, make the new method pass the new test, and only then merge the
new
behavior back in with the old code. Merge following the rule “refactor
low
hanging fruit”. Don’t jam two big methods back together; use Extract
Method
Refactor on the smallest common lines within them, until only the
differences
are left.

This implies clone-and-modify is part of the process. In test code, the
best way
to write a new test is clone the best example and change the assertions
in the
clone. (Ripping all the old assertions out, and changing the sample data
on
principle, are both best practices here.)

Refactor test code with two further constraints: Unlike production code,
each
test case should tell a little story. And squeezing that last bit of
duplication
out of the test cases is not likely to improve the tests’ designs.

New tests should always be easy to write. If you get that, then the
refactoring
is working.