Now that I’ve built up a few shoulda helpers for testing the different authentication APIs in Redmine, I can now start testing the actual bug reported.

Creating a standard test macro for all APIs

The first thing I did was to create a standard test macro for all of the different API authentication types. Otherwise I would have to type out all three shoulda macros for each controller and action. I came up with a simple should_allow_api_authentication macro.

1
2
3
4
5
6
7
8
9
10
11
12
13
14

  # Test that a request allows the three types of API authentication
  #
  # * HTTP Basic with username and password
  # * HTTP Basic with an api key for the username
  # * Key based with the key=X parameter
  #
  # @param [Symbol] http_method the HTTP method for request (:get, :post, :put, :delete)
  # @param [String] url the request url
  # @param [optional, Hash] parameters additional request parameters
  def self.should_allow_api_authentication(http_method, url, parameters={})
    should_allow_http_basic_auth_with_username_and_password(http_method, url, parameters)
    should_allow_http_basic_auth_with_key(http_method, url, parameters)
    should_allow_key_based_auth(http_method, url, parameters)
  end

Update Issues #index

Next I wanted to use this method to make sure IssuesController#index is correctly allowing all authentication types. After reviewing the existing code, I found a subtle bug in the test.

1
2
3
4
5
6
7
8

  context "/index.xml" do
    setup do
      get '/issues.xml'
    end
 
    should_respond_with :success
    should_respond_with_content_type 'application/xml'
  end

All that the test is doing here is checking that the issues.xml page loaded and returned an XML content type. Never mind the fact that Redmine will always return a page on that action, showing the list of issues you can see (or the ones the Anonymous user can see). So instead of “testing” the global issues list which always returns, I changed it to test the issues list for a private project which will require authentication.

1
2
3

  context "/index.xml" do
    should_allow_api_authentication(:get, "/projects/private-child/issues.xml")
  end

The other nice thing about using my new should_allow_api_authentication is that it creates 23 tests.

Yes… a one line macro creates 23 separate tests that test all of the major cases and many of the minor edge cases. That’s a bit better than the 2 cases the old test had.

Update Issues #show

I also wanted to update IssuesController#show to make sure it’s supporting the authentication (remember, the bug is about some actions not supporting all authentication types). Once again, I swapped in my should_allow_api_authentication macro and was able to throw away a lot of custom code.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21

  context "/issues/1.xml" do
    setup do
      get '/issues/1.xml'
    end
 
    should_respond_with :success
    should_respond_with_content_type 'application/xml'
  end
 
  context "/issues/1.json" do
    setup do
      get '/issues/1.json'
    end
 
    should_respond_with :success
    should_respond_with_content_type 'application/json'
 
    should 'return a valid JSON string' do
      assert ActiveSupport::JSON.decode(response.body)
    end
  end

New Code:

1
2
3
4
5
6
7

  context "/issues/6.xml" do
    should_allow_api_authentication(:get, "/issues/6.xml")
  end
 
  context "/issues/6.json" do
    should_allow_api_authentication(:get, "/issues/6.json")
  end

Once again, Issue 1 is public so this wasn’t going through any authentication at all. Issue 6 (new code) is on a private project so this test exercises all of the authentication methods.

Other actions

I was going to continue onto the other Issue actions but I noticed that I’ll need to make my macros more generic first. For example, right now the macros are making sure the request was successful (HTTP 200) but that won’t work when creating a new issue because it returns the created response (HTTP 201). I’m going to tackle issue creation tomorrow.

The full commit of these changes is online. If you check the test_helper.rb I also added a few tests to make sure that the response is a valid XML or JSON string.