From c6202af005f10981f8575fd2a40566fc860a3f3e Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 24 Jul 2020 17:16:52 +0800 Subject: [PATCH] Update rubocop to 2.3.1. --- Gemfile.lock | 2 +- app/jobs/base.rb | 2 +- spec/components/admin_confirmation_spec.rb | 6 +- spec/components/cooked_post_processor_spec.rb | 9 +- spec/components/discourse_updates_spec.rb | 15 ++- spec/components/email/processor_spec.rb | 10 +- spec/components/email_updater_spec.rb | 105 ++++++++++++------ spec/components/post_creator_spec.rb | 17 +-- spec/components/post_destroyer_spec.rb | 5 +- spec/jobs/enqueue_digest_emails_spec.rb | 17 ++- spec/jobs/jobs_spec.rb | 12 +- spec/models/post_action_spec.rb | 11 +- spec/models/reviewable_user_spec.rb | 31 ++---- spec/models/topic_timer_spec.rb | 37 ++---- spec/models/user_option_spec.rb | 9 +- spec/models/user_spec.rb | 17 +-- spec/rails_helper.rb | 1 + spec/requests/admin/badges_controller_spec.rb | 13 +-- .../admin/site_texts_controller_spec.rb | 39 +++---- spec/requests/admin/users_controller_spec.rb | 10 +- spec/requests/users_controller_spec.rb | 26 +++-- spec/services/notification_emailer_spec.rb | 88 +++++++++++---- spec/services/user_activator_spec.rb | 22 ++-- spec/support/sidekiq_helpers.rb | 66 +++++++++++ 24 files changed, 360 insertions(+), 210 deletions(-) create mode 100644 spec/support/sidekiq_helpers.rb diff --git a/Gemfile.lock b/Gemfile.lock index b460b627c3a..73fba9a698a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -359,7 +359,7 @@ GEM unicode-display_width (>= 1.4.0, < 2.0) rubocop-ast (0.2.0) parser (>= 2.7.0.1) - rubocop-discourse (2.2.0) + rubocop-discourse (2.3.1) rubocop (>= 0.69.0) rubocop-rspec (>= 1.39.0) rubocop-rspec (1.42.0) diff --git a/app/jobs/base.rb b/app/jobs/base.rb index da1623f8161..b69302861a3 100644 --- a/app/jobs/base.rb +++ b/app/jobs/base.rb @@ -357,7 +357,7 @@ module Jobs end def self.enqueue_at(datetime, job_name, opts = {}) - secs = [(datetime - Time.zone.now).to_i, 0].max + secs = [datetime.to_f - Time.zone.now.to_f, 0].max enqueue_in(secs, job_name, opts) end diff --git a/spec/components/admin_confirmation_spec.rb b/spec/components/admin_confirmation_spec.rb index 749c566090d..14740524611 100644 --- a/spec/components/admin_confirmation_spec.rb +++ b/spec/components/admin_confirmation_spec.rb @@ -33,8 +33,10 @@ describe AdminConfirmation do expect(ac.performed_by).to eq(admin) expect(ac.target_user).to eq(user) expect(ac.token).to eq(@token) - Jobs.expects(:enqueue).with(:send_system_message, user_id: user.id, message_type: 'welcome_staff', message_options: { role: :admin }) - ac.email_confirmed! + + expect_enqueued_with(job: :send_system_message, args: { user_id: user.id, message_type: 'welcome_staff', message_options: { role: :admin } }) do + ac.email_confirmed! + end user.reload expect(user.admin?).to eq(true) diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 9164a7a4786..a4b9c2bc143 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -1491,19 +1491,22 @@ describe CookedPostProcessor do end context "and there is enough disk space" do - before { cpp.expects(:disable_if_low_on_disk_space) } + before { cpp.expects(:disable_if_low_on_disk_space).at_least_once } context "and the post has been updated by an actual user" do before { post.id = 42 } it "ensures only one job is scheduled right after the editing_grace_period" do + freeze_time + Jobs.expects(:cancel_scheduled_job).with(:pull_hotlinked_images, post_id: post.id).once delay = SiteSetting.editing_grace_period + 1 - Jobs.expects(:enqueue_in).with(delay.seconds, :pull_hotlinked_images, post_id: post.id).once - cpp.pull_hotlinked_images + expect_enqueued_with(job: :pull_hotlinked_images, args: { post_id: post.id }, at: Time.zone.now + delay.seconds) do + cpp.pull_hotlinked_images + end end end diff --git a/spec/components/discourse_updates_spec.rb b/spec/components/discourse_updates_spec.rb index 8e9ef2596b4..4b83510c245 100644 --- a/spec/components/discourse_updates_spec.rb +++ b/spec/components/discourse_updates_spec.rb @@ -76,8 +76,9 @@ describe DiscourseUpdates do end it 'queues a version check' do - Jobs.expects(:enqueue).with(:version_check, anything) - subject + expect_enqueued_with(job: :version_check) do + subject + end end end @@ -86,8 +87,9 @@ describe DiscourseUpdates do context 'old version check data' do shared_examples "queue version check and report that version is ok" do it 'queues a version check' do - Jobs.expects(:enqueue).with(:version_check, anything) - subject + expect_enqueued_with(job: :version_check) do + subject + end end it 'reports 0 missing versions' do @@ -118,8 +120,9 @@ describe DiscourseUpdates do shared_examples "when last_installed_version is old" do it 'queues a version check' do - Jobs.expects(:enqueue).with(:version_check, anything) - subject + expect_enqueued_with(job: :version_check) do + subject + end end it 'reports 0 missing versions' do diff --git a/spec/components/email/processor_spec.rb b/spec/components/email/processor_spec.rb index 8a4a5ea355b..85d41482822 100644 --- a/spec/components/email/processor_spec.rb +++ b/spec/components/email/processor_spec.rb @@ -59,13 +59,15 @@ describe Email::Processor do end it "enqueues a background job by default" do - Jobs.expects(:enqueue).with(:process_email, mail: mail) - Email::Processor.process!(mail, retry_on_rate_limit: true) + expect_enqueued_with(job: :process_email, args: { mail: mail }) do + Email::Processor.process!(mail, retry_on_rate_limit: true) + end end it "doesn't enqueue a background job when retry is disabled" do - Jobs.expects(:enqueue).with(:process_email, mail: mail).never - expect { Email::Processor.process!(mail, retry_on_rate_limit: false) }.to raise_error(limit_exceeded) + expect_not_enqueued_with(job: :process_email, args: { mail: mail }) do + expect { Email::Processor.process!(mail, retry_on_rate_limit: false) }.to raise_error(limit_exceeded) + end end end diff --git a/spec/components/email_updater_spec.rb b/spec/components/email_updater_spec.rb index 3b110a78cae..4fd6cbaf6c9 100644 --- a/spec/components/email_updater_spec.rb +++ b/spec/components/email_updater_spec.rb @@ -22,21 +22,29 @@ describe EmailUpdater do let(:updater) { EmailUpdater.new(guardian: admin.guardian, user: user) } def expect_old_email_job - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(to_address: old_email, type: :notify_old_email, user_id: user.id)) + expect_enqueued_with(job: :critical_user_email, args: { to_address: old_email, type: :notify_old_email, user_id: user.id }) do + yield + end end def expect_forgot_password_job - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :forgot_password, user_id: user.id)) + expect_enqueued_with(job: :critical_user_email, args: { type: :forgot_password, user_id: user.id }) do + yield + end end context "for a regular user" do let(:user) { Fabricate(:user, email: old_email) } it "does not send an email to the user for them to confirm their new email but still sends the notification to the old email" do - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)).never - expect_old_email_job - expect_forgot_password_job - updater.change_to(new_email) + expect_old_email_job do + expect_forgot_password_job do + updater.change_to(new_email) + + expect(Jobs::CriticalUserEmail.jobs.size).to eq(2) + end + end + end it "creates a change request authorizing the new email and immediately confirms it " do @@ -46,9 +54,11 @@ describe EmailUpdater do end it "sends a reset password email to the user so they can set a password for their new email" do - expect_old_email_job - expect_forgot_password_job - updater.change_to(new_email) + expect_old_email_job do + expect_forgot_password_job do + updater.change_to(new_email) + end + end end end @@ -56,8 +66,10 @@ describe EmailUpdater do let(:user) { Fabricate(:moderator, email: old_email) } before do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_old_email, to_address: old_email)) - updater.change_to(new_email) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_old_email, to_address: old_email }) do + updater.change_to(new_email) + end + @change_req = user.email_change_requests.first end @@ -83,8 +95,11 @@ describe EmailUpdater do before do admin.update(email: old_email) - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_old_email, to_address: old_email)) - updater.change_to(new_email) + + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_old_email, to_address: old_email }) do + updater.change_to(new_email) + end + @change_req = user.email_change_requests.first end @@ -112,8 +127,10 @@ describe EmailUpdater do context "changing primary email" do before do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) - updater.change_to(new_email) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_new_email, to_address: new_email }) do + updater.change_to(new_email) + end + @change_req = user.email_change_requests.first end @@ -139,10 +156,10 @@ describe EmailUpdater do context 'confirming a valid token' do it "updates the user's email" do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email)) - event = DiscourseEvent.track_events { - updater.confirm(@change_req.new_email_token.token) + expect_enqueued_with(job: :critical_user_email, args: { type: :notify_old_email, to_address: old_email }) do + updater.confirm(@change_req.new_email_token.token) + end }.last expect(updater.errors).to be_blank @@ -159,8 +176,10 @@ describe EmailUpdater do context "adding an email" do before do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) - updater.change_to(new_email, add: true) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_new_email, to_address: new_email }) do + updater.change_to(new_email, add: true) + end + @change_req = user.email_change_requests.first end @@ -168,10 +187,10 @@ describe EmailUpdater do it "adds a user email" do expect(UserHistory.where(action: UserHistory.actions[:add_email], acting_user_id: user.id).last).to be_present - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email_add, to_address: old_email)) - event = DiscourseEvent.track_events { - updater.confirm(@change_req.new_email_token.token) + expect_enqueued_with(job: :critical_user_email, args: { type: :notify_old_email_add, to_address: old_email }) do + updater.confirm(@change_req.new_email_token.token) + end }.last expect(updater.errors).to be_blank @@ -187,19 +206,25 @@ describe EmailUpdater do context 'that was deleted before' do it 'works' do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email_add, to_address: old_email)) - updater.confirm(@change_req.new_email_token.token) + expect_enqueued_with(job: :critical_user_email, args: { type: :notify_old_email_add, to_address: old_email }) do + updater.confirm(@change_req.new_email_token.token) + end + expect(user.reload.user_emails.pluck(:email)).to contain_exactly(old_email, new_email) user.user_emails.where(email: new_email).delete_all expect(user.reload.user_emails.pluck(:email)).to contain_exactly(old_email) - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) - updater.change_to(new_email, add: true) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_new_email, to_address: new_email }) do + updater.change_to(new_email, add: true) + end + @change_req = user.email_change_requests.first - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email_add, to_address: old_email)) - updater.confirm(@change_req.new_email_token.token) + expect_enqueued_with(job: :critical_user_email, args: { type: :notify_old_email_add, to_address: old_email }) do + updater.confirm(@change_req.new_email_token.token) + end + expect(user.reload.user_emails.pluck(:email)).to contain_exactly(old_email, new_email) end end @@ -211,8 +236,10 @@ describe EmailUpdater do let(:updater) { EmailUpdater.new(guardian: user.guardian, user: user) } before do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_old_email, to_address: old_email)) - updater.change_to(new_email) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_old_email, to_address: old_email }) do + updater.change_to(new_email) + end + @change_req = user.email_change_requests.first end @@ -238,8 +265,10 @@ describe EmailUpdater do context 'confirming a valid token' do before do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :confirm_new_email, to_address: new_email)) - updater.confirm(@change_req.old_email_token.token) + expect_enqueued_with(job: :critical_user_email, args: { type: :confirm_new_email, to_address: new_email }) do + updater.confirm(@change_req.old_email_token.token) + end + @change_req.reload end @@ -263,8 +292,9 @@ describe EmailUpdater do context "completing the new update process" do before do - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email)).never - updater.confirm(@change_req.new_email_token.token) + expect_not_enqueued_with(job: :critical_user_email, args: { type: :notify_old_email, to_address: old_email }) do + updater.confirm(@change_req.new_email_token.token) + end end it "updates the user's email" do @@ -294,8 +324,9 @@ describe EmailUpdater do end it 'sends an email to the owner of the account with the new email' do - Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :account_exists, user_id: existing.id)) - updater.change_to(existing.email) + expect_enqueued_with(job: :critical_user_email, args: { type: :account_exists, user_id: existing.id }) do + updater.change_to(existing.email) + end end end end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 196bcfa8c07..636bf9c5df5 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -221,23 +221,18 @@ describe PostCreator do end it 'passes the invalidate_oneboxes along to the job if present' do - Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id)) - Jobs.expects(:enqueue).with(:notify_mailing_list_subscribers, has_key(:post_id)) - Jobs.expects(:enqueue).with(:post_alert, has_key(:post_id)) - Jobs.expects(:enqueue).with(:update_topic_upload_security, has_key(:topic_id)) - Jobs.expects(:enqueue).with(:process_post, has_key(:invalidate_oneboxes)) creator.opts[:invalidate_oneboxes] = true creator.create + + expect(job_enqueued?(job: :process_post, args: { invalidate_oneboxes: true })).to eq(true) end it 'passes the image_sizes along to the job if present' do - Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id)) - Jobs.expects(:enqueue).with(:notify_mailing_list_subscribers, has_key(:post_id)) - Jobs.expects(:enqueue).with(:post_alert, has_key(:post_id)) - Jobs.expects(:enqueue).with(:update_topic_upload_security, has_key(:topic_id)) - Jobs.expects(:enqueue).with(:process_post, has_key(:image_sizes)) - creator.opts[:image_sizes] = { 'http://an.image.host/image.jpg' => { 'width' => 17, 'height' => 31 } } + image_sizes = { 'http://an.image.host/image.jpg' => { 'width' => 17, 'height' => 31 } } + creator.opts[:image_sizes] = image_sizes creator.create + + expect(job_enqueued?(job: :process_post, args: { image_sizes: image_sizes })).to eq(true) end it 'assigns a category when supplied' do diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 45c298c4ba6..75eb1494ae5 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -738,8 +738,9 @@ describe PostDestroyer do fab!(:post) { Fabricate(:post, raw: "Hello @CodingHorror") } it "should feature the users again (in case they've changed)" do - Jobs.expects(:enqueue).with(:feature_topic_users, has_entries(topic_id: post.topic_id)) - PostDestroyer.new(moderator, post).destroy + expect_enqueued_with(job: :feature_topic_users, args: { topic_id: post.topic_id }) do + PostDestroyer.new(moderator, post).destroy + end end describe 'with a reply' do diff --git a/spec/jobs/enqueue_digest_emails_spec.rb b/spec/jobs/enqueue_digest_emails_spec.rb index 3c9fcad5d66..fa3fea9f890 100644 --- a/spec/jobs/enqueue_digest_emails_spec.rb +++ b/spec/jobs/enqueue_digest_emails_spec.rb @@ -128,8 +128,10 @@ describe Jobs::EnqueueDigestEmails do it "enqueues the digest email job" do SiteSetting.disable_digest_emails = false - Jobs.expects(:enqueue).with(:user_email, type: :digest, user_id: user.id) - Jobs::EnqueueDigestEmails.new.execute({}) + + expect_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do + Jobs::EnqueueDigestEmails.new.execute({}) + end end end @@ -137,10 +139,12 @@ describe Jobs::EnqueueDigestEmails do before do Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never SiteSetting.private_email = true - Jobs.expects(:enqueue).with(:user_email, type: :digest, user_id: user.id).never end + it "doesn't return users with email disabled" do - Jobs::EnqueueDigestEmails.new.execute({}) + expect_not_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do + Jobs::EnqueueDigestEmails.new.execute({}) + end end end @@ -151,8 +155,9 @@ describe Jobs::EnqueueDigestEmails do end it "does not enqueue the digest email job" do - Jobs.expects(:enqueue).with(:user_email, type: :digest, user_id: user.id).never - Jobs::EnqueueDigestEmails.new.execute({}) + expect_not_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do + Jobs::EnqueueDigestEmails.new.execute({}) + end end end diff --git a/spec/jobs/jobs_spec.rb b/spec/jobs/jobs_spec.rb index d956b92a988..0f3f8d48ca0 100644 --- a/spec/jobs/jobs_spec.rb +++ b/spec/jobs/jobs_spec.rb @@ -143,14 +143,18 @@ describe Jobs do describe 'enqueue_at' do it 'calls enqueue_in for you' do freeze_time - Jobs.expects(:enqueue_in).with(3 * 60 * 60, :eat_lunch, {}).returns(true) - Jobs.enqueue_at(3.hours.from_now, :eat_lunch, {}) + + expect_enqueued_with(job: :process_post, at: 3.hours.from_now) do + Jobs.enqueue_at(3.hours.from_now, :process_post, {}) + end end it 'handles datetimes that are in the past' do freeze_time - Jobs.expects(:enqueue_in).with(0, :eat_lunch, {}).returns(true) - Jobs.enqueue_at(3.hours.ago, :eat_lunch, {}) + + expect_enqueued_with(job: :process_post, at: Time.zone.now) do + Jobs.enqueue_at(3.hours.ago, :process_post, {}) + end end end diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 8cdc6591a1f..5ffc7d7db50 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -763,13 +763,10 @@ describe PostAction do expect(timer.execute_at).to eq_time(1.hour.from_now) freeze_time timer.execute_at - Jobs.expects(:enqueue_in).with( - 1.hour.to_i, - :toggle_topic_closed, - topic_timer_id: timer.id, - state: false - ).returns(true) - Jobs::ToggleTopicClosed.new.execute(topic_timer_id: timer.id, state: false) + + expect_enqueued_with(job: :toggle_topic_closed, args: { topic_timer_id: timer.id, state: false }, at: Time.zone.now + 1.hour) do + Jobs::ToggleTopicClosed.new.execute(topic_timer_id: timer.id, state: false) + end expect(topic.reload.closed).to eq(true) expect(timer.reload.execute_at).to eq_time(1.hour.from_now) diff --git a/spec/models/reviewable_user_spec.rb b/spec/models/reviewable_user_spec.rb index bb0899b6bce..ba00b8641db 100644 --- a/spec/models/reviewable_user_spec.rb +++ b/spec/models/reviewable_user_spec.rb @@ -134,38 +134,27 @@ RSpec.describe ReviewableUser, type: :model do before do SiteSetting.must_approve_users = true Jobs.run_immediately! + @reviewable = ReviewableUser.find_by(target: user) + Jobs.run_later! end it "creates the ReviewableUser for a user, with moderator access" do - reviewable = ReviewableUser.find_by(target: user) - expect(reviewable).to be_present - expect(reviewable.reviewable_by_moderator).to eq(true) + expect(@reviewable.reviewable_by_moderator).to eq(true) end context "email jobs" do - let(:reviewable) { ReviewableUser.find_by(target: user) } - before do - reviewable - - # We can ignore these notifications for the purpose of this test - Jobs.stubs(:enqueue).with(:notify_reviewable, has_key(:reviewable_id)) - end - - after do - ReviewableUser.find_by(target: user).perform(admin, :approve_user) - end - it "enqueues a 'signup after approval' email if must_approve_users is true" do - Jobs.expects(:enqueue).with( - :critical_user_email, has_entries(type: :signup_after_approval) - ) + expect_enqueued_with(job: :critical_user_email, args: { type: :signup_after_approval }) do + @reviewable.perform(admin, :approve_user) + end end it "doesn't enqueue a 'signup after approval' email if must_approve_users is false" do SiteSetting.must_approve_users = false - Jobs.expects(:enqueue).with( - :critical_user_email, has_entries(type: :signup_after_approval) - ).never + + expect_not_enqueued_with(job: :critical_user_email, args: { type: :signup_after_approval }) do + @reviewable.perform(admin, :approve_user) + end end end diff --git a/spec/models/topic_timer_spec.rb b/spec/models/topic_timer_spec.rb index ad093c223ec..6c19bd6ca79 100644 --- a/spec/models/topic_timer_spec.rb +++ b/spec/models/topic_timer_spec.rb @@ -109,27 +109,19 @@ RSpec.describe TopicTimer, type: :model do :toggle_topic_closed, topic_timer_id: topic_timer.id ) - Jobs.expects(:enqueue_at).with( - 3.days.from_now, :toggle_topic_closed, - topic_timer_id: topic_timer.id, - state: true - ) - - topic_timer.update!(execute_at: 3.days.from_now, created_at: Time.zone.now) + expect_enqueued_with(job: :toggle_topic_closed, args: { topic_timer_id: topic_timer.id, state: true }, at: 3.days.from_now) do + topic_timer.update!(execute_at: 3.days.from_now, created_at: Time.zone.now) + end end describe 'when execute_at is smaller than the current time' do it 'should enqueue the job immediately' do - Jobs.expects(:enqueue_at).with( - Time.zone.now, :toggle_topic_closed, - topic_timer_id: topic_timer.id, - state: true - ) - - topic_timer.update!( - execute_at: Time.zone.now - 1.hour, - created_at: Time.zone.now - 2.hour - ) + expect_enqueued_with(job: :toggle_topic_closed, args: { topic_timer_id: topic_timer.id, state: true }, at: Time.zone.now) do + topic_timer.update!( + execute_at: Time.zone.now - 1.hour, + created_at: Time.zone.now - 2.hour + ) + end end end end @@ -140,14 +132,9 @@ RSpec.describe TopicTimer, type: :model do :toggle_topic_closed, topic_timer_id: topic_timer.id ) - Jobs.expects(:enqueue_at).with( - topic_timer.execute_at, - :toggle_topic_closed, - topic_timer_id: topic_timer.id, - state: true - ) - - topic_timer.update!(user: admin) + expect_enqueued_with(job: :toggle_topic_closed, args: { topic_timer_id: topic_timer.id, state: true }, at: topic_timer.execute_at) do + topic_timer.update!(user: admin) + end end end diff --git a/spec/models/user_option_spec.rb b/spec/models/user_option_spec.rb index 715b1e67cc8..621b585ea93 100644 --- a/spec/models/user_option_spec.rb +++ b/spec/models/user_option_spec.rb @@ -122,12 +122,17 @@ describe UserOption do user.stubs(:last_seen_at).returns(5.minutes.ago) end + after do + $redis.flushdb + end + it "should have a reason for the first visit" do freeze_time do delay = SiteSetting.active_user_rate_limit_secs / 2 - Jobs.expects(:enqueue_in).with(delay, :update_top_redirection, user_id: user.id, redirected_at: Time.zone.now) - expect(user.user_option.redirected_to_top).to eq(reason: I18n.t('redirected_to_top_reasons.new_user'), period: :monthly) + expect_enqueued_with(job: :update_top_redirection, args: { user_id: user.id, redirected_at: Time.zone.now.to_s }, at: Time.zone.now + delay) do + expect(user.user_option.redirected_to_top).to eq(reason: I18n.t('redirected_to_top_reasons.new_user'), period: :monthly) + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 791153b43ff..313d3819900 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -153,14 +153,17 @@ describe User do let(:user) { Fabricate(:user) } it 'enqueues the system message' do - Jobs.expects(:enqueue).with(:send_system_message, user_id: user.id, message_type: 'welcome_user') - user.enqueue_welcome_message('welcome_user') + expect_enqueued_with(job: :send_system_message, args: { user_id: user.id, message_type: 'welcome_user' }) do + user.enqueue_welcome_message('welcome_user') + end end it "doesn't enqueue the system message when the site settings disable it" do SiteSetting.send_welcome_message = false - Jobs.expects(:enqueue).with(:send_system_message, user_id: user.id, message_type: 'welcome_user').never - user.enqueue_welcome_message('welcome_user') + + expect_not_enqueued_with(job: :send_system_message, args: { user_id: user.id, message_type: 'welcome_user' }) do + user.enqueue_welcome_message('welcome_user') + end end end @@ -1433,9 +1436,9 @@ describe User do user = Fabricate(:user) - Jobs.expects(:enqueue).with(:update_gravatar, anything) - - user.refresh_avatar + expect_enqueued_with(job: :update_gravatar, args: { user_id: user.id }) do + user.refresh_avatar + end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 6b6d1265b08..5819be7faa8 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -172,6 +172,7 @@ RSpec.configure do |config| config.include IntegrationHelpers, type: :request config.include WebauthnIntegrationHelpers config.include SiteSettingsHelpers + config.include SidekiqHelpers config.mock_framework = :mocha config.order = 'random' config.infer_spec_type_from_file_location! diff --git a/spec/requests/admin/badges_controller_spec.rb b/spec/requests/admin/badges_controller_spec.rb index 185143e4f96..14e7d29f7fc 100644 --- a/spec/requests/admin/badges_controller_spec.rb +++ b/spec/requests/admin/badges_controller_spec.rb @@ -164,16 +164,15 @@ describe Admin::BadgesController do end it 'updates the user title in a job' do - Jobs.expects(:enqueue).with( - :bulk_user_title_update, + expect_enqueued_with(job: :bulk_user_title_update, args: { new_title: 'Shieldbearer', granted_badge_id: badge.id, action: Jobs::BulkUserTitleUpdate::UPDATE_ACTION - ) - - put "/admin/badges/#{badge.id}.json", params: { - name: "Shieldbearer" - } + }) do + put "/admin/badges/#{badge.id}.json", params: { + name: "Shieldbearer" + } + end end end end diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb index 22fbe7a7e97..09985c0076a 100644 --- a/spec/requests/admin/site_texts_controller_spec.rb +++ b/spec/requests/admin/site_texts_controller_spec.rb @@ -453,36 +453,31 @@ RSpec.describe Admin::SiteTextsController do end it 'updates matching user titles to the override text in a job' do - Jobs.expects(:enqueue).with( - :bulk_user_title_update, + expect_enqueued_with(job: :bulk_user_title_update, args: { new_title: 'Terminator', granted_badge_id: badge.id, action: Jobs::BulkUserTitleUpdate::UPDATE_ACTION - ) - put '/admin/customize/site_texts/badges.regular.name.json', params: { - site_text: { value: 'Terminator' } - } - - Jobs.expects(:enqueue).with( - :bulk_user_title_update, - granted_badge_id: badge.id, - action: Jobs::BulkUserTitleUpdate::RESET_ACTION - ) + }) do + put '/admin/customize/site_texts/badges.regular.name.json', params: { + site_text: { value: 'Terminator' } + } + end # Revert - delete "/admin/customize/site_texts/badges.regular.name.json" + expect_enqueued_with(job: :bulk_user_title_update, args: { + granted_badge_id: badge.id, + action: Jobs::BulkUserTitleUpdate::RESET_ACTION + }) do + delete "/admin/customize/site_texts/badges.regular.name.json" + end end it 'does not update matching user titles when overriding non-title badge text' do - Jobs.expects(:enqueue).with( - :bulk_user_title_update, - new_title: 'Terminator', - granted_badge_id: badge.id, - action: Jobs::BulkUserTitleUpdate::UPDATE_ACTION - ).never - put '/admin/customize/site_texts/badges.regular.long_description.json', params: { - site_text: { value: 'Terminator' } - } + expect_not_enqueued_with(job: :bulk_user_title_update) do + put '/admin/customize/site_texts/badges.regular.long_description.json', params: { + site_text: { value: 'Terminator' } + } + end end end end diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 3cbdf5a7d85..e010f365bf0 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -453,8 +453,14 @@ RSpec.describe Admin::UsersController do end it 'updates the moderator flag' do - Jobs.expects(:enqueue).with(:send_system_message, user_id: another_user.id, message_type: 'welcome_staff', message_options: { role: :moderator }) - put "/admin/users/#{another_user.id}/grant_moderation.json" + expect_enqueued_with(job: :send_system_message, args: { + user_id: another_user.id, + message_type: 'welcome_staff', + message_options: { role: :moderator } + }) do + put "/admin/users/#{another_user.id}/grant_moderation.json" + end + expect(response.status).to eq(200) another_user.reload expect(another_user.moderator).to eq(true) diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 4d1707d33d9..629c8f2f55e 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2137,14 +2137,14 @@ describe UsersController do context 'for an activated account with unconfirmed email' do it 'should send an email' do user = post_user - user.update(active: true) - user.save! - user.email_tokens.create(email: user.email) - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup, to_address: user.email)) + user.update!(active: true) + user.email_tokens.create!(email: user.email) - post "/u/action/send_activation_email.json", params: { - username: user.username - } + expect_enqueued_with(job: :critical_user_email, args: { type: :signup, to_address: user.email }) do + post "/u/action/send_activation_email.json", params: { + username: user.username + } + end expect(response.status).to eq(200) @@ -2192,10 +2192,14 @@ describe UsersController do context 'with a valid email_token' do it 'should send the activation email' do user = post_user - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) - post "/u/action/send_activation_email.json", params: { - username: user.username - } + + expect_enqueued_with(job: :critical_user_email, args: { type: :signup }) do + post "/u/action/send_activation_email.json", params: { + username: user.username + } + end + + expect(response.status).to eq(200) expect(session[SessionController::ACTIVATE_USER_KEY]).to eq(nil) end end diff --git a/spec/services/notification_emailer_spec.rb b/spec/services/notification_emailer_spec.rb index 9ca6d409575..c986f725185 100644 --- a/spec/services/notification_emailer_spec.rb +++ b/spec/services/notification_emailer_spec.rb @@ -5,6 +5,7 @@ require 'rails_helper' describe NotificationEmailer do before do + freeze_time NotificationEmailer.enable end @@ -24,37 +25,65 @@ describe NotificationEmailer do shared_examples "enqueue" do it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification, type)) - NotificationEmailer.process_notification(notification) + expect_enqueued_with( + job: :user_email, + args: NotificationEmailer::EmailUser.notification_params(notification, type), + at: Time.zone.now + delay + ) do + NotificationEmailer.process_notification(notification) + end end context "inactive user" do before { notification.user.active = false } it "doesn't enqueue a job" do - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end it "enqueues a job if the user is staged for non-linked and non-quoted types" do notification.user.staged = true + if type == :user_linked || type == :user_quoted - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never + expect_not_enqueued_with( + job: :user_email, + args: { type: type } + ) do + NotificationEmailer.process_notification(notification) + end else - Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification, type)) + expect_enqueued_with( + job: :user_email, + args: NotificationEmailer::EmailUser.notification_params(notification, type), + at: Time.zone.now + delay + ) do + NotificationEmailer.process_notification(notification) + end end - NotificationEmailer.process_notification(notification) end it "enqueues a job if the user is staged even if site requires user approval for non-linked and non-quoted typed" do notification.user.staged = true SiteSetting.must_approve_users = true + if type == :user_linked || type == :user_quoted - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never + expect_not_enqueued_with( + job: :user_email, + args: { type: type } + ) do + NotificationEmailer.process_notification(notification) + end else - Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification, type)) + expect_enqueued_with( + job: :user_email, + args: NotificationEmailer::EmailUser.notification_params(notification, type), + at: Time.zone.now + delay + ) do + NotificationEmailer.process_notification(notification) + end end - NotificationEmailer.process_notification(notification) end end @@ -66,8 +95,9 @@ describe NotificationEmailer do end it "doesn't enqueue a job" do - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end end @@ -75,8 +105,10 @@ describe NotificationEmailer do it "doesn't enqueue a job" do Post.any_instance.expects(:post_type).returns(Post.types[:small_action]) - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end end @@ -88,8 +120,10 @@ describe NotificationEmailer do it "doesn't enqueue a job if the user has mention emails disabled" do notification.user.user_option.update_columns(email_level: UserOption.email_level_types[:never]) - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end end @@ -98,8 +132,10 @@ describe NotificationEmailer do it "doesn't enqueue a job if the user has private message emails disabled" do notification.user.user_option.update_columns(email_messages_level: UserOption.email_level_types[:never]) - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end end @@ -113,8 +149,14 @@ describe NotificationEmailer do it "enqueue a delayed job for users that are online" do notification.user.last_seen_at = 1.minute.ago - Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification, type)) - NotificationEmailer.process_notification(notification) + + expect_enqueued_with( + job: :user_email, + args: NotificationEmailer::EmailUser.notification_params(notification, type), + at: Time.zone.now + delay + ) do + NotificationEmailer.process_notification(notification) + end end end @@ -160,8 +202,10 @@ describe NotificationEmailer do it "doesn't enqueue a job for a small action" do notification.data_hash["original_post_type"] = Post.types[:small_action] - Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never - NotificationEmailer.process_notification(notification) + + expect_not_enqueued_with(job: :user_email, args: { type: type }) do + NotificationEmailer.process_notification(notification) + end end end diff --git a/spec/services/user_activator_spec.rb b/spec/services/user_activator_spec.rb index 5172843319f..adf3191bab9 100644 --- a/spec/services/user_activator_spec.rb +++ b/spec/services/user_activator_spec.rb @@ -11,8 +11,9 @@ describe UserActivator do user = Fabricate(:user) activator = EmailActivator.new(user, nil, nil, nil) - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup, email_token: user.email_tokens.first.token)) - activator.activate + expect_enqueued_with(job: :critical_user_email, args: { type: :signup, email_token: user.email_tokens.first.token }) do + activator.activate + end end it 'creates and send new email token if the existing token expired' do @@ -24,12 +25,19 @@ describe UserActivator do email_token.update_column(:created_at, 48.hours.ago) activator = EmailActivator.new(user, nil, nil, nil) - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup)) - Jobs.expects(:enqueue).with(:critical_user_email, has_entries(type: :signup, email_token: email_token.token)).never - activator.activate + expect_not_enqueued_with(job: :critical_user_email, args: { type: :signup, user_id: user.id, email_token: email_token.token }) do + activator.activate + end - user.reload - expect(user.email_tokens.last.created_at).to eq_time(now) + email_token = user.reload.email_tokens.last + + expect(job_enqueued?(job: :critical_user_email, args: { + type: :signup, + user_id: user.id, + email_token: email_token.token + })).to eq(true) + + expect(email_token.created_at).to eq_time(now) end end diff --git a/spec/support/sidekiq_helpers.rb b/spec/support/sidekiq_helpers.rb new file mode 100644 index 00000000000..a49e7c144a5 --- /dev/null +++ b/spec/support/sidekiq_helpers.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module SidekiqHelpers + # expect_enqueued_with(job: feature_topic_users, args: { topic_id: topic.id }) do + # PostCreator.new.create + # end + def expect_enqueued_with(job:, args: {}, at: nil, expectation: true) + klass = job.instance_of?(Class) ? job : "::Jobs::#{job.to_s.camelcase}".constantize + at = at.to_f if at.is_a?(Time) + expected = { job: job, args: args, at: at }.compact + original_jobs = klass.jobs.dup + + yield if block_given? + + matched_job = false + jobs = klass.jobs - original_jobs + matched_job = match_jobs(jobs: jobs, args: args, at: at) if jobs.present? + + expect(matched_job).to( + eq(expectation), + expectation ? "No enqueued job with #{expected} found" : "Enqueued job with #{expected} found" + ) + end + + # expect_not_enqueued_with(job: feature_topic_users, args: { topic_id: topic.id }) do + # # Do Nothing... + # end + def expect_not_enqueued_with(job:, args: {}, at: nil) + expect_enqueued_with(job: job, args: args, at: at, expectation: false) do + yield + end + end + + def job_enqueued?(job:, args: {}, at: nil) + klass = job.instance_of?(Class) ? job : "::Jobs::#{job.to_s.camelcase}".constantize + at = at.to_f if at.is_a?(Time) + match_jobs(jobs: klass.jobs, args: args, at: at) + end + + private + + def match_jobs(jobs:, args:, at:) + matched_job = false + + args = JSON.parse(args.to_json) + args.merge!(at: at) if at + + jobs.each do |job| + job_args = job["args"].first.with_indifferent_access + job_args.merge!(at: job["at"]) if job["at"] + job_args.merge!(enqueued_at: job["enqueued_at"]) if job["enqueued_at"] + + matched_job ||= args.all? do |key, value| + value = value.to_s if value.is_a?(Symbol) + + if key == :at && !job_args.has_key?(:at) + value == job_args[:enqueued_at] + else + value == job_args[key] + end + end + end + + matched_job + end +end