From 72a7fc67cabd80452aeaeb6d8c05d61ca05eaef1 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sun, 19 Oct 2014 21:34:50 +0100 Subject: [PATCH] Tidy up changeset comment tests and beef them up a bit --- test/controllers/changeset_controller_test.rb | 106 ++++++++++-------- test/controllers/relation_controller_test.rb | 2 +- test/fixtures/changesets.yml | 6 + test/fixtures/changesets_subscribers.yml | 3 + test/models/changeset_test.rb | 2 +- 5 files changed, 70 insertions(+), 49 deletions(-) create mode 100644 test/fixtures/changesets_subscribers.yml diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index c3fd0fadc..e4c2903e0 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -3,7 +3,7 @@ require 'changeset_controller' class ChangesetControllerTest < ActionController::TestCase api_fixtures - fixtures :changeset_comments + fixtures :changeset_comments, :changesets_subscribers ## # test all routes which lead to this controller @@ -28,14 +28,6 @@ class ChangesetControllerTest < ActionController::TestCase { :path => "/api/0.6/changeset/1", :method => :get }, { :controller => "changeset", :action => "read", :id => "1" } ) - assert_routing( - { :path => "/api/0.6/changeset/1", :method => :put }, - { :controller => "changeset", :action => "update", :id => "1" } - ) - assert_routing( - { :path => "/api/0.6/changeset/1/close", :method => :put }, - { :controller => "changeset", :action => "close", :id => "1" } - ) assert_routing( { :path => "/api/0.6/changeset/1/subscribe", :method => :post }, { :controller => "changeset", :action => "subscribe", :id => "1" } @@ -45,12 +37,16 @@ class ChangesetControllerTest < ActionController::TestCase { :controller => "changeset", :action => "unsubscribe", :id => "1" } ) assert_routing( - { :path => "/api/0.6/changeset/comment/1/hide", :method => :post }, - { :controller => "changeset", :action => "hide_comment", :id => "1" } + { :path => "/api/0.6/changeset/1", :method => :put }, + { :controller => "changeset", :action => "update", :id => "1" } ) assert_routing( - { :path => "/api/0.6/changeset/comment/1/unhide", :method => :post }, - { :controller => "changeset", :action => "unhide_comment", :id => "1" } + { :path => "/api/0.6/changeset/1/close", :method => :put }, + { :controller => "changeset", :action => "close", :id => "1" } + ) + assert_routing( + { :path => "/api/0.6/changeset/1/comment", :method => :post }, + { :controller => "changeset", :action => "comment", :id => "1" } ) assert_routing( { :path => "/api/0.6/changeset/comments/feed", :method => :get }, @@ -60,6 +56,14 @@ class ChangesetControllerTest < ActionController::TestCase { :path => "/api/0.6/changeset/1/comments/feed", :method => :get }, { :controller => "changeset", :action => "comments_feed", :id => "1", :format =>"rss" } ) + assert_routing( + { :path => "/api/0.6/changeset/comment/1/hide", :method => :post }, + { :controller => "changeset", :action => "hide_comment", :id => "1" } + ) + assert_routing( + { :path => "/api/0.6/changeset/comment/1/unhide", :method => :post }, + { :controller => "changeset", :action => "unhide_comment", :id => "1" } + ) assert_routing( { :path => "/api/0.6/changesets", :method => :get }, { :controller => "changeset", :action => "query" } @@ -177,14 +181,18 @@ class ChangesetControllerTest < ActionController::TestCase # document structure. def test_read changeset_id = changesets(:normal_user_first_change).id - get :read, :id => changeset_id, :format => :xml + get :read, :id => changeset_id assert_response :success, "cannot get first changeset" assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1 assert_select "osm>changeset[id=#{changeset_id}]", 1 assert_select "osm>changeset>discussion", 0 - get :read, :id => changeset_id, :format => :xml, :include_discussion => true + get :read, :id => changeset_id, :include_discussion => true + assert_response :success, "cannot get first changeset with comments" + + assert_select "osm[version=#{API_VERSION}][generator=\"OpenStreetMap server\"]", 1 + assert_select "osm>changeset[id=#{changeset_id}]", 1 assert_select "osm>changeset>discussion", 1 end @@ -1414,8 +1422,7 @@ EOF end # get the bounding box back from the changeset - get :read, :id => changeset_id, :format => :xml - + get :read, :id => changeset_id assert_response :success, "Couldn't read back changeset." assert_select "osm>changeset[min_lon=1.0]", 1 assert_select "osm>changeset[max_lon=1.0]", 1 @@ -1430,7 +1437,7 @@ EOF end # get the bounding box back from the changeset - get :read, :id => changeset_id, :format => :xml + get :read, :id => changeset_id assert_response :success, "Couldn't read back changeset for the second time." assert_select "osm>changeset[min_lon=1.0]", 1 assert_select "osm>changeset[max_lon=2.0]", 1 @@ -1446,7 +1453,7 @@ EOF end # get the bounding box back from the changeset - get :read, :id => changeset_id, :format => 'xml' + get :read, :id => changeset_id assert_response :success, "Couldn't read back changeset for the third time." # note that the 3.1 here is because of the bbox overexpansion assert_select "osm>changeset[min_lon=1.0]", 1 @@ -1535,11 +1542,11 @@ EOF basic_authorization "test@openstreetmap.org", "test" get :query, :user => users(:normal_user).id assert_response :success, "can't get changesets by user ID" - assert_changesets [1,3,6] + assert_changesets [1,3,6,8] get :query, :display_name => users(:normal_user).display_name assert_response :success, "can't get changesets by user name" - assert_changesets [1,3,6] + assert_changesets [1,3,6,8] # check that the correct error is given when we provide both UID and name get :query, :user => users(:normal_user).id, :display_name => users(:normal_user).display_name @@ -1567,11 +1574,11 @@ EOF get :query, :closed => 'true' assert_response :success, "can't get changesets by closed-ness" - assert_changesets [3,5,6,7] + assert_changesets [3,5,6,7,8] get :query, :closed => 'true', :user => users(:normal_user).id assert_response :success, "can't get changesets by closed-ness and user" - assert_changesets [3,6] + assert_changesets [3,6,8] get :query, :closed => 'true', :user => users(:public_user).id assert_response :success, "can't get changesets by closed-ness and user" @@ -1879,7 +1886,7 @@ EOF basic_authorization(users(:public_user).email, 'test') assert_difference('ChangesetComment.count') do - post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment', :format => :xml } + post :comment, { :id => changesets(:normal_user_closed_change).id, :text => 'This is a comment' } end assert_response :success end @@ -1925,7 +1932,7 @@ EOF changeset = changesets(:normal_user_closed_change) assert_difference('changeset.subscribers.count') do - post :subscribe, { :id => changeset.id, :format => :xml } + post :subscribe, { :id => changeset.id } end assert_response :success end @@ -1933,33 +1940,32 @@ EOF ## # test subscribe fail def test_subscribe_fail + # unauthorized changeset = changesets(:normal_user_closed_change) assert_no_difference('changeset.subscribers.count') do - post :subscribe, { :id => changeset.id, :format => :xml } + post :subscribe, { :id => changeset.id } end assert_response :unauthorized basic_authorization(users(:public_user).email, 'test') + # bad changeset id assert_no_difference('changeset.subscribers.count') do post :subscribe, { :id => 999111 } end assert_response :not_found + # not closed changeset changeset = changesets(:normal_user_first_change) assert_no_difference('changeset.subscribers.count') do post :subscribe, { :id => changeset.id } end assert_response :conflict - # subscribing - changeset = changesets(:normal_user_closed_change) - post :subscribe, { :id => changeset.id, :format => :xml } - assert_response :success - - # trying to subsrcirbe one more time + # trying to subscribe when already subscribed + changeset = changesets(:normal_user_subscribed_change) assert_no_difference('changeset.subscribers.count') do - post :subscribe, { :id => changeset.id, :format => :xml } + post :subscribe, { :id => changeset.id } end assert_response :conflict end @@ -1968,11 +1974,10 @@ EOF # test unsubscribe success def test_unsubscribe_success basic_authorization(users(:public_user).email, 'test') - changeset = changesets(:normal_user_closed_change) - post :subscribe, { :id => changeset.id, :format => :xml } - # unsubscribe + changeset = changesets(:normal_user_subscribed_change) + assert_difference('changeset.subscribers.count', -1) do - post :unsubscribe, { :id => changeset.id, :format => :xml } + post :unsubscribe, { :id => changeset.id } end assert_response :success end @@ -1980,6 +1985,7 @@ EOF ## # test unsubscribe fail def test_unsubscribe_fail + # unauthorized changeset = changesets(:normal_user_closed_change) assert_no_difference('changeset.subscribers.count') do post :unsubscribe, { :id => changeset.id } @@ -1988,11 +1994,13 @@ EOF basic_authorization(users(:public_user).email, 'test') + # bad changeset id assert_no_difference('changeset.subscribers.count', -1) do post :unsubscribe, { :id => 999111 } end assert_response :not_found + # not closed changeset changeset = changesets(:normal_user_first_change) assert_no_difference('changeset.subscribers.count', -1) do post :unsubscribe, { :id => changeset.id } @@ -2010,23 +2018,25 @@ EOF ## # test hide comment fail def test_hide_comment_fail + # unauthorized comment = changeset_comments(:normal_comment_1) assert('comment.visible') do - post :hide_comment, { :id => comment.id, :format => "xml" } + post :hide_comment, { :id => comment.id } assert_response :unauthorized end - basic_authorization(users(:public_user).email, 'test') + # not a moderator assert('comment.visible') do - post :hide_comment, { :id => comment.id, :format => "xml" } + post :hide_comment, { :id => comment.id } assert_response :forbidden end basic_authorization(users(:moderator_user).email, 'test') - post :hide_comment, { :id => 999111, :format => "xml" } + # bad comment id + post :hide_comment, { :id => 999111 } assert_response :not_found end @@ -2038,7 +2048,7 @@ EOF basic_authorization(users(:moderator_user).email, 'test') assert('!comment.visible') do - post :hide_comment, { :id => comment.id, :format => "xml" } + post :hide_comment, { :id => comment.id } end assert_response :success end @@ -2046,23 +2056,25 @@ EOF ## # test unhide comment fail def test_unhide_comment_fail + # unauthorized comment = changeset_comments(:normal_comment_1) assert('comment.visible') do - post :unhide_comment, { :id => comment.id, :format => "xml" } + post :unhide_comment, { :id => comment.id } assert_response :unauthorized end - basic_authorization(users(:public_user).email, 'test') + # not a moderator assert('comment.visible') do - post :unhide_comment, { :id => comment.id, :format => "xml" } + post :unhide_comment, { :id => comment.id } assert_response :forbidden end basic_authorization(users(:moderator_user).email, 'test') - post :unhide_comment, { :id => 999111, :format => "xml" } + # bad comment id + post :unhide_comment, { :id => 999111 } assert_response :not_found end @@ -2074,7 +2086,7 @@ EOF basic_authorization(users(:moderator_user).email, 'test') assert('!comment.visible') do - post :unhide_comment, { :id => comment.id, :format => "xml" } + post :unhide_comment, { :id => comment.id } end assert_response :success end diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index ee712f748..66d601284 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -859,7 +859,7 @@ OSM # now download the changeset to check its bounding box with_controller(ChangesetController.new) do - get :read, :id => changeset_id, :format => :xml # TODO why it doesn't use defaults? + get :read, :id => changeset_id assert_response :success, "can't re-read changeset for modify test" assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}" assert_select "osm>changeset[id=#{changeset_id}]", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}" diff --git a/test/fixtures/changesets.yml b/test/fixtures/changesets.yml index d80b517fe..3cfec5bbb 100644 --- a/test/fixtures/changesets.yml +++ b/test/fixtures/changesets.yml @@ -69,3 +69,9 @@ too_many_elements_changeset: max_lat: <%= 4 * SCALE %> num_changes: <%= Changeset::MAX_ELEMENTS + 1 %> +normal_user_subscribed_change: + id: 8 + user_id: 1 + created_at: "2007-01-01 00:00:00" + closed_at: "2007-01-02 00:00:00" + num_changes: 0 diff --git a/test/fixtures/changesets_subscribers.yml b/test/fixtures/changesets_subscribers.yml new file mode 100644 index 000000000..f46c688a2 --- /dev/null +++ b/test/fixtures/changesets_subscribers.yml @@ -0,0 +1,3 @@ +t1: + changeset_id: 8 + subscriber_id: 2 diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb index b6e5ba46a..715a90b92 100644 --- a/test/models/changeset_test.rb +++ b/test/models/changeset_test.rb @@ -4,7 +4,7 @@ class ChangesetTest < ActiveSupport::TestCase api_fixtures def test_changeset_count - assert_equal 7, Changeset.count + assert_equal 8, Changeset.count end def test_from_xml_no_text -- 2.43.2