Test Integrity November 5th, 2007

CruiseControl.rb is a wonderful tool. Once setup, every time you commit a change your app's entire test suite is run. If anything fails, it notifies everyone via email, with a followup when it's fixed.

In addition to providing continuous integration, it creates a social stigma when someone makes a careless commit.

I've put two apps on Cruise Control without problem. When I went to add a third project, which I'm not developing, I ran its test suite for the first time and noticed two failures. I tracked them back over 1,500 revisions, months of work. That's as far a I could go before our repository restructure, so I gave up.

I asked one of the team members about it. They said, "Oh. Those are the two tests that always fail."

Sometimes words speak louder than actions.

Formatting September 20th, 2007

You can tell a lot about a person by their formatting.

<li class="first">
<% if listing.rateable? %><%= logged_link_to :rate_it, listing, "Rate it", new_rating_url(listing.listing_id), :onclick => 'return signInOrPopup(this.href);' %><% else %>Rate it<% end -%></li><%if listing.has_ratings? %><li><%= logged_link_to :read_reviews, listing, "Read Reviews", more_info_reviews_url(listing) %></li><% else %><li>Read Reviews</li><% end %>

This tells me: "WATCH OUT!"

Step 1: What the hell is going on?

With a little cleaning, we can survey the mess.

<li class="first">
    <% if listing.rateable? %>
        <%= logged_link_to :rate_it, listing, "Rate it", new_rating_url(listing.listing_id), :onclick => 'return signInOrPopup(this.href);' %>
    <% else %>
        Rate it
    <% end %>
</li>
<% if listing.has_ratings? %>
    <li>
        <%= logged_link_to :read_reviews, listing, "Read Reviews", more_info_reviews_url(listing) %>
    </li>
<% else %>
    <li>Read Reviews</li>
<% end %>

That's better.

It's a simple pattern: "If that link applies to the record, make it a link. Otherwise, make it plain text." There should be a helper for that. Like link_to_if.

link_to_if(listing.has_ratings?, "Read Reviews", more_info_reviews_url(listing))

Except, we're using our own helper for logging purposes called "logged_link_to." Writing logged_link_to_if is a sign something is wrong with our overall logging system, and we should be overriding link_to with an alias_method_chain, but that's a project for another day. I just need something so I can sleep at night.

<li class="first">
  <%=
    if listing.rateable?
      logged_link_to(:rate_it, listing,
        "Rate it", new_rating_url(listing.listing_id),
        :onclick => 'return signInOrPopup(this.href);')
    else
      'Rate it'
    end %>
</li>
<li>
  <%=
    if listing.has_ratings? 
      logged_link_to(:read_reviews, listing,
      "Read Reviews", more_info_reviews_url(listing))
    else
      'Read Reviews'
    end
   %>
</li>