From 83a8468ee968f6de4a86a8434fd90ca831b62785 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 23 Jun 2020 03:09:22 -0500 Subject: [PATCH] tests: remove unnecessary rescueing of Net::OpenTimeout errors. These exceptions are no longer thrown now that we've switched from HTTParty to http.rb. Swallowing unexpected exceptions during testing was a bad practice anyway. --- test/functional/artists_controller_test.rb | 9 +- test/test_helpers/download_test_helper.rb | 2 - test/unit/artist_test.rb | 18 +-- test/unit/sources/pixiv_test.rb | 3 - test/unit/upload_service_test.rb | 171 +++++++++------------ 5 files changed, 77 insertions(+), 126 deletions(-) diff --git a/test/functional/artists_controller_test.rb b/test/functional/artists_controller_test.rb index 2907aa2a9..2ae68d78e 100644 --- a/test/functional/artists_controller_test.rb +++ b/test/functional/artists_controller_test.rb @@ -4,11 +4,8 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest def assert_artist_found(expected_artist, source_url = nil) if source_url get_auth artists_path(format: "json", search: { url_matches: source_url }), @user - if response.body =~ /Net::OpenTimeout/ - skip "Remote connection to #{source_url} failed" - return - end end + assert_response :success json = JSON.parse(response.body) assert_equal(1, json.size, "Testing URL: #{source_url}") @@ -17,10 +14,6 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest def assert_artist_not_found(source_url) get_auth artists_path(format: "json", search: { url_matches: source_url }), @user - if response.body =~ /Net::OpenTimeout/ - skip "Remote connection to #{source_url} failed" - return - end assert_response :success json = JSON.parse(response.body) diff --git a/test/test_helpers/download_test_helper.rb b/test/test_helpers/download_test_helper.rb index c3f70c75d..68e14abc6 100644 --- a/test/test_helpers/download_test_helper.rb +++ b/test/test_helpers/download_test_helper.rb @@ -3,8 +3,6 @@ module DownloadTestHelper strategy = Sources::Strategies.find(source, referer) file = strategy.download_file! assert_equal(expected_filesize, file.size, "Tested source URL: #{source}") - rescue Net::OpenTimeout - skip "Remote connection to #{source} failed" end def assert_rewritten(expected_source, test_source, test_referer = nil) diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index b9dc3a90c..44beba428 100644 --- a/test/unit/artist_test.rb +++ b/test/unit/artist_test.rb @@ -6,15 +6,11 @@ class ArtistTest < ActiveSupport::TestCase assert_equal(1, artists.size) assert_equal(expected_name, artists.first.name, "Testing URL: #{source_url}") - rescue Net::OpenTimeout, PixivApiClient::Error - skip "Remote connection failed for #{source_url}" end def assert_artist_not_found(source_url) artists = ArtistFinder.find_artists(source_url).to_a assert_equal(0, artists.size, "Testing URL: #{source_url}") - rescue Net::OpenTimeout - skip "Remote connection failed for #{source_url}" end context "An artist" do @@ -172,15 +168,11 @@ class ArtistTest < ActiveSupport::TestCase a2 = FactoryBot.create(:artist, :name => "subway", :url_string => "http://subway.com/x/test.jpg") a3 = FactoryBot.create(:artist, :name => "minko", :url_string => "https://minko.com/x/test.jpg") - begin - assert_artist_found("rembrandt", "http://rembrandt.com/x/test.jpg") - assert_artist_found("rembrandt", "http://rembrandt.com/x/another.jpg") - assert_artist_not_found("http://nonexistent.com/test.jpg") - assert_artist_found("minko", "https://minko.com/x/test.jpg") - assert_artist_found("minko", "http://minko.com/x/test.jpg") - rescue Net::OpenTimeout - skip "network failure" - end + assert_artist_found("rembrandt", "http://rembrandt.com/x/test.jpg") + assert_artist_found("rembrandt", "http://rembrandt.com/x/another.jpg") + assert_artist_not_found("http://nonexistent.com/test.jpg") + assert_artist_found("minko", "https://minko.com/x/test.jpg") + assert_artist_found("minko", "http://minko.com/x/test.jpg") end should "be case-insensitive to domains when finding matches by url" do diff --git a/test/unit/sources/pixiv_test.rb b/test/unit/sources/pixiv_test.rb index 323e40a00..82f03f5ba 100644 --- a/test/unit/sources/pixiv_test.rb +++ b/test/unit/sources/pixiv_test.rb @@ -15,10 +15,7 @@ module Sources def get_source(source) @site = Sources::Strategies.find(source) - @site - rescue Net::OpenTimeout - skip "Remote connection to #{source} failed" end context "in all cases" do diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index b15a399a5..d44ecdf44 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -131,12 +131,9 @@ class UploadServiceTest < ActiveSupport::TestCase end should "download the file" do - begin - @service = UploadService::Preprocessor.new(source: @source, referer_url: @ref) - @upload = @service.start! - rescue Net::OpenTimeout - skip "network failure" - end + @service = UploadService::Preprocessor.new(source: @source, referer_url: @ref) + @upload = @service.start! + assert_equal("preprocessed", @upload.status) assert_equal(294591, @upload.file_size) assert_equal("jpg", @upload.file_ext) @@ -155,11 +152,8 @@ class UploadServiceTest < ActiveSupport::TestCase skip unless MediaFile::Ugoira.videos_enabled? @service = UploadService::Preprocessor.new(source: @source) - begin - @upload = @service.start! - rescue Net::OpenTimeout - skip "network problems" - end + @upload = @service.start! + assert_equal("preprocessed", @upload.status) assert_equal(2804, @upload.file_size) assert_equal("zip", @upload.file_ext) @@ -176,11 +170,8 @@ class UploadServiceTest < ActiveSupport::TestCase should "download the file" do @service = UploadService::Preprocessor.new(source: @source) - begin - @upload = @service.start! - rescue Net::OpenTimeout - skip "network problems" - end + @upload = @service.start! + assert_equal("preprocessed", @upload.status) assert_equal(181309, @upload.file_size) assert_equal("jpg", @upload.file_ext) @@ -512,36 +503,28 @@ class UploadServiceTest < ActiveSupport::TestCase context "a post with a pixiv html source" do should "replace with the full size image" do - begin - as(@user) do - @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") - end - - assert_equal(80, @post.image_width) - assert_equal(82, @post.image_height) - assert_equal(16275, @post.file_size) - assert_equal("png", @post.file_ext) - assert_equal("4ceadc314938bc27f3574053a3e1459a", @post.md5) - assert_equal("4ceadc314938bc27f3574053a3e1459a", Digest::MD5.file(@post.file).hexdigest) - assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.replacements.last.replacement_url) - assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.source) - rescue Net::OpenTimeout - skip "Remote connection to Pixiv failed" + as(@user) do + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") end + + assert_equal(80, @post.image_width) + assert_equal(82, @post.image_height) + assert_equal(16275, @post.file_size) + assert_equal("png", @post.file_ext) + assert_equal("4ceadc314938bc27f3574053a3e1459a", @post.md5) + assert_equal("4ceadc314938bc27f3574053a3e1459a", Digest::MD5.file(@post.file).hexdigest) + assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.replacements.last.replacement_url) + assert_equal("https://i.pximg.net/img-original/img/2017/04/04/08/54/15/62247350_p0.png", @post.source) end should "delete the old files after thirty days" do - begin - @post.unstub(:queue_delete_files) - FileUtils.expects(:rm_f).times(3) + @post.unstub(:queue_delete_files) + FileUtils.expects(:rm_f).times(3) - as(@user) { @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") } + as(@user) { @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") } - travel_to((PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now) do - perform_enqueued_jobs - end - rescue Net::OpenTimeout - skip "Remote connection to Pixiv failed" + travel_to((PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now) do + perform_enqueued_jobs end end end @@ -568,34 +551,30 @@ class UploadServiceTest < ActiveSupport::TestCase context "a post that is replaced to another file then replaced back to the original file" do should "not delete the original files" do - begin - skip unless MediaFile::Ugoira.videos_enabled? - @post.unstub(:queue_delete_files) + skip unless MediaFile::Ugoira.videos_enabled? + @post.unstub(:queue_delete_files) - # this is called thrice to delete the file for 62247364 - FileUtils.expects(:rm_f).times(3) + # this is called thrice to delete the file for 62247364 + FileUtils.expects(:rm_f).times(3) - as(@user) do - @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") - @post.reload - @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") - @post.reload - Upload.destroy_all - @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") - end - - assert_nothing_raised { @post.file(:original) } - assert_nothing_raised { @post.file(:preview) } - - assert_enqueued_jobs 3, only: DeletePostFilesJob - travel PostReplacement::DELETION_GRACE_PERIOD + 1.day - assert_raise(Post::DeletionError) { perform_enqueued_jobs } - - assert_nothing_raised { @post.file(:original) } - assert_nothing_raised { @post.file(:preview) } - rescue Net::OpenTimeout - skip "Remote connection to Pixiv failed" + as(@user) do + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + @post.reload + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + @post.reload + Upload.destroy_all + @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") end + + assert_nothing_raised { @post.file(:original) } + assert_nothing_raised { @post.file(:preview) } + + assert_enqueued_jobs 3, only: DeletePostFilesJob + travel PostReplacement::DELETION_GRACE_PERIOD + 1.day + assert_raise(Post::DeletionError) { perform_enqueued_jobs } + + assert_nothing_raised { @post.file(:original) } + assert_nothing_raised { @post.file(:preview) } end end @@ -609,39 +588,35 @@ class UploadServiceTest < ActiveSupport::TestCase should "not delete the still active files" do # swap the images between @post1 and @post2. - begin - as(@user) do - skip unless MediaFile::Ugoira.videos_enabled? + as(@user) do + skip unless MediaFile::Ugoira.videos_enabled? - @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") - @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") - assert_equal("4ceadc314938bc27f3574053a3e1459a", @post1.md5) - assert_equal("cad1da177ef309bf40a117c17b8eecf5", @post2.md5) + @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + assert_equal("4ceadc314938bc27f3574053a3e1459a", @post1.md5) + assert_equal("cad1da177ef309bf40a117c17b8eecf5", @post2.md5) - @post2.reload - @post2.replace!(replacement_url: "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg") - assert_equal("d34e4cf0a437a5d65f8e82b7bcd02606", @post2.md5) - Upload.destroy_all - @post1.reload - @post2.reload + @post2.reload + @post2.replace!(replacement_url: "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg") + assert_equal("d34e4cf0a437a5d65f8e82b7bcd02606", @post2.md5) + Upload.destroy_all + @post1.reload + @post2.reload - @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") - @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") - assert_equal("cad1da177ef309bf40a117c17b8eecf5", @post1.md5) - assert_equal("4ceadc314938bc27f3574053a3e1459a", @post2.md5) - end - - travel_to (PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now do - assert_raise(Post::DeletionError) do - perform_enqueued_jobs - end - end - - assert_nothing_raised { @post1.file(:original) } - assert_nothing_raised { @post2.file(:original) } - rescue Net::OpenTimeout - skip "Remote connection to Pixiv failed" + @post1.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + @post2.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350") + assert_equal("cad1da177ef309bf40a117c17b8eecf5", @post1.md5) + assert_equal("4ceadc314938bc27f3574053a3e1459a", @post2.md5) end + + travel_to (PostReplacement::DELETION_GRACE_PERIOD + 1).days.from_now do + assert_raise(Post::DeletionError) do + perform_enqueued_jobs + end + end + + assert_nothing_raised { @post1.file(:original) } + assert_nothing_raised { @post2.file(:original) } end end @@ -885,12 +860,8 @@ class UploadServiceTest < ActiveSupport::TestCase end should "record the canonical source" do - begin - post = subject.new({}).create_post_from_upload(@upload) - assert_equal(@source, post.source) - rescue Net::OpenTimeout - skip "network failure" - end + post = subject.new({}).create_post_from_upload(@upload) + assert_equal(@source, post.source) end end