From 21d60e359a6e972731e0385c51dd86536b7ea777 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 4 Mar 2015 21:49:43 +0000 Subject: [PATCH] Tests! --- .rubocop_todo.yml | 4 +- app/controllers/application_controller.rb | 6 -- test/controllers/changeset_controller_test.rb | 16 ++++- .../diary_entry_controller_test.rb | 68 ++++++++++++++++++- test/controllers/notes_controller_test.rb | 42 ++++++++++++ test/controllers/site_controller_test.rb | 22 ++++++ test/fixtures/changesets_subscribers.yml | 4 ++ test/fixtures/users.yml | 1 - test/integration/locale_test.rb | 37 ++++++++++ test/integration/oauth_test.rb | 18 +++++ test/integration/user_terms_seen_test.rb | 2 + test/models/trace_test.rb | 28 ++++++++ 12 files changed, 235 insertions(+), 13 deletions(-) create mode 100644 test/integration/locale_test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 025169122..a993d5fff 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -28,7 +28,7 @@ Lint/ParenthesesAsGroupedExpression: # Offense count: 542 Metrics/AbcSize: - Max: 194 + Max: 270 # Offense count: 12 Metrics/BlockNesting: @@ -37,7 +37,7 @@ Metrics/BlockNesting: # Offense count: 60 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 1543 + Max: 1552 # Offense count: 68 Metrics/CyclomaticComplexity: diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8322201ff..618f08a1f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -389,12 +389,6 @@ class ApplicationController < ActionController::Base render :action => "timeout" end - ## - # is the requestor logged in? - def logged_in? - !@user.nil? - end - ## # ensure that there is a "this_user" instance variable def lookup_this_user diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 6a443850f..148e54767 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -1892,16 +1892,23 @@ EOF assert_response :success assert_difference "ChangesetComment.count", 1 do - assert_no_difference "ActionMailer::Base.deliveries.size" do + assert_difference "ActionMailer::Base.deliveries.size", 1 do post :comment, :id => changesets(:normal_user_subscribed_change).id, :text => "This is a comment" end end assert_response :success + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.length + assert_equal "[OpenStreetMap] test2 has commented on one of your changesets", email.subject + assert_equal "test@openstreetmap.org", email.to.first + + ActionMailer::Base.deliveries.clear + basic_authorization(users(:second_public_user).email, "test") assert_difference "ChangesetComment.count", 1 do - assert_difference "ActionMailer::Base.deliveries.size", 1 do + assert_difference "ActionMailer::Base.deliveries.size", 2 do post :comment, :id => changesets(:normal_user_subscribed_change).id, :text => "This is a comment" end end @@ -1909,6 +1916,11 @@ EOF email = ActionMailer::Base.deliveries.first assert_equal 1, email.to.length + assert_equal "[OpenStreetMap] pulibc_test2 has commented on one of your changesets", email.subject + assert_equal "test@openstreetmap.org", email.to.first + + email = ActionMailer::Base.deliveries.second + assert_equal 1, email.to.length assert_equal "[OpenStreetMap] pulibc_test2 has commented on a changeset you are interested in", email.subject assert_equal "test@example.com", email.to.first diff --git a/test/controllers/diary_entry_controller_test.rb b/test/controllers/diary_entry_controller_test.rb index aa90ecd72..1e700da87 100644 --- a/test/controllers/diary_entry_controller_test.rb +++ b/test/controllers/diary_entry_controller_test.rb @@ -147,6 +147,32 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_equal new_language_code, entry.language_code end + def test_new_spammy + # Generate some spammy content + spammy_title = "Spam Spam Spam Spam Spam" + spammy_body = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ") + + # Try creating a spammy diary entry + assert_difference "DiaryEntry.count", 1 do + post :new, { :commit => "save", + :diary_entry => { :title => spammy_title, :body => spammy_body, :language_code => "en" } }, + { :user => users(:normal_user).id } + end + assert_response :redirect + assert_redirected_to :action => :list, :display_name => users(:normal_user).display_name + entry = DiaryEntry.order(:id).last + assert_equal users(:normal_user).id, entry.user_id + assert_equal spammy_title, entry.title + assert_equal spammy_body, entry.body + assert_equal "en", entry.language_code + assert_equal "suspended", User.find(users(:normal_user).id).status + + # Follow the redirect + get :list, { :display_name => users(:normal_user).display_name }, { :user => users(:normal_user).id } + assert_response :redirect + assert_redirected_to :controller => :user, :action => :suspended + end + def test_edit entry = diary_entries(:normal_user_entry_1) @@ -287,7 +313,7 @@ class DiaryEntryControllerTest < ActionController::TestCase assert_match /New comment/, email.text_part.decoded assert_match /New comment/, email.html_part.decoded ActionMailer::Base.deliveries.clear - comment = DiaryComment.find(5) + comment = DiaryComment.order(:id).last assert_equal entry.id, comment.diary_entry_id assert_equal users(:public_user).id, comment.user_id assert_equal "New comment", comment.body @@ -296,13 +322,51 @@ class DiaryEntryControllerTest < ActionController::TestCase get :view, :display_name => entry.user.display_name, :id => entry.id assert_response :success assert_select ".diary-comment", :count => 1 do - assert_select "#comment5", :count => 1 do + assert_select "#comment#{comment.id}", :count => 1 do assert_select "a[href='/user/#{users(:public_user).display_name}']", :text => users(:public_user).display_name, :count => 1 end assert_select ".richtext", :text => /New comment/, :count => 1 end end + def test_comment_spammy + # Find the entry to comment on + entry = diary_entries(:normal_user_entry_1) + + # Generate some spammy content + spammy_text = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ") + + # Try creating a spammy comment + assert_difference "ActionMailer::Base.deliveries.size", 1 do + assert_difference "DiaryComment.count", 1 do + post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => spammy_text } }, { :user => users(:public_user).id } + end + end + assert_response :redirect + assert_redirected_to :action => :view, :display_name => entry.user.display_name, :id => entry.id + email = ActionMailer::Base.deliveries.first + assert_equal [users(:normal_user).email], email.to + assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on your diary entry", email.subject + assert_match %r{http://example.com/spam}, email.text_part.decoded + assert_match %r{http://example.com/spam}, email.html_part.decoded + ActionMailer::Base.deliveries.clear + comment = DiaryComment.order(:id).last + assert_equal entry.id, comment.diary_entry_id + assert_equal users(:public_user).id, comment.user_id + assert_equal spammy_text, comment.body + assert_equal "suspended", User.find(users(:public_user).id).status + + # Follow the redirect + get :list, { :display_name => users(:normal_user).display_name }, { :user => users(:public_user).id } + assert_response :redirect + assert_redirected_to :controller => :user, :action => :suspended + + # Now view the diary entry, and check the new comment is not present + get :view, :display_name => entry.user.display_name, :id => entry.id + assert_response :success + assert_select ".diary-comment", :count => 0 + end + def test_list_all # Try a list of all diary entries get :list diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb index 96f535e42..16ad623e4 100644 --- a/test/controllers/notes_controller_test.rb +++ b/test/controllers/notes_controller_test.rb @@ -281,6 +281,48 @@ class NotesControllerTest < ActionController::TestCase assert_nil js["properties"]["comments"].last["user"] ActionMailer::Base.deliveries.clear + + basic_authorization(users(:public_user).email, "test") + + assert_difference "NoteComment.count", 1 do + assert_difference "ActionMailer::Base.deliveries.size", 2 do + post :comment, :id => notes(:note_with_comments_by_users).id, :text => "This is an additional comment", :format => "json" + end + end + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal "Feature", js["type"] + assert_equal notes(:note_with_comments_by_users).id, js["properties"]["id"] + assert_equal "open", js["properties"]["status"] + assert_equal 4, js["properties"]["comments"].count + assert_equal "commented", js["properties"]["comments"].last["action"] + assert_equal "This is an additional comment", js["properties"]["comments"].last["text"] + assert_equal "test2", js["properties"]["comments"].last["user"] + + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.length + assert_equal "[OpenStreetMap] test2 has commented on one of your notes", email.subject + assert_equal "test@openstreetmap.org", email.to.first + + email = ActionMailer::Base.deliveries.second + assert_equal 1, email.to.length + assert_equal "[OpenStreetMap] test2 has commented on a note you are interested in", email.subject + assert_equal "public@OpenStreetMap.org", email.to.first + + get :show, :id => notes(:note_with_comments_by_users).id, :format => "json" + assert_response :success + js = ActiveSupport::JSON.decode(@response.body) + assert_not_nil js + assert_equal "Feature", js["type"] + assert_equal notes(:note_with_comments_by_users).id, js["properties"]["id"] + assert_equal "open", js["properties"]["status"] + assert_equal 4, js["properties"]["comments"].count + assert_equal "commented", js["properties"]["comments"].last["action"] + assert_equal "This is an additional comment", js["properties"]["comments"].last["text"] + assert_equal "test2", js["properties"]["comments"].last["user"] + + ActionMailer::Base.deliveries.clear end def test_comment_fail diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb index 01c1f3a22..7fa558e78 100644 --- a/test/controllers/site_controller_test.rb +++ b/test/controllers/site_controller_test.rb @@ -199,6 +199,28 @@ class SiteControllerTest < ActionController::TestCase assert_template "index" end + # Test the right editor gets used when the URL has an override + def test_edit_with_override + get :edit, { :editor => "id" }, { :user => users(:public_user).id } + assert_response :success + assert_template "edit" + assert_template :partial => "_id", :count => 1 + + get :edit, { :editor => "potlatch2" }, { :user => users(:public_user).id } + assert_response :success + assert_template "edit" + assert_template :partial => "_potlatch2", :count => 1 + + get :edit, { :editor => "potlatch" }, { :user => users(:public_user).id } + assert_response :success + assert_template "edit" + assert_template :partial => "_potlatch", :count => 1 + + get :edit, { :editor => "remote" }, { :user => users(:public_user).id } + assert_response :success + assert_template "index" + end + # Test editing a specific node def test_edit_with_node user = users(:public_user) diff --git a/test/fixtures/changesets_subscribers.yml b/test/fixtures/changesets_subscribers.yml index f46c688a2..d8bd5be96 100644 --- a/test/fixtures/changesets_subscribers.yml +++ b/test/fixtures/changesets_subscribers.yml @@ -1,3 +1,7 @@ t1: changeset_id: 8 subscriber_id: 2 + +t2: + changeset_id: 8 + subscriber_id: 1 diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 1b958028f..50e24d904 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -63,7 +63,6 @@ second_public_user: home_zoom: 12 terms_agreed: "2010-01-01 11:22:33" terms_seen: true - languages: en moderator_user: id: 5 diff --git a/test/integration/locale_test.rb b/test/integration/locale_test.rb new file mode 100644 index 000000000..fbc2aff0e --- /dev/null +++ b/test/integration/locale_test.rb @@ -0,0 +1,37 @@ +require "test_helper" + +class LocaleTest < ActionDispatch::IntegrationTest + fixtures :users + + def test_defaulting + user = users(:second_public_user) + + post_via_redirect "/login", :username => user.email, :password => "test" + + get "/diary/new", {} + assert_equal [], User.find(user.id).languages + assert_select "html[lang=?]", "en" + + get "/diary/new", {}, { "HTTP_ACCEPT_LANGUAGE" => "fr, en" } + assert_equal %w(fr en), User.find(user.id).languages + assert_select "html[lang=?]", "fr" + end + + def test_override + user = users(:german_user) + + get "/diary" + assert_select "html[lang=?]", "en" + + get "/diary", :locale => "es" + assert_select "html[lang=?]", "es" + + post_via_redirect "/login", :username => user.email, :password => "test" + + get "/diary" + assert_select "html[lang=?]", "de" + + get "/diary", :locale => "fr" + assert_select "html[lang=?]", "fr" + end +end diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb index 4f0d0a809..0d6d30016 100644 --- a/test/integration/oauth_test.rb +++ b/test/integration/oauth_test.rb @@ -49,6 +49,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/user/preferences", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/gpx/2", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) @@ -94,6 +97,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/gpx/2", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/user/details", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) @@ -146,6 +152,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/user/preferences", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/gpx/2", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) @@ -204,6 +213,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/user/preferences", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/gpx/2", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) @@ -255,6 +267,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/gpx/2", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/user/details", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) @@ -315,6 +330,9 @@ class OAuthTest < ActionDispatch::IntegrationTest signed_get "/api/0.6/user/preferences", :consumer => client, :token => token assert_response :success + signed_get "/api/0.6/gpx/2", :consumer => client, :token => token + assert_response :forbidden + post "/oauth/revoke", :token => token.token assert_redirected_to oauth_clients_url(token.user.display_name) token = OauthToken.find_by_token(token.token) diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb index 3a42aa6bd..678492f3c 100644 --- a/test/integration/user_terms_seen_test.rb +++ b/test/integration/user_terms_seen_test.rb @@ -64,6 +64,8 @@ class UserTermsSeenTest < ActionDispatch::IntegrationTest # back to the terms page. get "/traces/mine" assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/traces/mine" + get "/traces/mine", :referer => "/test" + assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/test" end end diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb index 85b258758..524ec637a 100644 --- a/test/models/trace_test.rb +++ b/test/models/trace_test.rb @@ -7,11 +7,17 @@ class TraceTest < ActiveSupport::TestCase def setup @gpx_trace_dir = Object.send("remove_const", "GPX_TRACE_DIR") Object.const_set("GPX_TRACE_DIR", File.dirname(__FILE__) + "/../traces") + + @gpx_image_dir = Object.send("remove_const", "GPX_IMAGE_DIR") + Object.const_set("GPX_IMAGE_DIR", File.dirname(__FILE__) + "/../traces") end def teardown Object.send("remove_const", "GPX_TRACE_DIR") Object.const_set("GPX_TRACE_DIR", @gpx_trace_dir) + + Object.send("remove_const", "GPX_IMAGE_DIR") + Object.const_set("GPX_IMAGE_DIR", @gpx_image_dir) end def test_trace_count @@ -142,6 +148,28 @@ class TraceTest < ActiveSupport::TestCase assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:tar_bzip_trace_file).xml_file) end + def test_large_picture + picture = gpx_files(:public_trace_file).large_picture + trace = Trace.create + + trace.large_picture = picture + assert_equal "7c841749e084ee4a5d13f12cd3bef456", md5sum(File.new(trace.large_picture_name)) + assert_equal picture, trace.large_picture + + trace.destroy + end + + def test_icon_picture + picture = gpx_files(:public_trace_file).icon_picture + trace = Trace.create + + trace.icon_picture = picture + assert_equal "b47baf22ed0e85d77e808694fad0ee27", md5sum(File.new(trace.icon_picture_name)) + assert_equal picture, trace.icon_picture + + trace.destroy + end + private def check_query(query, traces) -- 2.39.5