On Nov 21, 2007 10:22 AM, Chris O. [email protected] wrote:
As for views I fail to see why testing it with a mock model does
anything. Nothing is ensuring that when changes are made to the model
that they will also be done to the mocks then causing the test to break.
If anything, having the false security when all tests pass is worse, in
that it may prevent you from double checking something crucial.
Keep in mind that testing is about design, rather than testing. When
your specs are all green, it means that your code is in a more-or-less
stable state, not that it’s correct.
Rails encourages you to do funky stuff like
user.company.sites.build params[:site]
That’s going to be a PITA to write a spec for. In my experience, if
something is hard to spec, it’s probably not the best design.
Compare
describe SitesController, " POST /companies/acme/sites" do
include UserSpecHelpers
before(:each) do
login_as mock_user
@mock_company = mock_model(Company)
Company.stub!(:find_by_nickname).and_return @mock_company
mock_user.stub!(:company).and_return @mock_company
@sites_proxy = mock(“sites proxy”)
@mock_company.stub!(:sites).and_return @sites_proxy
@mock_site = mock_model(Site)
@sites_proxy.stub!(:build).and_return @mock_site
end
def do_post
post :create, :company_id => “acme”, :site => {“name” => “foo”}
end
it “should find the company” do
Company.should_receive(:find_by_nickname).with(“acme”).and_return
@mock_company
do_post
end
assume that we do something like found_company == user.company
implicitly checking call because we’ve stubbed all the associations
it “should build a site” do
@sites_proxy.should_receive(:build).with(“name” =>
“foo”).and_return @mock_site
do_post
end
it “should save the site” do
@mock_site.should_receive(:save).and_return true
do_post
end
end
to
describe SitesController " POST /companies/acme/sites" do
include UserSpecHelpers
before(:each) do
login_as mock_user
@mock_company = mock_model(Company, :add_site => true)
Company.stub!(:find_by_nickname).and_return @mock_company
mock_user.stub!(:access_company?).and_return true
Site.stub!(:new).and_return :mock_site
end
def do_post
post :create, :company_id => “acme”, :site => {“name” => “foo”}
end
it “should find the company” do
Company.should_receive(:find_by_nickname).with(“acme”).and_return
@mock_company
do_post
end
it “should see if the user has access to the company” do
@mock_user.should_receive(:access_company?).with(@mock_company).and_return
true
do_post
end
it “should create a site” do
Site.should_receive(:new).with(“name” => “foo”).and_return
:mock_site
do_post
end
it “should add the site to the company” do
@mock_company.should_receive(:add_site).with(:mock_site).and_return
true
do_post
end
end
The first one works, but it’s ugly. We have to do a lot of setup, and
then the specs just aren’t very expressive. The controller is going
to be coupled to the low-level model structure, and we didn’t really
do anything to help ourselves.
The second one is much, much nicer. First of all, there’s less setup,
and the setup that’s there is much more clear. We’re just stubbing
out some methods, rather than hooking up a bunch of relationships.
The specs themselves are actually valuable. They read well, and,
assuming that we wrote the spec and controller before the underlying
model, they helped us discover a nice API. I think that
#access_company? and #add_site will prove to be quite useful.
It’s really easy to fall into the trap of writing controller code that
would lead to the first spec I showed. I even wrote those kinds of
specs when I first started! I was thinking too much about what I
“knew” to be the final implementation, rather than focusing on just
writing the specs in a fluid, natural way, guiding me towards the
final implementation. Eventually after writing all those ugly,
painful specs you realize that there’s probably a better way, and you
come on this list asking for help. People offer alternative
approaches and hopefully it snaps.
I think tests for views makes sense when there is a condition that must
be tested, ex. if a new member must fill in additional fields that are
made visible only when they are from a certain country. A test may then
make sense to ensure that the fields exist.
I didn’t do view specs for the longest time. Recently though I’ve
learned that they are important. I have a bad habit of working hard
to make the controller and model clean and well-designed, and then
just jamming a bunch of stuff in the view. I mean, it’s just the
view, right? There’s not a lot going on, and the designers are
supposed to deal with that stuff anyway.
As it turns out, it’s incredibly easy to write trainwreck code in the
view too! So writing view specs helps you avoid that, again. Just
like controller specs, view specs will help guide you to push more
things down into the model.
Also, I think the combo of view + controller specs help you make
design decisions like when to refactor to a Presenter. For example,
you’ve got a controller which pulls a couple things out of an order
object - the user, shipping address, billing address, and the line
items. As your app grows, you might end up with 5 or 6 specs, where
you have to stub out all of that stuff. You could refactor the specs
themselves, writing little helper methods to hook all of that up for
you. Or you could refactor the production code so you do something
like “@presenter = OrderPresenter.new(order)”. Then you can replace
those 5 slightly different specs with one generic spec that uses a
Presenter, and let the Presenter handle any details. I’ve done this
several times, and I don’t think I’d arrive at that solution if I
didn’t use the test style that I do.
There are no hard-and-fast rules, but as a general rule, I find that
if something is hard to spec then it’s probably not a great design.
Ultimately good design is about being able to easily use your code,
and change it when needed. Thorough testing, using mocks, helps you
first by defining how your code will be used, and second by pointing
out sources of pain when you try to make changes.
Pat
