Glad You're Ready. Let's Get Started!

Let us know how we can contact you.

Thank you!

We'll respond shortly.

  • Blog Navigation

Imagine for a moment that you run a big, important company. It’s important to you that your big, important company be successful at promoting, manufacturing, and distributing your big, important product, so you have decreed that the company must show a profit each and every quarter. In fact, your internal accounting software enforces this. For example:

describe "QuarterlyReportsController#create" do
  it "should reject quarterly reports that show a net loss" do
    post :create, :quarterly_report => { :net => -100 }
    response.response_code.should == 400

Ignoring the somewhat misguided domain requirements, this test is wrong because it probably won’t fail when it should. It’s an example of a problem in Rails controller testing that bites everyone sooner or later.

The problem is that HTTP requests don’t send their parameters as integers, or booleans, or Date objects. No, HTTP request parameters are just big piles of strings. Rails does a good job of hiding the process of converting these strings into the integers and booleans and Date objects that make sense in your domain, but ActiveRecord handles that little bit of sleight of hand (using the column types from your database schema), not ActionController.

So, when you execute that test up above, the value of params[:quarterly_report][:net] will be the integer value -100, which is a value that the controller will never receive from a real HTTP request. This test fails to test a real case.

Now, if you try to use this value as an integer, either in the controller or by overriding #net= in the model, the test will still pass. But, as soon as you send a real request to the controller (hopefully not in production) you’ll find yourself on the business end of a 500 response. The test is broken.

In order to prevent this sort of brokenness I wrote a small patch to Rails (available here), which was summarily, and I will admit not unexpectedly, rejected. After that, I turned the patch into a tiny plugin called Wapcaplet (available here). It simply checks the parameters you pass to functional tests, and throws a friendly exception if you pass something with an inappropriate type. It accepts strings and instances of TestUploadFile in all cases. For parameter that Rails uses for routing it will also accept subclasses of ActiveRecord::Base.

Some examples:

# POST users/
post :create, :user => { :likes_ice_cream => true } # BOOM
post :create, :user => { :likes_ice_cream => "1" } # OK

post :create, :user => { :best_friend => @other_user } #BOOM
post :create, :user => { :best_friend_id => } # BOOM
post :create, :user => { :best_friend_id => } #OK

# GET users/:id
get :show, :id => # BOOM
get :show, :id => @user.to_param # OK
get :show, :id => @user # OK

# PUT users/:user_id/profile
put :update,
  :user_id => @user,
  :profile => { :photo =>"some/file" } # BOOM
put :update,
  :user_id => @user,
  :profile => { :photo => fixture_file_upload("some/file", "image/jpg" } #OK

Incidentally, notice that this will catch the pathological case, which seems to afflict every Rails developer, in which you pass #id rather than #to_param for a routing parameter.

In anticipation of the misguided comments that I know will come, no, it would not be better to have the plugin (or patch) call #to_s or #to_param on every incoming parameter to functional tests. Consider the example of boolean values:

true.to_s # => "true"
false.to_s # => "false"
true.to_param # => true
false.to_param # => false

Neither #to_s nor #to_param return a value likely to appear in a real HTTP request. It doesn’t take much imagination to come up with other examples of types that would not convert to meaningful request values. Worse, it takes only a little more imagination to come up with a scenario in which the implicit string conversion in the test would create subtlely wrong behavior that would be an enormous pain to track down.

So, take Wapcaplet out for a spin, I hope it saves you some time. Special thanks to Parker for getting bit by this problem enough times to get angry and demand I fix it.

  • i really hate the problem too, but i think that the solution could be improved.

    How about just raising a warning (so ppl can migrate their old codebase easily) or just converting every integer to string and every true to ‘true’, so users do not need to worry about whats happening internally.

    Anyway, great plugin, still wonder why this problem was not solved already…

  • Alex Chaffee

    “throws a friendly exception” — I haven’t looked at the code yet, but if your exception does not extend FriendlyException i will be sorely disappointed!

    grosser — it wasn’t solved because many people (specifically Rails Core members) don’t see it as a problem. Adam and Parker have worked on enough serious, long-lived, well-tested Rails codebases to realize you can’t play fast and loose with input and output processing. Hopefully if the community raises a ruckus we can help educate the team that brought us “HashWithIndifferentAccess” and “h”.

    (I hope I’m not coming off as too snarky there — I’m actually pleased that DHH grudgingly admitted he was originally wrong about “h” at RailsConf, and I’m honestly optimistic about the community twisting the Core team’s arms to get other misbehaviors fixed.)

  • Alex Chaffee

    Adam – wth does “Wapcaplet” mean?

  • Josh Susser

    @alex: it’s from [Monty Python’s “string” sketch](

Share This