DEV: Call Discourse.redis.flushdb after the end of each test (#29117)

There have been too many flaky tests as a result of leaking state in Redis so it is easier to resolve them by ensuring we flush Redis' database. Locally on my machine, calling `Discourse.redis.flushdb` takes around 0.1ms which means this change will have very little impact on test runtimes.
2024-11-22 15:06:26 +08:00 · 2024-10-09 07:19:31 +08:00 · 2024-10-09 07:19:31 +08:00 · ed6c9d1545
commit ed6c9d1545
parent 44fe8c62d6
38 changed files with 10 additions and 154 deletions
--- a/lib/redis_snapshot.rb
+++ b/lib/redis_snapshot.rb
@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-class RedisSnapshot
-  def self.begin_faux_transaction(redis = Discourse.redis)
-    @stack ||= []
-    @stack.push(RedisSnapshot.load(redis))
-  end
-
-  def self.end_faux_transaction(redis = Discourse.redis)
-    @stack.pop.restore(redis)
-  end
-
-  def self.load(redis = Discourse.redis)
-    keys = redis.keys
-
-    values = redis.pipelined { |batch| keys.each { |key| batch.dump(key) } }
-
-    new(keys.zip(values))
-  end
-
-  def initialize(dump)
-    @dump = dump
-  end
-
-  def restore(redis = Discourse.redis)
-    redis.pipelined do |batch|
-      batch.flushdb
-
-      @dump.each { |key, value| batch.restore(key, 0, value) }
-    end
-
-    nil
-  end
-end
--- a/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb
+++ b/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb
@ -3,14 +3,16 @@
 describe Jobs::Chat::ProcessMessage do
  fab!(:chat_message) { Fabricate(:chat_message, message: "https://discourse.org/team") }

-  it "updates cooked with oneboxes" do
+  before do
    stub_request(:get, "https://discourse.org/team").to_return(
      status: 200,
      body: "<html><head><title>a</title></head></html>",
    )

    stub_request(:head, "https://discourse.org/team").to_return(status: 200)
+  end

+  it "updates cooked with oneboxes" do
    described_class.new.execute(chat_message_id: chat_message.id)
    expect(chat_message.reload.cooked).to eq(
      "<p><a href=\"https://discourse.org/team\" class=\"onebox\" target=\"_blank\" rel=\"noopener nofollow ugc\">https://discourse.org/team</a></p>",
--- a/plugins/chat/spec/requests/chat/incoming_webhooks_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/incoming_webhooks_controller_spec.rb
@ -72,8 +72,6 @@ RSpec.describe Chat::IncomingWebhooksController do
    end

    describe "rate limiting" do
-      use_redis_snapshotting
-
      it "rate limits" do
        RateLimiter.enable
        10.times { post "/chat/hooks/#{webhook.key}.json", params: valid_payload }
--- a/spec/helpers/redis_snapshot_helper.rb
+++ b/spec/helpers/redis_snapshot_helper.rb
@ -2,8 +2,6 @@

 module RedisSnapshotHelper
  def use_redis_snapshotting
-    before(:each) { RedisSnapshot.begin_faux_transaction }
-
-    after(:each) { RedisSnapshot.end_faux_transaction }
+    puts "DEPRECATION NOTICE: `use_redis_snapshotting` has been deprecated without replacement as we now flush the Redis database after each test."
  end
 end
--- a/spec/integration/rate_limiting_spec.rb
+++ b/spec/integration/rate_limiting_spec.rb
@ -4,8 +4,6 @@
 RSpec.describe "rate limiter integration" do
  before { RateLimiter.enable }

-  use_redis_snapshotting
-
  it "will rate limit message bus requests once queueing" do
    freeze_time

--- a/spec/jobs/jobs_base_spec.rb
+++ b/spec/jobs/jobs_base_spec.rb
@ -1,8 +1,6 @@
 # frozen_string_literal: true

 RSpec.describe ::Jobs::Base do
-  use_redis_snapshotting
-
  class GoodJob < ::Jobs::Base
    attr_accessor :count
    def execute(args)
--- a/spec/jobs/update_topic_hot_scores_spec.rb
+++ b/spec/jobs/update_topic_hot_scores_spec.rb
@ -3,7 +3,6 @@
 require "file_store/s3_store"

 RSpec.describe Jobs::UpdateTopicHotScores do
-  use_redis_snapshotting
  let(:job) { subject }

  fab!(:topic) { Fabricate(:topic, created_at: 1.day.ago) }
--- a/spec/lib/auth/default_current_user_provider_spec.rb
+++ b/spec/lib/auth/default_current_user_provider_spec.rb
@ -199,8 +199,6 @@ RSpec.describe Auth::DefaultCurrentUserProvider do
    context "with rate limiting" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      it "rate limits admin api requests" do
        global_setting :max_admin_api_reqs_per_minute, 3

--- a/spec/lib/middleware/anonymous_cache_spec.rb
+++ b/spec/lib/middleware/anonymous_cache_spec.rb
@ -230,8 +230,6 @@ RSpec.describe Middleware::AnonymousCache do
  describe "#force_anonymous!" do
    before { RateLimiter.enable }

-    use_redis_snapshotting
-
    it "will revert to anonymous once we reach the limit" do
      is_anon = false

--- a/spec/lib/middleware/request_tracker_spec.rb
+++ b/spec/lib/middleware/request_tracker_spec.rb
@ -258,8 +258,6 @@ RSpec.describe Middleware::RequestTracker do
        )
      end

-      use_redis_snapshotting
-
      def log_topic_view(authenticated: false, deferred: false)
        headers = { "action_dispatch.remote_ip" => "127.0.0.1" }

@ -519,8 +517,6 @@ RSpec.describe Middleware::RequestTracker do
      freeze_time_safe
    end

-    use_redis_snapshotting
-
    after { Rails.logger = @orig_logger }

    let :middleware do
--- a/spec/lib/mini_scheduler_long_running_job_logger_spec.rb
+++ b/spec/lib/mini_scheduler_long_running_job_logger_spec.rb
@ -3,8 +3,6 @@
 require "mini_scheduler_long_running_job_logger"

 RSpec.describe MiniSchedulerLongRunningJobLogger do
-  use_redis_snapshotting
-
  class Every10MinutesJob
    extend ::MiniScheduler::Schedule

--- a/spec/lib/post_action_creator_spec.rb
+++ b/spec/lib/post_action_creator_spec.rb
@ -9,8 +9,6 @@ RSpec.describe PostActionCreator do
  describe "rate limits" do
    before { RateLimiter.enable }

-    use_redis_snapshotting
-
    it "limits redo/undo" do
      PostActionCreator.like(user, post)
      PostActionDestroyer.destroy(user, post, :like)
--- a/spec/lib/post_revisor_spec.rb
+++ b/spec/lib/post_revisor_spec.rb
@ -306,10 +306,6 @@ RSpec.describe PostRevisor do
  end

  describe "revise wiki" do
-    # There used to be a bug where wiki changes were considered posting "too similar"
-    # so this is enabled and checked
-    use_redis_snapshotting
-
    before { SiteSetting.unique_posts_mins = 10 }

    it "allows the user to change it to a wiki" do
@ -803,8 +799,6 @@ RSpec.describe PostRevisor do
        SiteSetting.editing_grace_period = 0
      end

-      use_redis_snapshotting
-
      it "triggers a rate limiter" do
        EditRateLimiter.any_instance.expects(:performed!)
        post_revisor.revise!(changed_by, raw: "updated body")
--- a/spec/lib/site_settings/deprecated_settings_spec.rb
+++ b/spec/lib/site_settings/deprecated_settings_spec.rb
@ -34,10 +34,6 @@ RSpec.xdescribe SiteSettings::DeprecatedSettings do
  describe "when not overriding deprecated settings" do
    let(:override) { false }

-    # Necessary because Discourse.deprecate uses redis to see if the warning
-    # was already logged.
-    use_redis_snapshotting
-
    # NOTE: This fixture has some completely made up settings (e.g. min_trust_level_to_allow_invite_tl_and_staff)
    let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }

--- a/spec/lib/topic_query_spec.rb
+++ b/spec/lib/topic_query_spec.rb
@ -1489,8 +1489,6 @@ RSpec.describe TopicQuery do
  end

  describe "#list_suggested_for" do
-    use_redis_snapshotting
-
    def clear_cache!
      Discourse.redis.keys("random_topic_cache*").each { |k| Discourse.redis.del k }
    end
--- a/spec/models/about_spec.rb
+++ b/spec/models/about_spec.rb
@ -15,8 +15,6 @@ RSpec.describe About do
  after { DiscoursePluginRegistry.reset! }

  describe "#stats" do
-    use_redis_snapshotting
-
    it "adds plugin stats to the output" do
      stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 }
      register_stat("some_group", Proc.new { stats })
--- a/spec/models/global_setting_spec.rb
+++ b/spec/models/global_setting_spec.rb
@ -55,7 +55,10 @@ RSpec.describe GlobalSetting do
        Discourse.stubs(:redis).returns(nil)
      end

-      after { GlobalSetting.skip_redis = false }
+      after do
+        GlobalSetting.skip_redis = false
+        Discourse.unstub(:redis)
+      end

      it "generates a new random key in memory without redis" do
        GlobalSetting.reset_secret_key_base!
--- a/spec/models/invite_spec.rb
+++ b/spec/models/invite_spec.rb
@ -221,8 +221,6 @@ RSpec.describe Invite do
          3.times { Invite.generate(user, email: "test@example.com") }
        end

-        use_redis_snapshotting
-
        it "raises an error" do
          expect { Invite.generate(user, email: "test@example.com") }.to raise_error(
            RateLimiter::LimitExceeded,
--- a/spec/models/post_action_spec.rb
+++ b/spec/models/post_action_spec.rb
@ -439,7 +439,6 @@ RSpec.describe PostAction do

      PostActionType.types.each do |type_name, type_id|
        post = Fabricate(:post)
-
        PostActionCreator.create(codinghorror, post, type_name)
        actual_count = value_for(codinghorror.id, Date.today)
        expected_count = type_name == :like ? 2 : 1
@ -798,7 +797,7 @@ RSpec.describe PostAction do
      expect(result).to be_success
      expect(result.post_action.related_post_id).to be_nil
      expect(result.reviewable_score.meta_topic_id).to be_nil
-
+    ensure
      flag_without_message.destroy!
    end

--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@ -816,8 +816,6 @@ RSpec.describe Topic do
    context "with rate limits" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      context "when per day" do
        before { SiteSetting.max_topic_invitations_per_day = 1 }

@ -2698,8 +2696,6 @@ RSpec.describe Topic do
      RateLimiter.enable
    end

-    use_redis_snapshotting
-
    it "limits new users to max_topics_in_first_day and max_posts_in_first_day" do
      start = Time.now.tomorrow.beginning_of_day

@ -2751,8 +2747,6 @@ RSpec.describe Topic do
      RateLimiter.enable
    end

-    use_redis_snapshotting
-
    it "limits according to max_personal_messages_per_day" do
      create_post(
        user: user,
--- a/spec/multisite/flags_spec.rb
+++ b/spec/multisite/flags_spec.rb
@ -2,8 +2,6 @@

 RSpec.describe "Custom flags in multisite", type: :multisite do
  describe "PostACtionType#all_flags" do
-    use_redis_snapshotting
-
    it "does not share flag definitions between sites" do
      flag_1 = Flag.create!(name: "test flag 1", position: 99, applies_to: ["Post"])

--- a/spec/multisite/request_tracker_spec.rb
+++ b/spec/multisite/request_tracker_spec.rb
@ -8,8 +8,6 @@ RSpec.describe "RequestTracker in multisite", type: :multisite do
    RateLimiter.clear_all_global!
  end

-  use_redis_snapshotting
-
  def call(env, &block)
    Middleware::RequestTracker.new(block).call(env)
  end
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@ -716,7 +716,6 @@ RSpec.configure do |config|

    Capybara.reset_session!
    MessageBus.backend_instance.reset! # Clears all existing backlog from memory backend
-    Discourse.redis.flushdb
  end

  config.after :each do |example|
@ -743,6 +742,7 @@ RSpec.configure do |config|

    unfreeze_time
    ActionMailer::Base.deliveries.clear
+    Discourse.redis.flushdb
  end

  config.before(:each, type: :multisite) do
--- a/spec/requests/admin/backups_controller_spec.rb
+++ b/spec/requests/admin/backups_controller_spec.rb
@ -156,8 +156,6 @@ RSpec.describe Admin::BackupsController do
      context "with rate limiting enabled" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        after { RateLimiter.disable }

        it "is rate limited" do
--- a/spec/requests/api/rate_limits_spec.rb
+++ b/spec/requests/api/rate_limits_spec.rb
@ -39,8 +39,6 @@ RSpec.describe "rate limits" do
  let(:api_key) { @key }
  let!(:api_username) { "system" }

-  use_redis_snapshotting
-
  around(:each) { |example| stub_const(Object, :RateLimiter, MockRateLimiter) { example.run } }

  it "doesn't rate limit authenticated admin api requests" do
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@ -932,8 +932,6 @@ RSpec.describe ApplicationController do
    context "with rate limits" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      it "serves a LimitExceeded error in the preferred locale" do
        SiteSetting.max_likes_per_day = 1
        post1 = Fabricate(:post)
@ -1156,8 +1154,6 @@ RSpec.describe ApplicationController do

    before { RateLimiter.enable }

-    use_redis_snapshotting
-
    it "is included when API key is rate limited" do
      global_setting :max_admin_api_reqs_per_minute, 1
      api_key = ApiKey.create!(user_id: admin.id).key
@ -1208,8 +1204,6 @@ RSpec.describe ApplicationController do
      SiteSetting.slow_down_crawler_user_agents = "badcrawler|problematiccrawler"
    end

-    use_redis_snapshotting
-
    it "are rate limited" do
      now = Time.zone.now
      freeze_time now
--- a/spec/requests/bookmarks_controller_spec.rb
+++ b/spec/requests/bookmarks_controller_spec.rb
@ -11,8 +11,6 @@ RSpec.describe BookmarksController do
  describe "#create" do
    before { sign_in(current_user) }

-    use_redis_snapshotting
-
    it "rate limits creates" do
      SiteSetting.max_bookmarks_per_day = 1
      RateLimiter.enable
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@ -2870,8 +2870,6 @@ RSpec.describe GroupsController do
      end

      context "when rate limited" do
-        use_redis_snapshotting
-
        it "rate limits anon searches per user" do
          RateLimiter.enable

--- a/spec/requests/invites_controller_spec.rb
+++ b/spec/requests/invites_controller_spec.rb
@ -771,8 +771,6 @@ RSpec.describe InvitesController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "can send invite email" do
          sign_in(user)

@ -1598,8 +1596,6 @@ RSpec.describe InvitesController do
      RateLimiter.enable
    end

-    use_redis_snapshotting
-
    it "resends all non-redeemed invites by a user" do
      freeze_time

--- a/spec/requests/omniauth_callbacks_controller_spec.rb
+++ b/spec/requests/omniauth_callbacks_controller_spec.rb
@ -163,8 +163,6 @@ RSpec.describe Users::OmniauthCallbacksController do
    end

    context "when in readonly mode" do
-      use_redis_snapshotting
-
      it "should return a 503" do
        Discourse.enable_readonly_mode

@ -174,8 +172,6 @@ RSpec.describe Users::OmniauthCallbacksController do
    end

    context "when in staff writes only mode" do
-      use_redis_snapshotting
-
      before { Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) }

      it "returns a 503 for non-staff" do
--- a/spec/requests/presence_controller_spec.rb
+++ b/spec/requests/presence_controller_spec.rb
@ -45,8 +45,6 @@ RSpec.describe PresenceController do

  describe "#update" do
    context "in readonly mode" do
-      use_redis_snapshotting
-
      before { Discourse.enable_readonly_mode }

      it "produces 503" do
--- a/spec/requests/search_controller_spec.rb
+++ b/spec/requests/search_controller_spec.rb
@ -230,8 +230,6 @@ RSpec.describe SearchController do
    context "when rate limited" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      def unlimited_request(ip_address = "1.2.3.4")
        get "/search/query.json", params: { term: "wookie" }, env: { REMOTE_ADDR: ip_address }

@ -422,8 +420,6 @@ RSpec.describe SearchController do
    context "when rate limited" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      def unlimited_request(ip_address = "1.2.3.4")
        get "/search.json", params: { q: "wookie" }, env: { REMOTE_ADDR: ip_address }

--- a/spec/requests/session_controller_spec.rb
+++ b/spec/requests/session_controller_spec.rb
@ -136,8 +136,6 @@ RSpec.describe SessionController do
    before { SiteSetting.enable_local_logins_via_email = true }

    context "when in staff writes only mode" do
-      use_redis_snapshotting
-
      before { Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) }

      it "allows admins to login" do
@ -580,8 +578,6 @@ RSpec.describe SessionController do
    end

    context "when in staff writes only mode" do
-      use_redis_snapshotting
-
      before { Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) }

      it "allows staff to login" do
@ -1366,8 +1362,6 @@ RSpec.describe SessionController do
    end

    context "when in readonly mode" do
-      use_redis_snapshotting
-
      before { Discourse.enable_readonly_mode }

      it "disallows requests" do
@ -1840,8 +1834,6 @@ RSpec.describe SessionController do

  describe "#create" do
    context "when read only mode" do
-      use_redis_snapshotting
-
      before do
        Discourse.enable_readonly_mode
        EmailToken.confirm(email_token.token)
@ -1860,8 +1852,6 @@ RSpec.describe SessionController do
    end

    context "when in staff writes only mode" do
-      use_redis_snapshotting
-
      before do
        Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY)
        EmailToken.confirm(email_token.token)
@ -2045,8 +2035,6 @@ RSpec.describe SessionController do

        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "should return an error response code with the right error message" do
          post "/session.json", params: { login: user.username, password: "myawesomepassword" }

@ -2445,8 +2433,6 @@ RSpec.describe SessionController do
    context "when rate limited" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      it "rate limits login" do
        SiteSetting.max_logins_per_ip_per_hour = 2
        EmailToken.confirm(email_token.token)
@ -2737,8 +2723,6 @@ RSpec.describe SessionController do
    describe "rate limiting" do
      before { RateLimiter.enable }

-      use_redis_snapshotting
-
      it "should correctly rate limits" do
        user = Fabricate(:user)

--- a/spec/requests/slugs_controller_spec.rb
+++ b/spec/requests/slugs_controller_spec.rb
@ -30,8 +30,6 @@ RSpec.describe SlugsController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits" do
          stub_const(SlugsController, "MAX_SLUG_GENERATIONS_PER_MINUTE", 1) do
            post "/slugs.json?name=#{name}"
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@ -2249,8 +2249,6 @@ RSpec.describe TopicsController do
  end

  describe "#show" do
-    use_redis_snapshotting
-
    fab!(:private_topic) { pm }
    fab!(:topic) { Fabricate(:post, user: post_author1).topic }

--- a/spec/requests/uploads_controller_spec.rb
+++ b/spec/requests/uploads_controller_spec.rb
@ -23,8 +23,6 @@ RSpec.describe UploadsController do
      context "when rate limited" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "should return 429 response code when maximum number of uploads per minute has been exceeded for a user" do
          SiteSetting.max_uploads_per_minute = 1

@ -913,8 +911,6 @@ RSpec.describe UploadsController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits" do
          SiteSetting.max_presigned_put_per_minute = 1

@ -1085,8 +1081,6 @@ RSpec.describe UploadsController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits" do
          SiteSetting.max_create_multipart_per_minute = 1

@ -1280,8 +1274,6 @@ RSpec.describe UploadsController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits" do
          SiteSetting.max_batch_presign_multipart_per_minute = 1

@ -1507,8 +1499,6 @@ RSpec.describe UploadsController do
      describe "rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits" do
          SiteSetting.max_complete_multipart_per_minute = 1

--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@ -380,8 +380,6 @@ RSpec.describe UsersController do
      context "with rate limiting" do
        before { RateLimiter.enable }

-        use_redis_snapshotting
-
        it "rate limits reset passwords" do
          freeze_time

@ -4410,8 +4408,6 @@ RSpec.describe UsersController do
    end

    context "with a session variable" do
-      use_redis_snapshotting
-
      it "raises an error with an invalid session value" do
        post_user

@ -5625,8 +5621,6 @@ RSpec.describe UsersController do
  describe "#enable_second_factor_totp" do
    before { sign_in(user1) }

-    use_redis_snapshotting
-
    def create_totp
      stub_secure_session_confirmed
      post "/users/create_second_factor_totp.json"
--- a/spec/serializers/topic_view_serializer_spec.rb
+++ b/spec/serializers/topic_view_serializer_spec.rb
@ -10,9 +10,6 @@ RSpec.describe TopicViewSerializer do
    JSON.parse(MultiJson.dump(serializer)).deep_symbolize_keys!
  end

-  # Ensure no suggested ids are cached cause that can muck up suggested
-  use_redis_snapshotting
-
  fab!(:topic)
  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:user_2) { Fabricate(:user) }