diff --git a/plugins/automation/spec/scripts/topic_spec.rb b/plugins/automation/spec/scripts/topic_spec.rb index a76051a2418..93ac44bb9d9 100644 --- a/plugins/automation/spec/scripts/topic_spec.rb +++ b/plugins/automation/spec/scripts/topic_spec.rb @@ -167,12 +167,11 @@ describe "Topic" do end context "when creating the post fails" do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "logs a warning" do expect { UserUpdater.new(user, user).update(location: "Japan") }.to change { diff --git a/plugins/automation/spec/scripts/user_group_membership_through_badge_spec.rb b/plugins/automation/spec/scripts/user_group_membership_through_badge_spec.rb index d0660317599..baa6c791ce6 100644 --- a/plugins/automation/spec/scripts/user_group_membership_through_badge_spec.rb +++ b/plugins/automation/spec/scripts/user_group_membership_through_badge_spec.rb @@ -28,12 +28,11 @@ describe "UserGroupMembershipThroughBadge" do end context "with invalid field values" do - before do - @original_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @original_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } context "with an unknown badge" do let(:unknown_badge_id) { -1 } @@ -47,7 +46,7 @@ describe "UserGroupMembershipThroughBadge" do end it "logs warning message and does nothing" do - expect(@fake_logger.warnings).to include( + expect(fake_logger.warnings).to include( "[discourse-automation] Couldn’t find badge with id #{unknown_badge_id}", ) expect(user.reload.groups).to be_empty @@ -68,7 +67,7 @@ describe "UserGroupMembershipThroughBadge" do "user" => user, ) - expect(@fake_logger.warnings).to include( + expect(fake_logger.warnings).to include( "[discourse-automation] Couldn’t find group with id #{target_group.id}", ) expect(user.reload.groups).to be_empty diff --git a/plugins/automation/spec/scripts/zapier_webhook_spec.rb b/plugins/automation/spec/scripts/zapier_webhook_spec.rb index fa09c48a8c9..2662b295467 100644 --- a/plugins/automation/spec/scripts/zapier_webhook_spec.rb +++ b/plugins/automation/spec/scripts/zapier_webhook_spec.rb @@ -22,12 +22,11 @@ describe "ZapierWebhook" do end context "with invalid webhook url" do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "logs an error and do nothing" do expect { automation.trigger! }.not_to change { diff --git a/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb b/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb index 7897c52d169..59cba5cc4d7 100644 --- a/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb +++ b/plugins/chat/spec/lib/chat/channel_archive_service_spec.rb @@ -413,7 +413,6 @@ describe Chat::ChannelArchiveService do end it "handles errors gracefully, sends a private message to the archiving user, and is idempotent on retry" do - Rails.logger = @fake_logger = FakeLogger.new create_messages(35) && start_archive Chat::ChannelArchiveService diff --git a/spec/lib/discourse_hub_spec.rb b/spec/lib/discourse_hub_spec.rb index cd1ad998379..0f3bfe1ad92 100644 --- a/spec/lib/discourse_hub_spec.rb +++ b/spec/lib/discourse_hub_spec.rb @@ -106,12 +106,11 @@ RSpec.describe DiscourseHub do end describe ".collection_action" do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "should log correctly on error" do stub_request(:get, (ENV["HUB_BASE_URL"] || "http://local.hub:3000/api") + "/test").to_return( @@ -123,9 +122,9 @@ RSpec.describe DiscourseHub do DiscourseHub.collection_action(:get, "/test") - expect(@fake_logger.warnings).to eq([DiscourseHub.response_status_log_message("/test", 500)]) + expect(fake_logger.warnings).to eq([DiscourseHub.response_status_log_message("/test", 500)]) - expect(@fake_logger.errors).to eq([DiscourseHub.response_body_log_message("")]) + expect(fake_logger.errors).to eq([DiscourseHub.response_body_log_message("")]) end end end diff --git a/spec/lib/discourse_ip_info_spec.rb b/spec/lib/discourse_ip_info_spec.rb index 3525d9a0ff4..0da2049cd60 100644 --- a/spec/lib/discourse_ip_info_spec.rb +++ b/spec/lib/discourse_ip_info_spec.rb @@ -62,8 +62,8 @@ RSpec.describe DiscourseIpInfo do end it "should not throw an error and instead log the exception when database file fails to download" do - original_logger = Rails.logger - Rails.logger = fake_logger = FakeLogger.new + fake_logger = FakeLogger.new + Rails.logger.broadcast_to(fake_logger) global_setting :maxmind_license_key, "license_key" global_setting :maxmind_account_id, "account_id" @@ -81,7 +81,7 @@ RSpec.describe DiscourseIpInfo do "MaxMind database GeoLite2-City download failed. 500 Error", ) ensure - Rails.logger = original_logger + Rails.logger.stop_broadcasting_to(fake_logger) end end end diff --git a/spec/lib/discourse_spec.rb b/spec/lib/discourse_spec.rb index 12a3928d223..610166208b3 100644 --- a/spec/lib/discourse_spec.rb +++ b/spec/lib/discourse_spec.rb @@ -446,12 +446,11 @@ RSpec.describe Discourse do old_method(m) end - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "can deprecate usage" do k = SecureRandom.hex @@ -459,19 +458,19 @@ RSpec.describe Discourse do expect(old_method_caller(k)).to include("discourse_spec") expect(old_method_caller(k)).to include(k) - expect(@fake_logger.warnings).to eq([old_method_caller(k)]) + expect(fake_logger.warnings).to eq([old_method_caller(k)]) end it "can report the deprecated version" do Discourse.deprecate(SecureRandom.hex, since: "2.1.0.beta1") - expect(@fake_logger.warnings[0]).to include("(deprecated since Discourse 2.1.0.beta1)") + expect(fake_logger.warnings[0]).to include("(deprecated since Discourse 2.1.0.beta1)") end it "can report the drop version" do Discourse.deprecate(SecureRandom.hex, drop_from: "2.3.0") - expect(@fake_logger.warnings[0]).to include("(removal in Discourse 2.3.0)") + expect(fake_logger.warnings[0]).to include("(removal in Discourse 2.3.0)") end it "can raise deprecation error" do diff --git a/spec/lib/email/processor_spec.rb b/spec/lib/email/processor_spec.rb index c9da1604ae8..5099edac38c 100644 --- a/spec/lib/email/processor_spec.rb +++ b/spec/lib/email/processor_spec.rb @@ -105,29 +105,27 @@ RSpec.describe Email::Processor do let(:mail2) do "Date: Fri, 15 Jan 2016 00:12:43 +0100\nFrom: #{from}\nTo: foo@foo.com\nSubject: BAR BAR\n\nBar bar bar bar?" end + let(:fake_logger) { FakeLogger.new } + + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "sends a rejection email on an unrecognized error" do - begin - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new + Email::Processor.any_instance.stubs(:can_send_rejection_email?).returns(true) + Email::Receiver.any_instance.stubs(:process_internal).raises("boom") - Email::Processor.any_instance.stubs(:can_send_rejection_email?).returns(true) - Email::Receiver.any_instance.stubs(:process_internal).raises("boom") + Email::Processor.process!(mail) - Email::Processor.process!(mail) + errors = fake_logger.errors + expect(errors.size).to eq(1) + expect(errors.first).to include("boom") - errors = @fake_logger.errors - expect(errors.size).to eq(1) - expect(errors.first).to include("boom") + incoming_email = IncomingEmail.last + expect(incoming_email.error).to eq("RuntimeError") + expect(incoming_email.rejection_message).to be_present - incoming_email = IncomingEmail.last - expect(incoming_email.error).to eq("RuntimeError") - expect(incoming_email.rejection_message).to be_present - - expect(EmailLog.last.email_type).to eq("email_reject_unrecognized_error") - ensure - Rails.logger = @orig_logger - end + expect(EmailLog.last.email_type).to eq("email_reject_unrecognized_error") end it "sends more than one rejection email per day" do diff --git a/spec/lib/middleware/discourse_public_exceptions_spec.rb b/spec/lib/middleware/discourse_public_exceptions_spec.rb index e3545bb9cf6..85c2303879b 100644 --- a/spec/lib/middleware/discourse_public_exceptions_spec.rb +++ b/spec/lib/middleware/discourse_public_exceptions_spec.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true RSpec.describe Middleware::DiscoursePublicExceptions do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } def env(opts = {}) { @@ -27,6 +26,6 @@ RSpec.describe Middleware::DiscoursePublicExceptions do ), ) - expect(@fake_logger.warnings.length).to eq(0) + expect(fake_logger.warnings.length).to eq(0) end end diff --git a/spec/lib/middleware/request_tracker_spec.rb b/spec/lib/middleware/request_tracker_spec.rb index eb57a11892b..fb14b0004af 100644 --- a/spec/lib/middleware/request_tracker_spec.rb +++ b/spec/lib/middleware/request_tracker_spec.rb @@ -505,19 +505,19 @@ RSpec.describe Middleware::RequestTracker do end describe "rate limiting" do + let(:fake_logger) { FakeLogger.new } + before do RateLimiter.enable RateLimiter.clear_all_global! - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - + Rails.logger.broadcast_to(fake_logger) # rate limiter tests depend on checks for retry-after # they can be sensitive to clock skew during test runs freeze_time_safe end - after { Rails.logger = @orig_logger } + after { Rails.logger.stop_broadcasting_to(fake_logger) } let :middleware do app = lambda { |env| [200, {}, ["OK"]] } @@ -559,7 +559,7 @@ RSpec.describe Middleware::RequestTracker do status, _ = middleware.call(env1) status, _ = middleware.call(env1) - expect(@fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq( + expect(fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq( warn_count, ) expect(status).to eq(429) @@ -675,9 +675,7 @@ RSpec.describe Middleware::RequestTracker do status, _ = middleware.call(env1) status, _ = middleware.call(env1) - expect(@fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq( - 0, - ) + expect(fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq(0) expect(status).to eq(200) end end @@ -689,7 +687,7 @@ RSpec.describe Middleware::RequestTracker do status, _ = middleware.call(env) status, headers = middleware.call(env) - expect(@fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq(1) + expect(fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq(1) expect(status).to eq(429) expect(headers["Retry-After"]).to eq("10") end @@ -701,7 +699,7 @@ RSpec.describe Middleware::RequestTracker do status, _ = middleware.call(env) status, _ = middleware.call(env) - expect(@fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq(1) + expect(fake_logger.warnings.count { |w| w.include?("Global rate limit exceeded") }).to eq(1) expect(status).to eq(200) end @@ -1077,12 +1075,11 @@ RSpec.describe Middleware::RequestTracker do end describe "error handling" do - before do - @original_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @original_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "logs requests even if they cause exceptions" do app = lambda { |env| raise RateLimiter::LimitExceeded, 1 } @@ -1090,7 +1087,7 @@ RSpec.describe Middleware::RequestTracker do expect { tracker.call(env) }.to raise_error(RateLimiter::LimitExceeded) CachedCounting.flush expect(ApplicationRequest.stats).to include("http_total_total" => 1) - expect(@fake_logger.warnings).to be_empty + expect(fake_logger.warnings).to be_empty end end end diff --git a/spec/lib/mini_scheduler_long_running_job_logger_spec.rb b/spec/lib/mini_scheduler_long_running_job_logger_spec.rb index a83d0326717..e18d5c2ac10 100644 --- a/spec/lib/mini_scheduler_long_running_job_logger_spec.rb +++ b/spec/lib/mini_scheduler_long_running_job_logger_spec.rb @@ -38,12 +38,11 @@ RSpec.describe MiniSchedulerLongRunningJobLogger do manager.stop! end - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "logs long running jobs" do with_running_scheduled_job(Every10MinutesJob) do @@ -58,28 +57,28 @@ RSpec.describe MiniSchedulerLongRunningJobLogger do wait_for { loops == 1 } - expect(@fake_logger.warnings.size).to eq(1) + expect(fake_logger.warnings.size).to eq(1) - expect(@fake_logger.warnings.first).to match( + expect(fake_logger.warnings.first).to match( "Sidekiq scheduled job `Every10MinutesJob` has been running for more than 30 minutes", ) # Matches the backtrace - expect(@fake_logger.warnings.first).to match("sleep") + expect(fake_logger.warnings.first).to match("sleep") # Check that the logger doesn't log repeated warnings after 2 loops expect do checker.thread.wakeup # Force the thread to run the next loop wait_for { loops == 2 } - end.not_to change { @fake_logger.warnings.size } + end.not_to change { fake_logger.warnings.size } # Check that the logger doesn't log repeated warnings after 3 loops expect do checker.thread.wakeup # Force the thread to run the next loop wait_for { loops == 3 } - end.not_to change { @fake_logger.warnings.size } + end.not_to change { fake_logger.warnings.size } ensure checker.stop expect(checker.thread).to eq(nil) @@ -100,9 +99,9 @@ RSpec.describe MiniSchedulerLongRunningJobLogger do wait_for { loops == 1 } - expect(@fake_logger.warnings.size).to eq(1) + expect(fake_logger.warnings.size).to eq(1) - expect(@fake_logger.warnings.first).to match( + expect(fake_logger.warnings.first).to match( "Sidekiq scheduled job `DailyJob` has been running for more than 120 minutes", ) ensure diff --git a/spec/lib/sidekiq_long_running_job_logger_spec.rb b/spec/lib/sidekiq_long_running_job_logger_spec.rb index 2b6486a9742..8964bd9cd93 100644 --- a/spec/lib/sidekiq_long_running_job_logger_spec.rb +++ b/spec/lib/sidekiq_long_running_job_logger_spec.rb @@ -3,12 +3,11 @@ require "sidekiq_long_running_job_logger" RSpec.describe SidekiqLongRunningJobLogger do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "logs long-running jobs" do hostname = Discourse.os_hostname @@ -58,9 +57,9 @@ RSpec.describe SidekiqLongRunningJobLogger do wait_for { loops == 1 } - expect(@fake_logger.warnings.size).to eq(1) + expect(fake_logger.warnings.size).to eq(1) - expect(@fake_logger.warnings).to include( + expect(fake_logger.warnings).to include( "Sidekiq job `TestWorker` has been running for more than 10 minutes\nline\nlines\n", ) @@ -68,9 +67,9 @@ RSpec.describe SidekiqLongRunningJobLogger do wait_for { loops == 2 } - expect(@fake_logger.warnings.size).to eq(1) + expect(fake_logger.warnings.size).to eq(1) - expect(@fake_logger.warnings).to include( + expect(fake_logger.warnings).to include( "Sidekiq job `TestWorker` has been running for more than 10 minutes\nline\nlines\n", ) ensure diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 1830921055f..4c436461369 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -1036,12 +1036,11 @@ RSpec.describe Report do end describe "unexpected error on report initialization" do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "returns no report" do class ReportInitError < StandardError @@ -1053,7 +1052,7 @@ RSpec.describe Report do expect(report).to be_nil - expect(@fake_logger.errors).to eq(["Couldn’t create report `signups`: "]) + expect(fake_logger.errors).to eq(["Couldn’t create report `signups`: "]) end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 58e458139a1..503bfc7f7e4 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -967,12 +967,12 @@ ensure end def track_log_messages - old_logger = Rails.logger - logger = Rails.logger = FakeLogger.new + logger = FakeLogger.new + Rails.logger.broadcast_to(logger) yield logger logger ensure - Rails.logger = old_logger + Rails.logger.stop_broadcasting_to(logger) end # this takes a string and returns a copy where 2 different diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 52f71416363..eb1dc518efe 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -426,12 +426,11 @@ RSpec.describe ApplicationController do end describe "no logspam" do - before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new - end + let(:fake_logger) { FakeLogger.new } - after { Rails.logger = @orig_logger } + before { Rails.logger.broadcast_to(fake_logger) } + + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "should handle 404 to a css file" do Discourse.cache.delete("page_not_found_topics:#{I18n.locale}") @@ -453,9 +452,9 @@ RSpec.describe ApplicationController do expect(response.body).to include(topic1.title) expect(response.body).to_not include(topic2.title) - expect(@fake_logger.fatals.length).to eq(0) - expect(@fake_logger.errors.length).to eq(0) - expect(@fake_logger.warnings.length).to eq(0) + expect(fake_logger.fatals.length).to eq(0) + expect(fake_logger.errors.length).to eq(0) + expect(fake_logger.warnings.length).to eq(0) end end diff --git a/spec/requests/csp_reports_controller_spec.rb b/spec/requests/csp_reports_controller_spec.rb index cd3b429a493..d97fa3e319a 100644 --- a/spec/requests/csp_reports_controller_spec.rb +++ b/spec/requests/csp_reports_controller_spec.rb @@ -2,15 +2,16 @@ RSpec.describe CspReportsController do describe "#create" do + let(:fake_logger) { FakeLogger.new } + before do SiteSetting.content_security_policy = true SiteSetting.content_security_policy_collect_reports = true - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new + Rails.logger.broadcast_to(fake_logger) end - after { Rails.logger = @orig_logger } + after { Rails.logger.stop_broadcasting_to(fake_logger) } def send_report post "/csp_reports", @@ -73,7 +74,7 @@ RSpec.describe CspReportsController do it "logs the violation report" do send_report - expect(@fake_logger.warnings).to include( + expect(fake_logger.warnings).to include( "CSP Violation: 'http://suspicio.us/assets.js' \n\nconsole.log('unsafe')", ) end diff --git a/spec/services/user_stat_count_updater_spec.rb b/spec/services/user_stat_count_updater_spec.rb index 69708dddda6..ce8f4d68160 100644 --- a/spec/services/user_stat_count_updater_spec.rb +++ b/spec/services/user_stat_count_updater_spec.rb @@ -6,32 +6,33 @@ RSpec.describe UserStatCountUpdater do fab!(:post) fab!(:post_2) { Fabricate(:post, topic: post.topic) } + let(:fake_logger) { FakeLogger.new } + before do - @orig_logger = Rails.logger - Rails.logger = @fake_logger = FakeLogger.new + Rails.logger.broadcast_to(fake_logger) SiteSetting.verbose_user_stat_count_logging = true end - after { Rails.logger = @orig_logger } + after { Rails.logger.stop_broadcasting_to(fake_logger) } it "should log the exception when a negative count is inserted" do UserStatCountUpdater.decrement!(post, user_stat: user_stat) - expect(@fake_logger.warnings.last).to match("topic_count") - expect(@fake_logger.warnings.last).to match(post.id.to_s) + expect(fake_logger.warnings.last).to match("topic_count") + expect(fake_logger.warnings.last).to match(post.id.to_s) UserStatCountUpdater.decrement!(post_2, user_stat: user_stat) - expect(@fake_logger.warnings.last).to match("post_count") - expect(@fake_logger.warnings.last).to match(post_2.id.to_s) + expect(fake_logger.warnings.last).to match("post_count") + expect(fake_logger.warnings.last).to match(post_2.id.to_s) end it "should log the exception when a negative count will be inserted but 0 is used instead" do UserStatCountUpdater.set!(user_stat: user_stat, count: -10, count_column: :post_count) - expect(@fake_logger.warnings.last).to match("post_count") - expect(@fake_logger.warnings.last).to match("using 0") - expect(@fake_logger.warnings.last).to match("user #{user_stat.user_id}") + expect(fake_logger.warnings.last).to match("post_count") + expect(fake_logger.warnings.last).to match("using 0") + expect(fake_logger.warnings.last).to match("user #{user_stat.user_id}") expect(user_stat.reload.post_count).to eq(0) end end