From 3474911d254b8fe52839c95a85cbd6703bb08858 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Wed, 26 Jul 2023 17:33:37 +0300 Subject: [PATCH] Rubocop fixes in spec/services (#19844) --- spec/services/admin/charts_data_spec.rb | 7 +++-- ...rticle_with_video_creation_service_spec.rb | 2 +- .../welcome_notification/generator_spec.rb | 1 - spec/services/comment_creator_spec.rb | 6 ++-- spec/services/homepage/fetch_articles_spec.rb | 4 +-- spec/services/html/parser_spec.rb | 2 +- spec/services/images/optimizer_spec.rb | 28 ++++++++++++------- spec/services/languages/detection_spec.rb | 10 +++---- spec/services/mailchimp/bot_spec.rb | 2 +- spec/services/search/podcast_episode_spec.rb | 1 - spec/services/search/reading_list_spec.rb | 2 +- spec/services/search/username_spec.rb | 4 +-- .../create_from_controller_params_spec.rb | 3 +- .../users/suggest_for_sidebar_spec.rb | 2 +- .../comments/user_delete_a_comment_spec.rb | 2 +- .../unsubscribe.html.erb_spec.rb | 2 +- 16 files changed, 44 insertions(+), 34 deletions(-) diff --git a/spec/services/admin/charts_data_spec.rb b/spec/services/admin/charts_data_spec.rb index 58203ffb1..060d7685a 100644 --- a/spec/services/admin/charts_data_spec.rb +++ b/spec/services/admin/charts_data_spec.rb @@ -29,7 +29,8 @@ RSpec.describe Admin::ChartsData, type: :service do Timecop.return end - xit "returns proper number of items" do + it "returns proper number of items" do + skip "flaky spec" create(:article, published_at: Time.zone.today) create_list(:article, 3, published_at: 4.days.ago) create_list(:article, 2, published_at: 7.days.ago) @@ -43,7 +44,9 @@ RSpec.describe Admin::ChartsData, type: :service do expect(described_class.new.call.first.second).to eq(0) end - xit "goes back seven days by default" do + it "goes back seven days by default" do + skip "flaky spec" + create(:article, published_at: 7.days.ago) create(:article, published_at: 8.days.ago) diff --git a/spec/services/article_with_video_creation_service_spec.rb b/spec/services/article_with_video_creation_service_spec.rb index 37c6c512e..b612b2074 100644 --- a/spec/services/article_with_video_creation_service_spec.rb +++ b/spec/services/article_with_video_creation_service_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ArticleWithVideoCreationService, type: :service do end describe "#create!" do - it "works" do + it "creates a correct article" do Timecop.travel(3.weeks.ago) user = create(:user) user.setting.update(editor_version: "v1") diff --git a/spec/services/broadcasts/welcome_notification/generator_spec.rb b/spec/services/broadcasts/welcome_notification/generator_spec.rb index 400389c07..819100d49 100644 --- a/spec/services/broadcasts/welcome_notification/generator_spec.rb +++ b/spec/services/broadcasts/welcome_notification/generator_spec.rb @@ -48,7 +48,6 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do end.not_to change(user.notifications, :count) end - # rubocop:disable RSpec/ExampleLength # rubocop:disable RSpec/MultipleExpectations it "sends only 1 notification at a time, in the correct order" do user.update!(created_at: 1.day.ago) diff --git a/spec/services/comment_creator_spec.rb b/spec/services/comment_creator_spec.rb index a049984e4..43a3ca20f 100644 --- a/spec/services/comment_creator_spec.rb +++ b/spec/services/comment_creator_spec.rb @@ -37,13 +37,15 @@ RSpec.describe CommentCreator, type: :service do creator.save end - xit "notifies subscribers" do + it "notifies subscribers" do + skip "flaky spec" expect(NotificationSubscription).to have_received(:create) expect(Notification).to have_received(:send_new_comment_notifications_without_delay) expect(Mention).to have_received(:create_all) end - xit "creates a new reaction" do + it "creates a new reaction" do + skip "flaky spec" expect(Reaction).to have_received(:create) end end diff --git a/spec/services/homepage/fetch_articles_spec.rb b/spec/services/homepage/fetch_articles_spec.rb index 437d54b2a..b8da5625a 100644 --- a/spec/services/homepage/fetch_articles_spec.rb +++ b/spec/services/homepage/fetch_articles_spec.rb @@ -2,7 +2,6 @@ require "rails_helper" RSpec.describe Homepage::FetchArticles, type: :service do describe ".call" do - # rubocop:disable RSpec/ExampleLength it "returns results in the correct format", :aggregate_failures do tag = create(:tag, name: "ama", bg_color_hex: "#f3f3f3", text_color_hex: "#cccccc") article = create(:article, video_thumbnail_url: "https://example.com", tags: tag.name) @@ -17,7 +16,7 @@ RSpec.describe Homepage::FetchArticles, type: :service do published_at_int readable_publish_date reading_time tag_list title user user_id video_duration_string ] - expect(result.keys.sort).to contain_exactly(*keys) + expect(result.keys.sort).to match_array(keys) expect(result[:class_name]).to eq("Article") expect(result[:cloudinary_video_url]).to eq(article.cloudinary_video_url) @@ -36,7 +35,6 @@ RSpec.describe Homepage::FetchArticles, type: :service do expect(result[:user_id]).to eq(article.user_id) expect(result[:video_duration_string]).to eq(article.video_duration_in_minutes) end - # rubocop:enable RSpec/ExampleLength it "returns the user object in the correct format", :aggregate_failures do article = create(:article) diff --git a/spec/services/html/parser_spec.rb b/spec/services/html/parser_spec.rb index e801ef520..67decda7f 100644 --- a/spec/services/html/parser_spec.rb +++ b/spec/services/html/parser_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe Html::Parser, type: :service do it "has the correct raw tag delimiters" do - expect(described_class::RAW_TAG_DELIMITERS).to match_array(["{", "}", "raw", "endraw", "----"]) + expect(described_class::RAW_TAG_DELIMITERS).to contain_exactly("{", "}", "raw", "endraw", "----") end describe "#remove_nested_linebreak_in_list" do diff --git a/spec/services/images/optimizer_spec.rb b/spec/services/images/optimizer_spec.rb index 721a89fca..fb4560a7e 100644 --- a/spec/services/images/optimizer_spec.rb +++ b/spec/services/images/optimizer_spec.rb @@ -77,7 +77,7 @@ RSpec.describe Images::Optimizer, type: :service do end describe "#imgproxy" do - it "works" do + it "generates correct url" do allow(described_class).to receive(:imgproxy_enabled?).and_return(true) imgproxy_url = described_class.imgproxy(image_url, width: 500, height: 500) # mb = maximum bytes, defaults to 500_000 bytes @@ -87,13 +87,17 @@ RSpec.describe Images::Optimizer, type: :service do end describe "#cloudflare" do + let(:cloudfare_domain) { ApplicationConfig["CLOUDFLARE_IMAGES_DOMAIN"] } + let(:cloudfare_basic_url) { "https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=900,fit=cover,gravity=auto,format=auto/" } + before do allow(ApplicationConfig).to receive(:[]).with("CLOUDFLARE_IMAGES_DOMAIN").and_return("images.example.com") end it "generates correct url based on h/w input" do cloudflare_url = described_class.cloudflare(image_url, width: 821, height: 420) - expect(cloudflare_url).to match(%r{/width=821,height=420,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}}) + url_regexp = %r{/width=821,height=420,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}} + expect(cloudflare_url).to match(url_regexp) end it "does not error if nil" do @@ -103,32 +107,36 @@ RSpec.describe Images::Optimizer, type: :service do it "pulls suffix if nested cloudflare url is provided" do cloudflare_url = described_class.cloudflare( - "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=900,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}", width: 821, height: 420 + [cloudfare_basic_url, CGI.escape(image_url)].join, + width: 821, height: 420, ) - expect(cloudflare_url).to eq("https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}") + expect(cloudflare_url).to eq("https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}") end it "does not error out if image is empty" do cloudflare_url = described_class.cloudflare( - "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=900,fit=cover,gravity=auto,format=auto/", width: 821, height: 420 + cloudfare_basic_url, + width: 821, height: 420, ) - expect(cloudflare_url).to eq("https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/") + expect(cloudflare_url).to eq("https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/") end it "does not error out if image is not proper url and has https" do image_url = "https:hello" cloudflare_url = described_class.cloudflare( - "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=900,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}", width: 821, height: 420 + [cloudfare_basic_url, CGI.escape(image_url)].join, + width: 821, height: 420, ) - expect(cloudflare_url).to eq("https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/https%3Ahello") + expect(cloudflare_url).to eq("https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/https%3Ahello") end it "does not error out if image is not proper url and does not have https" do image_url = "hello" cloudflare_url = described_class.cloudflare( - "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=900,fit=cover,gravity=auto,format=auto/#{CGI.escape(image_url)}", width: 821, height: 420 + [cloudfare_basic_url, CGI.escape(image_url)].join, + width: 821, height: 420, ) - expect(cloudflare_url).to eq("https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/") + expect(cloudflare_url).to eq("https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=420,fit=cover,gravity=auto,format=auto/") end end diff --git a/spec/services/languages/detection_spec.rb b/spec/services/languages/detection_spec.rb index 7fd98f7b0..e29519e93 100644 --- a/spec/services/languages/detection_spec.rb +++ b/spec/services/languages/detection_spec.rb @@ -34,27 +34,27 @@ RSpec.describe Languages::Detection, type: :service do CLD3::NNetLanguageIdentifier::Result, language: :es, probability: 0.4, - reliable?: true + reliable?: true, ) end it "returns nil" do - expect(described_class.call(text)).to eq(nil) + expect(described_class.call(text)).to be_nil end end context "when reliability is low" do let(:language_outcome) do instance_double( - 'CLD3::NNetLanguageIdentifier::Result', + CLD3::NNetLanguageIdentifier::Result, language: :es, probability: 0.9, - reliable?: false + reliable?: false, ) end it "returns nil" do - expect(described_class.call(text)).to be(nil) + expect(described_class.call(text)).to be_nil end end diff --git a/spec/services/mailchimp/bot_spec.rb b/spec/services/mailchimp/bot_spec.rb index e4445080c..a53d303b2 100644 --- a/spec/services/mailchimp/bot_spec.rb +++ b/spec/services/mailchimp/bot_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Mailchimp::Bot, type: :service do end describe "#upsert" do - it "works" do + it "calls upsert" do described_class.new(user).upsert expect(my_gibbon_client).to have_received(:upsert) end diff --git a/spec/services/search/podcast_episode_spec.rb b/spec/services/search/podcast_episode_spec.rb index 75141d915..94f9213cf 100644 --- a/spec/services/search/podcast_episode_spec.rb +++ b/spec/services/search/podcast_episode_spec.rb @@ -1,6 +1,5 @@ require "rails_helper" -# rubocop:disable RSpec/ExampleLength RSpec.describe Search::PodcastEpisode, type: :service do let(:podcast_episode) { create(:podcast_episode) } diff --git a/spec/services/search/reading_list_spec.rb b/spec/services/search/reading_list_spec.rb index a00a815d6..50007a4a3 100644 --- a/spec/services/search/reading_list_spec.rb +++ b/spec/services/search/reading_list_spec.rb @@ -173,7 +173,7 @@ RSpec.describe Search::ReadingList, type: :service do create(:reaction, reactable: article_1, user: user, category: :readinglist) result = described_class.search_documents(user, tags: [:beginners]) - expect(extract_from_results(result, :path)).to match_array([article_1.path]) + expect(extract_from_results(result, :path)).to contain_exactly(article_1.path) end it "selects items belonging to an article with all the requested tags", :aggregate_failures do diff --git a/spec/services/search/username_spec.rb b/spec/services/search/username_spec.rb index c4e563d28..6e8586d62 100644 --- a/spec/services/search/username_spec.rb +++ b/spec/services/search/username_spec.rb @@ -153,7 +153,7 @@ RSpec.describe Search::Username, type: :service do it "ranks author higher than commenter" do results = search("Per", context: article) - expect(results.pluck(:username)).to contain_exactly(*%w[author commentator]) + expect(results.pluck(:username)).to match_array(%w[author commentator]) end it "ranks unrelated user lower" do @@ -161,7 +161,7 @@ RSpec.describe Search::Username, type: :service do results = search("Per", context: article) - expect(results.pluck(:username)).to contain_exactly(*%w[author commentator unrelated]) + expect(results.pluck(:username)).to match_array(%w[author commentator unrelated]) end it "does not have authorship ranking for PodcastEpisode (yet)" do diff --git a/spec/services/user_subscriptions/create_from_controller_params_spec.rb b/spec/services/user_subscriptions/create_from_controller_params_spec.rb index 6086fb209..285c88f97 100644 --- a/spec/services/user_subscriptions/create_from_controller_params_spec.rb +++ b/spec/services/user_subscriptions/create_from_controller_params_spec.rb @@ -26,7 +26,8 @@ RSpec.describe UserSubscriptions::CreateFromControllerParams, type: :service do end # TODO: [@forem/delightful]: re-enable this once email confirmation is re-enabled - xit "returns an error for an email mismatch" do + it "returns an error for an email mismatch" do + skip "email confirmation disabled" source = create(:article, :with_user_subscription_tag_role_user, with_user_subscription_tag: true) user_subscription_params = { source_type: source.class.name, source_id: source.id, subscriber_email: "old@email.com" } diff --git a/spec/services/users/suggest_for_sidebar_spec.rb b/spec/services/users/suggest_for_sidebar_spec.rb index 5199baa1c..261f7ac78 100644 --- a/spec/services/users/suggest_for_sidebar_spec.rb +++ b/spec/services/users/suggest_for_sidebar_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Users::SuggestForSidebar, type: :service do tags = "html" article1 = create(:article, tags: tags) article2 = create(:article, tags: tags) - expect(described_class.new(user, tags).suggest.to_a).to match_array([article1.user, article2.user]) + expect(described_class.new(user, tags).suggest.to_a).to contain_exactly(article1.user, article2.user) end it "returns no user if there's not enough sample" do diff --git a/spec/system/comments/user_delete_a_comment_spec.rb b/spec/system/comments/user_delete_a_comment_spec.rb index 2fbe7d3c4..7e8d778a1 100644 --- a/spec/system/comments/user_delete_a_comment_spec.rb +++ b/spec/system/comments/user_delete_a_comment_spec.rb @@ -12,7 +12,7 @@ RSpec.describe "Deleting Comment", js: true do sign_in user end - it "works" do + it "returns to article page" do visit "/" visit "#{comment.path}/delete_confirm" diff --git a/spec/views/email_subscriptions/unsubscribe.html.erb_spec.rb b/spec/views/email_subscriptions/unsubscribe.html.erb_spec.rb index 463cd19aa..f5255f8f0 100644 --- a/spec/views/email_subscriptions/unsubscribe.html.erb_spec.rb +++ b/spec/views/email_subscriptions/unsubscribe.html.erb_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe "email_subscriptions/unsubscribe" do - it "works" do + it "has unsubscribed info" do assign(:email_type, "#{Settings::Community.community_name} digest emails") render expect(rendered)