Wednesday, January 14, 2009

My first Rails bug – accepted!

At the time that I posted about My first Rails bug the tests that existed for Action Pack contained failures and the only test in integration_upload_test.rb was:

assert_equal(:multipart_form, SessionUploadTest.last_request_type)

I felt that my change could be submitted with no tests because:

  • The change was trivial (6 characters)
  • The change was for a well known Windows file issue
  • The tests that existed didn’t pass
  • There were no example tests for me to build on

Three months after I submitted the ticket, Pratik dismissed it by changing its state to incomplete with the comment “Missing a test case.”  Are these guys CRAZY?  No, no they are not.  Just because there were broken windows in the neighborhood did not give me the right to break any more.

So, I sucked it up, cloned the latest rails repository again and to my surprise … all the Action Pack tests passed! … and there was a nice simple example in multipart_params_parsing_test.rb:

  test "uploads and reads file" do
    with_test_routing do
      post '/read', :uploaded_data => fixture_file_upload(FIXTURE_PATH + "/hello.txt", "text/plain")
      assert_equal "File: Hello", response.body
    end
  end

This is too easy!  I added a test for my issue:

  test "uploads and reads a binary file in windows" do
    with_test_routing do
      fixture_file = FIXTURE_PATH + "/mona_lisa.jpg"
      post '/read', :uploaded_data => fixture_file_upload(fixture_file, "image/jpg")
      assert_equal 'File: '.length + File.size(fixture_file), response.content_length
    end
  end

Watched it fail.  Added the magic 6 characters (“, ‘rb’”).  Watched it pass.  Wrapped it up and shipped it off.  The following morning I pick up an email letting me know that Pratik has reopened the ticket and assigned it to Joshua Peek for review.  The change was committed before the day was out!

Many thanks to Pratik and Josh for attending to this so promptly.

1 comment :

  1. Congrats, I still haven't been able to submit a patch to the Rails core. I have a few ideas but haven't been able to find the time to package them up.

    ReplyDelete