Saturday, March 14, 2015

TMI from RSpec

   When using RSpec, there is a small annoyance waiting to getcha, when you compare numbers.

   TL;DR: using "be" (or "equal", which does exactly the same thing) produces distracting and usually meaningless extra verbiage, while "eq" (or "eql", which is a little different but close enough for this purpose) does not.

   Frex, if you are sending an HTTP request and expecting to get back the OK status code, you might say:

    expect(response.status).to be 200
   This will work fine, and so long as the status actually is 200, then all is right with the world.

   The minor, trivial, nitpicky, but still a bit annoying, problem, comes when the status is not 200.  Sure, you'll still get a test failure... but you'll get some added little distractions, the bold parts here:

    Failure/Error: expect(response.status).to be 200

       expected #<Fixnum:401> => 200
            got #<Fixnum:1001> => 500

       Compared using equal?, which compares object identity,
       but expected and actual are not the same object. Use
       `expect(actual).to eq(expected)` if you don't care about
       object identity in this example.
   All you really needed is this:

       expected: 200
            got: 500

       (compared using ==)
   (Okay, granted, you didn't really need to know it was compared using "==", but still, that's not too bad... at least it's a lot less distracting verbiage to wade through than the prior message.)

   (And okay, granted, four short lines of additional feedback aren't that much of a pain... but still, they add up.  If you're new on a project with lots of failing tests, as I am now, it can make it harder to wade through all the failures and see which ones actually mean anything.)

   So how do you get rid of it?  You might try using "==" instead of "be", especially if you're used to RSpec's older "should" syntax rather than the new "expect" syntax (which I'll ass-u-me you are now using)... but then you'd see:

     Failure/Error: expect(response.status).to == 200
       The expect syntax does not support operator matchers, so you must pass a matcher to `#to`.
   But despair not, dear reader, all is not lost!  It's still very simple.  Instead of "be", use "eq".  Then you get:

     Failure/Error: expect(response.status).to eq 200

       expected: 200
            got: 500

       (compared using ==)
   As you may recall from my Ruby Gotchas presentation, there are several ways to compare things in Ruby:

  • "==" compares the value, without regard for the actual class(es) of the objects.  So, frex, "1.0 == 1" is true.  This is what RSpec uses when you say "eq" (or "==" in the older syntax); hence the message "(compared using ==)".

  • ".eql?" compares the value and class!  So, "1.0.eql? 1" is false!

  • ".equal?" is object identity.  That is, even if two things have the same value, and the same class, if they're not the exact same object, they are not ".equal?".  One of the oddities of the Ruby object system is that an object is created for each new unique Float value, so the result of:

        x = 1.0
        y = 1.0
        x.equal? y
    is indeed true!  However, if we use a more complex class, like String, we can see that "'foo'.equal? 'foo'" is false.
   You may now be wondering, what if you try "expect(response.status).to eql 200"?  Pretty much the same thing, just now it says "(compared using eql?)".

   But... if you take it one step further, and use "expect(response.status).to equal 200"... then you're back to Square One.  That shouldn't surprise any of you who paid close attention to the original verbose error message: it told you that when we used "be", it "Compared using equal?, which compares object identity".

Saturday, February 21, 2015

Case Study: TDD Is Worth It!

   One of my most popular blog posts, counting its reading elsewhere, is Is TDD Worth It?  Recently I had a great example of how, to quote myself, "It's almost like giving you guard rails."

   Suppose the application domain is as follows:
  • Things have Parts.
  • Parts have Types, which have Categories.
  • Within a given Thing, Parts are used with various specific Connectors.
   I had been asked to develop functions to find the Things that use a given Connector, and:
  • Parts of a given Type or Category.
  • Parts of any Type or Category out of a set of Types or Categories.
  • Parts of all the Types or Categories out of a set of them.
  • The negatives of all of the above, i.e., those using:
    • no Parts of that Type or Category,
    • no Parts of any Type or Category of a set,
    • and those not using Parts of all (though they may use some) Types or Categories of a given set.
   The problem is, I had been given these requirements one at a time, so I thought the client wanted a bunch of methods on Thing with names like:
  • find_with_connector_and_part_type(conn_type, part_type)
  • find_with_connector_and_any_part_types(conn_type, part_types)
  • find_with_connector_and_all_part_types(conn_type, part_types)
  • find_with_connector_and_part_category(conn_type, part_category)
  • find_with_connector_and_any_part_categories(conn_type, part_categories)
  • find_with_connector_and_all_part_categories(conn_type, part_categories)
  • find_without_connector_and_part_type(conn_type, part_type)
  • find_without_connector_and_any_part_types(conn_type, part_types)
  • find_without_connector_and_all_part_types(conn_type, part_types)
  • find_without_connector_and_part_category(conn_type, part_category)
  • find_without_connector_and_any_part_categories(conn_type, part_categories)
  • find_without_connector_and_all_part_categories(conn_type, part_categories)
   I kept the code as DRY as I could, making the negatives simply call the positives and invert the finding, and making the singles pass a single-element set to the "any" case. That still left a lot of structural duplication.  Since they were class-methods used as scopes, they all looked like this:
def self.find_with_connector_and_part_SOMETHING(conn_type,
  id_query = assorted_magic_happens_here(several_lines)
  where(id: id_query)
  # negative version: where("id NOT IN (?)", id_query)
   That got me thinking about how to combine this set of literally a dozen different functions into one, something like Thing.find_with_parts(part_options), where part_options would include what Connector, what Types (and whether we want all or just any, which would be the default), what Categories (ditto), and whether to invert the finding.  When the client later said they didn't in fact want a bunch of separate methods, I was ready, and had a lot of the idea already thought out.

   But... how do I get from Point A to Point B?  This is where TDD came in handy!

   Of course, I had been TDD'ing the original functions, using the classic "red, green, refactor" cycle.  (Actually, I use a variant that adds a step: "refactor the tests".)  So, I substituted a simple call to my proposed new function, for the guts of one of the old ones:
def self.find_with_connector_and_part_type(conn_type,
  self.find_with_parts(connector_type: conn_type,
                       part_types: [part_type])
and reran its tests.  Of course it failed, as I hadn't written find_with_parts yet... but that came pretty easily, based on the logic that had previously found Things having Parts of any of several Types, used with the given Connector.  That test quickly passed.

   Long story short, I followed this pattern, over and over:
  1. Substitute a call to find_with_parts for the guts of a specific method.
  2. Run its test.
  3. If it works, break it by changing find_with_parts!  You always want to start with a failing test!
  4. Fix find_with_parts to make the test pass.
  5. Run the rest of the tests!
  6. If any of them fail, go back to fixing find_with_parts.
When all the old methods had had their guts substituted, I knew the new method could do it all, so I ripped the old methods out.

   Had it not been for having an existing test suite, which I had because I had TDD'ed the original code, I would have had to be a lot more slow, careful, and methodical in that process.  Instead, I could just quickly try what came to mind, and see if it worked without breaking anything else.

   The resulting function, including some extracted functions to keep that code dry, runs to a mere 43 lines of code, and is structured in such a way as to make it very easy to add additional items to filter on, such as the Material a Part Type is made of, the color a given Part is painted when used in that Thing, etc.

   (Yes, yes, I could have created the test suite just before embarking on this... but seriously, what are the chances?  Most developers would not bother, for something smallish like this.  Perhaps for a larger code archaeology expedition, where writing a test suite to ensure continuing the current behavior, whether correct or not, is a common first step.)