Daily Refactor #59: Move Routing Tests

Next, I’m going to work on refactoring Redmine’s controllers in order to make them match the REST pattern and to take advantage of Rail’s built in REST helpers. In order to do this though, I need to refactor how Redmine’s routes are setup. Since the routes.rb file is over 290 lines long, the first thing I need to do is to make sure that the test suite is covering the main urls.

The Refactoring

Redmine’s routing tests are scattered across several functional tests, so this refactoring pulls some of them into a single RoutingTest integration test.

Before

1
2
3
4

# test/integration/routing_test.rb
class RoutingTest < ActionController::IntegrationTest
  # ... existing tests
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

# test/functional/users_controller_test.rb
class UsersControllerTest  'users', :action => 'index'
    )
    assert_routing(
      {:method => :get, :path => '/users'},
      :controller => 'users', :action => 'index'
    )
    assert_recognizes(
      {:controller => 'users', :action => 'index'},
      {:method => :get, :path => '/users'}
    )
  end
 
  def test_show_routing
    assert_routing(
      {:method => :get, :path => '/users/44'},
      :controller => 'users', :action => 'show', :id => '44'
    )
    assert_recognizes(
      {:controller => 'users', :action => 'show', :id => '44'},
      {:method => :get, :path => '/users/44'}
    )
  end
end

After

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45

# test/integration/routing_test.rb
class RoutingTest  'users', :action => 'index'
    should_route :get, "/users/44", :controller => 'users', :action => 'show', :id => '44'
    should_route :get, "/users/new", :controller => 'users', :action => 'add'
    should_route :get, "/users/444/edit", :controller => 'users', :action => 'edit', :id => '444'
    should_route :get, "/users/222/edit/membership", :controller => 'users', :action => 'edit', :id => '222', :tab => 'membership'
 
    should_route :post, "/users/new", :controller => 'users', :action => 'add'
    should_route :post, "/users/444/edit", :controller => 'users', :action => 'edit', :id => '444'
    should_route :post, "/users/123/memberships", :controller => 'users', :action => 'edit_membership', :id => '123'
    should_route :post, "/users/123/memberships/55", :controller => 'users', :action => 'edit_membership', :id => '123', :membership_id => '55'
    should_route :post, "/users/567/memberships/12/destroy", :controller => 'users', :action => 'destroy_membership', :id => '567', :membership_id => '12'
  end
 
  context "versions" do
    should_route :get, "/projects/foo/versions/new", :controller => 'versions', :action => 'new', :project_id => 'foo'
 
    should_route :post, "/projects/foo/versions/new", :controller => 'versions', :action => 'new', :project_id => 'foo'
  end
 
  context "wikis" do
    should_route :get, "/projects/567/wiki", :controller => 'wiki', :action => 'index', :id => '567'
    should_route :get, "/projects/567/wiki/lalala", :controller => 'wiki', :action => 'index', :id => '567', :page => 'lalala'
    should_route :get, "/projects/567/wiki/my_page/edit", :controller => 'wiki', :action => 'edit', :id => '567', :page => 'my_page'
    should_route :get, "/projects/1/wiki/CookBook_documentation/history", :controller => 'wiki', :action => 'history', :id => '1', :page => 'CookBook_documentation'
    should_route :get, "/projects/1/wiki/CookBook_documentation/diff/2/vs/1", :controller => 'wiki', :action => 'diff', :id => '1', :page => 'CookBook_documentation', :version => '2', :version_from => '1'
    should_route :get, "/projects/1/wiki/CookBook_documentation/annotate/2", :controller => 'wiki', :action => 'annotate', :id => '1', :page => 'CookBook_documentation', :version => '2'
    should_route :get, "/projects/22/wiki/ladida/rename", :controller => 'wiki', :action => 'rename', :id => '22', :page => 'ladida'
    should_route :get, "/projects/567/wiki/page_index", :controller => 'wiki', :action => 'special', :id => '567', :page => 'page_index'
    should_route :get, "/projects/567/wiki/Page_Index", :controller => 'wiki', :action => 'special', :id => '567', :page => 'Page_Index'
    should_route :get, "/projects/567/wiki/date_index", :controller => 'wiki', :action => 'special', :id => '567', :page => 'date_index'
    should_route :get, "/projects/567/wiki/export", :controller => 'wiki', :action => 'special', :id => '567', :page => 'export'
 
    should_route :post, "/projects/567/wiki/my_page/edit", :controller => 'wiki', :action => 'edit', :id => '567', :page => 'my_page'
    should_route :post, "/projects/567/wiki/CookBook_documentation/preview", :controller => 'wiki', :action => 'preview', :id => '567', :page => 'CookBook_documentation'
    should_route :post, "/projects/22/wiki/ladida/rename", :controller => 'wiki', :action => 'rename', :id => '22', :page => 'ladida'
    should_route :post, "/projects/22/wiki/ladida/destroy", :controller => 'wiki', :action => 'destroy', :id => '22', :page => 'ladida'
    should_route :post, "/projects/22/wiki/ladida/protect", :controller => 'wiki', :action => 'protect', :id => '22', :page => 'ladida'
  end
 
  context "administration panel" do
    should_route :get, "/admin/projects", :controller => 'admin', :action => 'projects'
  end
 
end
 
1
2
3
4

# test/functional/users_controller_test.rb
class UsersControllerTest < ActionController::TestCase
  # ...
end

Review

I didn’t show all of the changes but if you compare the before and after, it’s easy to see that using shoulda and a single integration test will make routes easier to maintain.

Reference commit