DEV: Fix various spec linting issues (#24672)

Duplicated specs, incorrect descriptions, incorrect assertions, incorrect filenames, old todo

lib/user_name_suggester.rb

@@ -114,8 +114,6 @@ module UserNameSuggester
     if SiteSetting.unicode_usernames
       name.unicode_normalize!
 
-      # TODO: Jan 2022, review if still needed
-      # see: https://meta.discourse.org/t/unicode-username-with-as-the-final-char-leads-to-an-error-loading-profile-page/173182
       if name.include?("Σ")
         ctx = MiniRacer::Context.new
         name = ctx.eval("#{name.to_s.to_json}.toLowerCase()")

spec/fixtures/onebox/githubpullrequest.response

@@ -211,6 +211,6 @@
   "review_comments": 0,
   "commits": 1,
   "additions": 19,
-  "deletions": 1,
+  "deletions": 5,
   "changed_files": 4
 }

spec/jobs/notify_mailing_list_subscribers_spec.rb

@@ -57,12 +57,12 @@ RSpec.describe Jobs::NotifyMailingListSubscribers do
     end
 
     context "with an invalid post_id" do
-      before { post.update(deleted_at: Time.now) }
+      before { post.destroy! }
       include_examples "no emails"
     end
 
     context "with a deleted post" do
-      before { post.update(deleted_at: Time.now) }
+      before { post.trash! }
       include_examples "no emails"
     end
 

spec/jobs/regular/group_smtp_email_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
   end
   let(:staged1) { Fabricate(:staged, email: "otherguy@test.com") }
   let(:staged2) { Fabricate(:staged, email: "cormac@lit.com") }
-  let(:normaluser) { Fabricate(:user, email: "justanormalguy@test.com", username: "normaluser") }
+  let(:normal_user) { Fabricate(:user, email: "justanormalguy@test.com", username: "normal_user") }
 
   before do
     SiteSetting.enable_smtp = true
@@ -33,7 +33,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
     TopicAllowedUser.create(user: recipient_user, topic: topic)
     TopicAllowedUser.create(user: staged1, topic: topic)
     TopicAllowedUser.create(user: staged2, topic: topic)
-    TopicAllowedUser.create(user: normaluser, topic: topic)
+    TopicAllowedUser.create(user: normal_user, topic: topic)
   end
 
   it "sends an email using the GroupSmtpMailer and Email::Sender" do
@@ -95,7 +95,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
     expect(email_text).not_to include("[otherguy@test.com]")
     expect(email_text).to include("cormac@lit.com")
     expect(email_text).not_to include("[cormac@lit.com]")
-    expect(email_text).to include("normaluser")
+    expect(email_text).to include("normal_user")
     expect(email_text).not_to include(recipient_user.username)
   end
 
@@ -143,20 +143,6 @@ RSpec.describe Jobs::GroupSmtpEmail do
     expect(email_log.message_id).to eq("discourse/post/#{post.id}@test.localhost")
   end
 
-  it "creates an IncomingEmail record with the correct details to avoid double processing IMAP" do
-    job.execute(args)
-    expect(ActionMailer::Base.deliveries.count).to eq(1)
-    expect(ActionMailer::Base.deliveries.last.subject).to eq("Re: Help I need support")
-    incoming_email =
-      IncomingEmail.find_by(post_id: post.id, topic_id: post.topic_id, user_id: post.user.id)
-    expect(incoming_email).not_to eq(nil)
-    expect(incoming_email.message_id).to eq("discourse/post/#{post.id}@test.localhost")
-    expect(incoming_email.created_via).to eq(IncomingEmail.created_via_types[:group_smtp])
-    expect(incoming_email.to_addresses).to eq("test@test.com")
-    expect(incoming_email.cc_addresses).to eq("otherguy@test.com;cormac@lit.com")
-    expect(incoming_email.subject).to eq("Re: Help I need support")
-  end
-
   it "does not create a post reply key, it always replies to the group email_username" do
     job.execute(args)
     expect(ActionMailer::Base.deliveries.count).to eq(1)

spec/lib/common_passwords/common_passwords_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe CommonPasswords do
 
     it "loads the passwords file if redis has an empty list" do
       Discourse.redis.without_namespace.stubs(:scard).returns(0)
-      described_class.expects(:load_passwords).returns(["password"])
+      described_class.expects(:load_passwords).returns([])
       list = described_class.password_list
       expect(list).to respond_to(:include?)
     end

spec/lib/discourse_redis_spec.rb

@@ -208,13 +208,6 @@ RSpec.describe DiscourseRedis do
       expect(helper.eval(Discourse.redis, %w[key1 key2], %w[arg1 arg2])).to eq("arg1arg2key1key2")
     end
 
-    it "works with arguments" do
-      helper = DiscourseRedis::EvalHelper.new <<~LUA
-        return ARGV[1]..ARGV[2]..KEYS[1]..KEYS[2]
-      LUA
-      expect(helper.eval(Discourse.redis, %w[key1 key2], %w[arg1 arg2])).to eq("arg1arg2key1key2")
-    end
-
     it "uses evalsha correctly" do
       redis_proxy =
         Class

spec/lib/email/styles_spec.rb

@@ -64,11 +64,6 @@ RSpec.describe Email::Styles do
       expect(frag.at("a")["style"]).to be_present
     end
 
-    it "attaches a style to a tags" do
-      frag = html_fragment("<a href>wat</a>")
-      expect(frag.at("a")["style"]).to be_present
-    end
-
     it "attaches a style to ul and li tags" do
       frag = html_fragment("<ul><li>hello</li></ul>")
       expect(frag.at("ul")["style"]).to be_present

spec/lib/onebox/engine/github_commit_onebox_spec.rb

@@ -15,12 +15,8 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do
     it_behaves_like "an engine"
 
     describe "#to_html" do
-      it "includes owner" do
-        expect(html).to include("discourse")
-      end
-
       it "includes repository name" do
-        expect(html).to include("discourse")
+        expect(html).to include("discourse/discourse")
       end
 
       it "includes commit sha" do
@@ -75,12 +71,8 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do
     # it_behaves_like "an engine"
 
     describe "#to_html" do
-      it "includes owner" do
-        expect(html).to include("discourse")
-      end
-
       it "includes repository name" do
-        expect(html).to include("discourse")
+        expect(html).to include("discourse/discourse")
       end
 
       it "includes commit sha" do

spec/lib/onebox/engine/github_pull_request_onebox_spec.rb

@@ -44,7 +44,7 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do
     end
 
     it "includes number of deletions" do
-      expect(html).to include("1")
+      expect(html).to include("5")
     end
 
     it "includes the body without comments" do

spec/lib/onebox/engine/reddit_media_onebox_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe Onebox::Engine::RedditMediaOnebox do
   let(:html) { described_class.new(link).to_html }
 
   before do
+    stub_request(:head, "#{link}/").to_return(status: 200)
     stub_request(:get, link).to_return(status: 200, body: onebox_response("reddit_image"))
     stub_request(:get, "#{link}/").to_return(status: 200, body: onebox_response("reddit_image"))
   end

spec/lib/pinned_check_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe PinnedCheck do
     end
 
     it "returns true if the topic is pinned" do
-      expect(PinnedCheck.pinned?(unpinned_topic)).to eq(false)
+      expect(PinnedCheck.pinned?(pinned_topic)).to eq(true)
     end
   end
 

spec/lib/upload_security_spec.rb

@@ -72,13 +72,6 @@ RSpec.describe UploadSecurity do
             end
           end
 
-          describe "for a public type avatar" do
-            let(:type) { "avatar" }
-            it "returns false" do
-              expect(security.should_be_secure?).to eq(false)
-            end
-          end
-
           describe "for a public type category_logo" do
             let(:type) { "category_logo" }
             it "returns false" do

spec/lib/user_name_suggester_spec.rb

@@ -72,8 +72,8 @@ RSpec.describe UserNameSuggester do
       end
     end
 
-    it "removes leading character if it is not alphanumeric" do
-      expect(UserNameSuggester.suggest(".myname")).to eq("myname")
+    it "replaces the leading character with _ if it is not alphanumeric" do
+      expect(UserNameSuggester.suggest("=myname")).to eq("_myname")
     end
 
     it "allows leading _" do
@@ -93,12 +93,16 @@ RSpec.describe UserNameSuggester do
       expect(UserNameSuggester.suggest("my.name")).to eq("my.name")
     end
 
-    it "remove leading dots" do
-      expect(UserNameSuggester.suggest(".myname")).to eq("myname")
+    it "replaces multiple dots in the middle with _" do
+      expect(UserNameSuggester.suggest("my..name")).to eq("my_name")
     end
 
-    it "remove trailing dots" do
-      expect(UserNameSuggester.suggest("myname.")).to eq("myname")
+    it "removes leading dots" do
+      expect(UserNameSuggester.suggest("..myname")).to eq("myname")
+    end
+
+    it "removes trailing dots" do
+      expect(UserNameSuggester.suggest("myname..")).to eq("myname")
     end
 
     it "handles usernames with a sequence of 2 or more special chars" do

spec/models/emoji_spec.rb

@@ -143,11 +143,6 @@ RSpec.describe Emoji do
       expect(emoji.group).to eq("flags")
     end
 
-    it "sets the group of the emoji" do
-      emoji = Emoji.create_from_db_item("name" => "scotland")
-      expect(emoji.group).to eq("flags")
-    end
-
     it "doesn’t create emoji when group is unknown" do
       emoji = Emoji.create_from_db_item("name" => "white_hair")
       expect(emoji).to be_nil

spec/models/reviewable_user_spec.rb

@@ -242,16 +242,5 @@ RSpec.describe ReviewableUser, type: :model do
       expect(event[:event_name]).to eq(:user_approved)
       expect(event[:params].first).to eq(user)
     end
-
-    it "triggers a extensibility event" do
-      user && admin # bypass the user_created event
-      event =
-        DiscourseEvent
-          .track_events { ReviewableUser.find_by(target: user).perform(admin, :approve_user) }
-          .first
-
-      expect(event[:event_name]).to eq(:user_approved)
-      expect(event[:params].first).to eq(user)
-    end
   end
 end

spec/models/site_setting_spec.rb

@@ -47,10 +47,6 @@ RSpec.describe SiteSetting do
     it "sets a setting" do
       SiteSetting.contact_email = "sam@sam.com"
     end
-
-    it "is always the correct default" do
-      expect(SiteSetting.contact_email).to eq("")
-    end
   end
 
   describe "anonymous_homepage" do

spec/requests/admin/themes_controller_spec.rb

@@ -73,12 +73,8 @@ RSpec.describe Admin::ThemesController do
         let(:uploaded_file) { Upload.find_by(original_filename: filename) }
         let(:response_json) { response.parsed_body }
 
-        before do
-          post "/admin/themes/upload_asset.json", params: { file: upload }
-          expect(response.status).to eq(201)
-        end
-
         it "reuses the original upload" do
+          post "/admin/themes/upload_asset.json", params: { file: upload }
           expect(response.status).to eq(201)
           expect(response_json["upload_id"]).to eq(uploaded_file.id)
         end

spec/requests/admin/users_controller_spec.rb

@@ -1321,11 +1321,7 @@ RSpec.describe Admin::UsersController do
         end
 
         it "does not block the urls by default" do
-          delete "/admin/users/#{delete_me.id}.json",
-                 params: {
-                   delete_posts: true,
-                   block_urls: false,
-                 }
+          delete "/admin/users/#{delete_me.id}.json", params: { delete_posts: true }
           expect(response.status).to eq(200)
           expect(ScreenedUrl.exists?(url: @urls)).to eq(false)
         end

spec/requests/published_pages_controller_spec.rb

@@ -80,11 +80,6 @@ RSpec.describe PublishedPagesController do
         end
       end
 
-      it "returns an error for an article you can't see" do
-        get "/pub/no-article-here-no-thx"
-        expect(response.status).to eq(404)
-      end
-
       context "when the article is valid" do
         before do
           SiteSetting.tagging_enabled = true

spec/requests/session_controller_spec.rb

@@ -1906,9 +1906,7 @@ RSpec.describe SessionController do
             I18n.t("login.incorrect_username_email_or_password"),
           )
         end
-      end
 
-      describe "invalid password" do
         it "should return an error with an invalid password if too long" do
           User.any_instance.expects(:confirm_password?).never
           post "/session.json",

spec/requests/static_controller_spec.rb

@@ -171,11 +171,6 @@ RSpec.describe StaticController do
           expect(response).to redirect_to "/login"
         end
 
-        it "#{page_name} page redirects to login page for anon" do
-          get "/#{page_name}"
-          expect(response).to redirect_to "/login"
-        end
-
         it "#{page_name} page loads for logged in user" do
           sign_in(Fabricate(:user))
 

spec/requests/topics_controller_spec.rb

@@ -1432,11 +1432,6 @@ RSpec.describe TopicsController do
       put "/t/99/mute.json"
       expect(response.status).to eq(403)
     end
-
-    it "needs you to be logged in" do
-      put "/t/99/unmute.json"
-      expect(response.status).to eq(403)
-    end
   end
 
   describe "#recover" do

spec/requests/user_badges_controller_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe UserBadgesController do
 
   describe "#index" do
     fab!(:badge) { Fabricate(:badge, target_posts: true, show_posts: false) }
+
     it "does not leak private info" do
       p = create_post
       UserBadge.create!(
@@ -39,9 +40,14 @@ RSpec.describe UserBadgesController do
       get "/user_badges.json", params: { badge_id: badge.id, offset: 100 }
       expect(response.status).to eq(200)
     end
+
+    it "requires username or badge_id to be specified" do
+      get "/user_badges.json"
+      expect(response.status).to eq(400)
+    end
   end
 
-  describe "#index" do
+  describe "#show" do
     fab!(:post)
     fab!(:private_message_post)
     let(:topic) { post.topic }
@@ -55,11 +61,6 @@ RSpec.describe UserBadgesController do
     fab!(:user_badge_2) { Fabricate(:user_badge, badge: badge, post: private_message_post) }
     fab!(:user_badge_3) { Fabricate(:user_badge, badge: badge, post: restricted_post) }
 
-    it "requires username or badge_id to be specified" do
-      get "/user_badges.json"
-      expect(response.status).to eq(400)
-    end
-
     it "returns user_badges for a user" do
       get "/user-badges/#{user.username}.json"
 

spec/requests/wizard_controller_spec.rb

@@ -7,9 +7,7 @@ RSpec.describe WizardController do
     it "needs you to be logged in" do
       get "/wizard.json"
       expect(response.status).to eq(403)
-    end
 
-    it "needs you to be logged in" do
       get "/wizard"
       # for whatever reason, no access is 404
       # we may want to revisit this at some point and make it 403

spec/services/hashtag_autocomplete_service_spec.rb

@@ -248,13 +248,7 @@ RSpec.describe HashtagAutocompleteService do
       fab!(:tag3) { Fabricate(:tag, name: "terrible-books") }
       fab!(:tag4) { Fabricate(:tag, name: "book") }
 
-      it "orders them by name within their type order" do
-        expect(service.search("book", %w[category tag], limit: 10).map(&:ref)).to eq(
-          %w[book book::tag book-dome book-zone the-book-club great-books mid-books terrible-books],
-        )
-      end
-
-      it "prioritises exact matches to the top of the list" do
+      it "orders them by name within their type order and prioritizes exact matches to the top of the list" do
         expect(service.search("book", %w[category tag], limit: 10).map(&:ref)).to eq(
           %w[book book::tag book-dome book-zone the-book-club great-books mid-books terrible-books],
         )

spec/services/inline_uploads_spec.rb

@@ -48,9 +48,7 @@ RSpec.describe InlineUploads do
 
         This is an invalid `<img ...>` tag
         MD
-      end
 
-      it "should work with invalid img tags" do
         md = '<img data-id="<>">'
         expect(InlineUploads.process(md)).to eq(md)
       end

spec/services/sidebar_site_settings_backfiller_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe SidebarSiteSettingsBackfiller do
         expect(SidebarSectionLink.exists?(id: category_sidebar_section_link_ids)).to eq(false)
       end
 
-      it "creates the right sidebar section link records when categories are added" do
+      it "updates the right sidebar section link records when categories are added" do
         backfiller =
           described_class.new(
             "default_navigation_menu_categories",

spec/services/staff_action_logger_spec.rb

@@ -690,32 +690,6 @@ RSpec.describe StaffActionLogger do
     end
   end
 
-  describe "log_post_staff_note" do
-    fab!(:post)
-
-    it "raises an error when argument is missing" do
-      expect { logger.log_topic_archived(nil) }.to raise_error(Discourse::InvalidParameters)
-    end
-
-    it "creates a new UserHistory record" do
-      expect {
-        logger.log_post_staff_note(post, { new_value: "my note", old_value: nil })
-      }.to change { UserHistory.count }.by(1)
-      user_history = UserHistory.last
-      expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_create])
-      expect(user_history.new_value).to eq("my note")
-      expect(user_history.previous_value).to eq(nil)
-
-      expect {
-        logger.log_post_staff_note(post, { new_value: "", old_value: "my note" })
-      }.to change { UserHistory.count }.by(1)
-      user_history = UserHistory.last
-      expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_destroy])
-      expect(user_history.new_value).to eq(nil)
-      expect(user_history.previous_value).to eq("my note")
-    end
-  end
-
   describe "log_post_staff_note" do
     fab!(:post)