From: Tom Hughes Date: Sun, 8 Mar 2015 00:48:13 +0000 (+0000) Subject: More test work X-Git-Tag: live~4211 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/7fef0353f25ecb02d400987ec30763d1ce881a12 More test work --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5f2a84fc9..ea074d52a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -37,7 +37,7 @@ Metrics/BlockNesting: # Offense count: 60 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 1552 + Max: 1628 # Offense count: 68 Metrics/CyclomaticComplexity: diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 2a8fec95e..a2efdce7b 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -254,7 +254,7 @@ class ChangesetController < ApplicationController # list edits (open changesets) in reverse chronological order def list if request.format == :atom && params[:max_id] - redirect_to params.merge(:max_id => nil), :status => :moved_permanently + redirect_to url_for(params.merge(:max_id => nil)), :status => :moved_permanently return end @@ -266,7 +266,7 @@ class ChangesetController < ApplicationController end end - if (params[:friends] || params[:nearby]) && !@user && request.format == :html + if (params[:friends] || params[:nearby]) && !@user require_user return end diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb index ed7853d6c..8ce53739f 100644 --- a/app/controllers/oauth_controller.rb +++ b/app/controllers/oauth_controller.rb @@ -38,11 +38,6 @@ class OauthController < ApplicationController protected def oauth1_authorize - unless @token - render :action => "authorize_failure" - return - end - if @token.invalidated? @message = t "oauth.oauthorize_failure.invalid" render :action => "authorize_failure" diff --git a/config/routes.rb b/config/routes.rb index f67121d25..f5b7e4e72 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -120,8 +120,8 @@ OpenStreetMap::Application.routes.draw do match "/user/:display_name/history" => "changeset#list", :via => :get match "/user/:display_name/history/feed" => "changeset#feed", :via => :get, :defaults => { :format => :atom } match "/user/:display_name/notes" => "notes#mine", :via => :get - match "/history/friends" => "changeset#list", :via => :get, :friends => true, :as => "friend_changesets" - match "/history/nearby" => "changeset#list", :via => :get, :nearby => true, :as => "nearby_changesets" + match "/history/friends" => "changeset#list", :via => :get, :friends => true, :as => "friend_changesets", :defaults => { :format => :html } + match "/history/nearby" => "changeset#list", :via => :get, :nearby => true, :as => "nearby_changesets", :defaults => { :format => :html } get "/browse/way/:id", :to => redirect(:path => "/way/%{id}") get "/browse/way/:id/history", :to => redirect(:path => "/way/%{id}/history") diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 1be10ec2a..46a10e9aa 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, :changesets_subscribers + fixtures :friends, :changeset_comments, :changesets_subscribers ## # test all routes which lead to this controller @@ -74,11 +74,11 @@ class ChangesetControllerTest < ActionController::TestCase ) assert_routing( { :path => "/history/friends", :method => :get }, - { :controller => "changeset", :action => "list", :friends => true } + { :controller => "changeset", :action => "list", :friends => true, :format => :html } ) assert_routing( { :path => "/history/nearby", :method => :get }, - { :controller => "changeset", :action => "list", :nearby => true } + { :controller => "changeset", :action => "list", :nearby => true, :format => :html } ) assert_routing( { :path => "/history", :method => :get }, @@ -1760,7 +1760,7 @@ EOF end ## - # This should display the last 20 changesets closed. + # This should display the last 20 changesets closed def test_list get :list, :format => "html" assert_response :success @@ -1768,25 +1768,15 @@ EOF assert_template :layout => "map" assert_select "h2", :text => "Changesets", :count => 1 - get :list, :format => "html", :list => "1", :bbox => "-180,-90,90,180" + xhr :get, :list, :format => "html", :list => "1" assert_response :success assert_template "list" - changesets = Changeset - .where("num_changes > 0 and min_lon is not null") - .order(:created_at => :desc) - .limit(20) - assert changesets.size <= 20 - - # Now check that all 20 (or however many were returned) changesets are in the html - assert_select "li", :count => changesets.size - changesets.each do |_changeset| - # FIXME: this test needs rewriting - test for table contents - end + check_list_result(Changeset.all) end ## - # This should display the last 20 changesets closed. + # This should display the last 20 changesets closed def test_list_xhr xhr :get, :list, :format => "html" assert_response :success @@ -1794,31 +1784,59 @@ EOF assert_template :layout => "xhr" assert_select "h2", :text => "Changesets", :count => 1 - get :list, :format => "html", :list => "1", :bbox => "-180,-90,90,180" + xhr :get, :list, :format => "html", :list => "1" assert_response :success assert_template "list" - changesets = Changeset - .where("num_changes > 0 and min_lon is not null") - .order(:created_at => :desc) - .limit(20) - assert changesets.size <= 20 + check_list_result(Changeset.all) + end - # Now check that all 20 (or however many were returned) changesets are in the html - assert_select "li", :count => changesets.size - changesets.each do |_changeset| - # FIXME: this test needs rewriting - test for table contents - end + ## + # This should display the last 20 changesets closed in a specific area + def test_list_bbox + get :list, :format => "html", :bbox => "4.5,4.5,5.5,5.5" + assert_response :success + assert_template "history" + assert_template :layout => "map" + assert_select "h2", :text => "Changesets", :count => 1 + + xhr :get, :list, :format => "html", :bbox => "4.5,4.5,5.5,5.5", :list => "1" + assert_response :success + assert_template "list" + + check_list_result(Changeset.where("min_lon < 55000000 and max_lon > 45000000 and min_lat < 55000000 and max_lat > 45000000")) end ## # Checks the display of the user changesets listing def test_list_user user = users(:public_user) + get :list, :format => "html", :display_name => user.display_name assert_response :success assert_template "history" - # FIXME: need to add more checks to see which if edits are actually shown if your data is public + + get :list, :format => "html", :display_name => user.display_name, :list => "1" + assert_response :success + assert_template "list" + + check_list_result(user.changesets) + end + + ## + # Checks the display of the user changesets listing for a private user + def test_list_private_user + user = users(:normal_user) + + get :list, :format => "html", :display_name => user.display_name + assert_response :success + assert_template "history" + + get :list, :format => "html", :display_name => user.display_name, :list => "1" + assert_response :success + assert_template "list" + + check_list_result(Changeset.none) end ## @@ -1827,33 +1845,89 @@ EOF get :list, :format => "html", :display_name => "Some random user" assert_response :not_found assert_template "user/no_such_user" + + get :list, :format => "html", :display_name => "Some random user", :list => "1" + assert_response :not_found + assert_template "user/no_such_user" + end + + ## + # Checks the display of the friends changesets listing + def test_list_friends + user = users(:normal_user) + + get :list, :friends => true + assert_response :redirect + assert_redirected_to :controller => :user, :action => :login, :referer => friend_changesets_path + + session[:user] = user.id + + get :list, :friends => true + assert_response :success + assert_template "history" + + get :list, :friends => true, :list => "1" + assert_response :success + assert_template "list" + + check_list_result(Changeset.where(:user => user.friend_users.identifiable)) + end + + ## + # Checks the display of the nearby user changesets listing + def test_list_nearby + user = users(:normal_user) + + get :list, :nearby => true + assert_response :redirect + assert_redirected_to :controller => :user, :action => :login, :referer => nearby_changesets_path + + session[:user] = user.id + + get :list, :nearby => true + assert_response :success + assert_template "history" + + get :list, :nearby => true, :list => "1" + assert_response :success + assert_template "list" + + check_list_result(Changeset.where(:user => user.nearby)) end ## - # This should display the last 20 changesets closed. + # This should display the last 20 changesets closed def test_feed - changesets = Changeset.where("num_changes > 0").order(:created_at => :desc).limit(20) - assert changesets.size <= 20 - get :feed, :format => "atom" + get :feed, :format => :atom assert_response :success assert_template "list" - # Now check that all 20 (or however many were returned) changesets are in the html - assert_select "feed", :count => 1 - assert_select "entry", :count => changesets.size - changesets.each do |_changeset| - # FIXME: this test needs rewriting - test for feed contents - end + assert_equal "application/atom+xml", response.content_type + + check_feed_result(Changeset.all) + end + + ## + # This should display the last 20 changesets closed in a specific area + def test_feed_bbox + get :feed, :format => :atom, :bbox => "4.5,4.5,5.5,5.5" + assert_response :success + assert_template "list" + assert_equal "application/atom+xml", response.content_type + + check_feed_result(Changeset.where("min_lon < 55000000 and max_lon > 45000000 and min_lat < 55000000 and max_lat > 45000000")) end ## # Checks the display of the user changesets feed def test_feed_user user = users(:public_user) - get :feed, :format => "atom", :display_name => user.display_name + + get :feed, :format => :atom, :display_name => user.display_name assert_response :success assert_template "list" assert_equal "application/atom+xml", response.content_type - # FIXME: need to add more checks to see which if edits are actually shown if your data is public + + check_feed_result(user.changesets) end ## @@ -1863,6 +1937,14 @@ EOF assert_response :not_found end + ## + # Check that we can't request later pages of the changesets feed + def test_feed_max_id + get :feed, :format => "atom", :max_id => 100 + assert_response :redirect + assert_redirected_to :action => :feed + end + ## # check that the changeset download for a changeset with a redacted # element in it doesn't contain that element. @@ -2149,9 +2231,7 @@ EOF end end - #------------------------------------------------------------ - # utility functions - #------------------------------------------------------------ + private ## # boilerplate for checking that certain changesets exist in the @@ -2195,4 +2275,38 @@ EOF xml.find("//osm/way").first[name] = value.to_s xml end + + ## + # check the result of a list + def check_list_result(changesets) + changesets = changesets.where("num_changes > 0") + .order(:created_at => :desc) + .limit(20) + assert changesets.size <= 20 + + assert_select "ol.changesets", :count => [changesets.size, 1].min do + assert_select "li", :count => changesets.size + + changesets.each do |changeset| + assert_select "li#changeset_#{changeset.id}", :count => 1 + end + end + end + + ## + # check the result of a feed + def check_feed_result(changesets) + changesets = changesets.where("num_changes > 0") + .order(:created_at => :desc) + .limit(20) + assert changesets.size <= 20 + + assert_select "feed", :count => [changesets.size, 1].min do + assert_select "entry", :count => changesets.size + + changesets.each do |changeset| + assert_select "entry > id", changeset_url(:id => changeset.id) + end + end + end end diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb index bcadc21b8..b52f55ea5 100644 --- a/test/integration/oauth_test.rb +++ b/test/integration/oauth_test.rb @@ -14,6 +14,7 @@ class OAuthTest < ActionDispatch::IntegrationTest oauth10_without_callback(client) oauth10_with_callback(client, "http://another.web.app.org/callback") + oauth10_refused(client) end def test_oauth10_desktop_app @@ -23,6 +24,7 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_response :success oauth10_without_callback(client) + oauth10_refused(client) end def test_oauth10a_web_app @@ -33,6 +35,7 @@ class OAuthTest < ActionDispatch::IntegrationTest oauth10a_without_callback(client) oauth10a_with_callback(client, "http://another.web.app.org/callback") + oauth10a_refused(client) end def test_oauth10a_desktop_app @@ -42,6 +45,7 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_response :success oauth10a_without_callback(client) + oauth10a_refused(client) end private @@ -57,7 +61,7 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_redirected_to "#{client.callback_url}?oauth_token=#{token.token}" else assert_response :success - assert_template "authorize_success" + assert_template :authorize_success end token.reload assert_not_nil token.created_at @@ -93,6 +97,26 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + def oauth10_refused(client) + token = get_request_token(client) + + post "/oauth/authorize", :oauth_token => token.token + assert_response :success + assert_template :authorize_failure + assert_select "p", "You have denied application #{client.name} access to your account." + token.reload + assert_nil token.authorized_at + assert_not_nil token.invalidated_at + + post "/oauth/authorize", :oauth_token => token.token + assert_response :success + assert_template :authorize_failure + assert_select "p", "The authorization token is not valid." + token.reload + assert_nil token.authorized_at + assert_not_nil token.invalidated_at + end + def oauth10_with_callback(client, callback_url) token = get_request_token(client) @@ -147,7 +171,7 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_redirected_to "http://some.web.app.org/callback?oauth_token=#{token.token}&oauth_verifier=#{verifier}" else assert_response :success - assert_template "authorize_success" + assert_template :authorize_success m = response.body.match("

The verification code is ([A-Za-z0-9]+).

") assert_not_nil m verifier = m[1] @@ -237,6 +261,26 @@ class OAuthTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + def oauth10a_refused(client) + token = get_request_token(client, :oauth_callback => "oob") + + post "/oauth/authorize", :oauth_token => token.token + assert_response :success + assert_template :authorize_failure + assert_select "p", "You have denied application #{client.name} access to your account." + token.reload + assert_nil token.authorized_at + assert_not_nil token.invalidated_at + + post "/oauth/authorize", :oauth_token => token.token + assert_response :success + assert_template :authorize_failure + assert_select "p", "The authorization token is not valid." + token.reload + assert_nil token.authorized_at + assert_not_nil token.invalidated_at + end + def get_request_token(client, options = {}) signed_get "/oauth/request_token", options.merge(:consumer => client) assert_response :success