Forum: RSpec Eliminating duplication from tests

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
F85bacbbd4814799d4526b3e35a431df?d=identicon&s=25 Brandon Olivares (Guest)
on 2009-04-10 06:17
(Received via mailing list)
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
5d38ab152e1e3e219512a9859fcd93af?d=identicon&s=25 David Chelimsky (Guest)
on 2009-04-10 09:11
(Received via mailing list)
On Fri, Apr 10, 2009 at 1:14 AM, Brandon Olivares
<programmer2188@gmail.com> 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:

1. Write an example. There is no code, so the example is a
specification for code that doesn't exist.

2. 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
F85bacbbd4814799d4526b3e35a431df?d=identicon&s=25 Brandon Olivares (Guest)
on 2009-04-10 09:41
(Received via mailing list)
> -----Original Message-----
> From: rspec-users-bounces@rubyforge.org [mailto:rspec-users-
> bounces@rubyforge.org] On Behalf Of David Chelimsky
> 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
5d38ab152e1e3e219512a9859fcd93af?d=identicon&s=25 David Chelimsky (Guest)
on 2009-04-10 10:46
(Received via mailing list)
On Fri, Apr 10, 2009 at 4:38 AM, Brandon Olivares
<programmer2188@gmail.com> 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
F85bacbbd4814799d4526b3e35a431df?d=identicon&s=25 Brandon Olivares (Guest)
on 2009-04-10 14:31
(Received via mailing list)
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
Aafa8848c4b764f080b1b31a51eab73d?d=identicon&s=25 Phlip (Guest)
on 2009-04-10 15:25
(Received via mailing list)
Brandon Olivares wrote:

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

http://c2.com/cgi/wiki?AbstractTest ?
F85bacbbd4814799d4526b3e35a431df?d=identicon&s=25 Brandon Olivares (Guest)
on 2009-04-10 15:27
(Received via mailing list)
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
Aafa8848c4b764f080b1b31a51eab73d?d=identicon&s=25 Phlip (Guest)
on 2009-04-10 15:28
(Received via mailing list)
Brandon Olivares 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.
This topic is locked and can not be replied to.