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
On Fri, Apr 10, 2009 at 1:14 AM, Brandon Olivares <programmer2188 at 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> Brandon > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
> -----Original Message----- > From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > bounces at 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
On Fri, Apr 10, 2009 at 4:38 AM, Brandon Olivares <programmer2188 at gmail.com> wrote:> > >> -----Original Message----- >> From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- >> bounces at 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_recognitionI 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>> 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?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> Thank you very much for the help. > > Brandon > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >
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> -----Original Message----- > From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > bounces at rubyforge.org] On Behalf Of David Chelimsky > Sent: Friday, April 10, 2009 3:57 AM > To: rspec-users > Subject: Re: [rspec-users] Eliminating duplication from tests > > On Fri, Apr 10, 2009 at 4:38 AM, Brandon Olivares > <programmer2188 at gmail.com> wrote: > > > > > >> -----Original Message----- > >> From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > >> bounces at 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 > > 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 > > >> 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? > > 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 > > > Thank you very much for the help. > > > > Brandon > > > > _______________________________________________ > > rspec-users mailing list > > rspec-users at rubyforge.org > > http://rubyforge.org/mailman/listinfo/rspec-users > > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users
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.
Brandon Olivares wrote:> Oh, and that described_class trick is great; thanks for showing me that.http://c2.com/cgi/wiki?AbstractTest ?
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> -----Original Message----- > From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > bounces at rubyforge.org] On Behalf Of David Chelimsky > Sent: Friday, April 10, 2009 3:57 AM > To: rspec-users > Subject: Re: [rspec-users] Eliminating duplication from tests > > On Fri, Apr 10, 2009 at 4:38 AM, Brandon Olivares > <programmer2188 at gmail.com> wrote: > > > > > >> -----Original Message----- > >> From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > >> bounces at 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 > > 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 > > >> 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? > > 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 > > > Thank you very much for the help. > > > > Brandon > > > > _______________________________________________ > > rspec-users mailing list > > rspec-users at rubyforge.org > > http://rubyforge.org/mailman/listinfo/rspec-users > > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users
> -----Original Message----- > From: rspec-users-bounces at rubyforge.org [mailto:rspec-users- > bounces at rubyforge.org] On Behalf Of Phlip > Sent: Friday, April 10, 2009 8:39 AM > To: rspec-users at rubyforge.org > Subject: Re: [rspec-users] Eliminating duplication from tests > > 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.I already factor when I see duplication in the code, but I haven''t ever seen what you are saying about duplicating the behavior in a new method. Usually I just change the existing method. Can you explain this a bit further?> > 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.Thank you. That helps a lot. Brandon