DEV: Fix flaky deprecated setting specs (#30550)

…and remove obsolete (and already-disabled) TL-migration related specs

spec/fixtures/site_settings/deprecated_test.yml

@@ -1,13 +1,4 @@
 category1:
-  use_https: true
-  min_trust_level_to_allow_invite:
-    default: 2
-    enum: "TrustLevelSetting"
-    hidden: true
-  min_trust_level_to_allow_invite_tl_and_staff:
-    default: 2
-    enum: "TrustLevelAndStaffSetting"
-    hidden: true
   old_one:
     default: false
   new_one:

spec/lib/site_settings/deprecated_settings_spec.rb

@@ -1,236 +1,53 @@
 # frozen_string_literal: true
 
-RSpec.xdescribe SiteSettings::DeprecatedSettings do
+RSpec.describe SiteSettings::DeprecatedSettings do
-  def deprecate_override!(settings, tl_group_overrides = [])
-    @original_settings = SiteSettings::DeprecatedSettings::SETTINGS.dup
-    SiteSettings::DeprecatedSettings::SETTINGS.clear
-    SiteSettings::DeprecatedSettings::SETTINGS.push(settings)
-
-    if tl_group_overrides.any?
-      @original_override_tl_group = SiteSettings::DeprecatedSettings::OVERRIDE_TL_GROUP_SETTINGS.dup
-      SiteSettings::DeprecatedSettings::OVERRIDE_TL_GROUP_SETTINGS.clear
-      SiteSettings::DeprecatedSettings::OVERRIDE_TL_GROUP_SETTINGS.push(*tl_group_overrides)
-    end
-
-    SiteSetting.setup_deprecated_methods
-  end
-
-  after do
-    if defined?(@original_settings)
-      SiteSettings::DeprecatedSettings::SETTINGS.clear
-      SiteSettings::DeprecatedSettings::SETTINGS.concat(@original_settings)
-    end
-
-    if defined?(@original_override_tl_group)
-      SiteSettings::DeprecatedSettings::OVERRIDE_TL_GROUP_SETTINGS.clear
-      SiteSettings::DeprecatedSettings::OVERRIDE_TL_GROUP_SETTINGS.concat(
-        @original_override_tl_group,
-      )
-    end
-
-    SiteSetting.setup_deprecated_methods
-  end
-
   describe "when not overriding deprecated settings" do
-    let(:override) { false }
+    it "does not act as a proxy to the new methods" do
+      stub_deprecated_settings!(override: false) do
+        SiteSetting.old_one = true
 
-    # NOTE: This fixture has some completely made up settings (e.g. min_trust_level_to_allow_invite_tl_and_staff)
+        expect(SiteSetting.new_one).to eq(false)
-    let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }
+        expect(SiteSetting.new_one?).to eq(false)
-
+      end
-    before do
-      SiteSetting.force_https = true
-      SiteSetting.load_settings(deprecated_test)
     end
 
-    it "should not act as a proxy to the new methods" do
+    it "logs warnings when deprecated settings are called" do
-      deprecate_override!(["use_https", "force_https", override, "0.0.1"])
+      stub_deprecated_settings!(override: false) do
-
-      SiteSetting.use_https = false
-
-      expect(SiteSetting.force_https).to eq(true)
-      expect(SiteSetting.force_https?).to eq(true)
-    end
-
-    it "should log warnings when deprecated settings are called" do
-      deprecate_override!(["use_https", "force_https", override, "0.0.1"])
-
         logger =
           track_log_messages do
-          expect(SiteSetting.use_https).to eq(true)
+            expect(SiteSetting.old_one).to eq(false)
-          expect(SiteSetting.use_https?).to eq(true)
+            expect(SiteSetting.old_one?).to eq(false)
           end
         expect(logger.warnings.count).to eq(3)
 
-      logger = track_log_messages { SiteSetting.use_https(warn: false) }
+        logger = track_log_messages { SiteSetting.old_one(warn: false) }
         expect(logger.warnings.count).to eq(0)
       end
     end
+  end
 
   describe "when overriding deprecated settings" do
-    let(:override) { true }
+    it "acts as a proxy to the new methods" do
-    let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }
+      stub_deprecated_settings!(override: true) do
+        SiteSetting.old_one = true
 
-    before do
+        expect(SiteSetting.new_one).to eq(true)
-      SiteSetting.force_https = true
+        expect(SiteSetting.new_one?).to eq(true)
-      SiteSetting.load_settings(deprecated_test)
+      end
     end
 
-    it "should act as a proxy to the new methods" do
+    it "logs warnings when deprecated settings are called" do
-      deprecate_override!(["use_https", "force_https", override, "0.0.1"])
+      stub_deprecated_settings!(override: true) do
-
-      SiteSetting.use_https = false
-
-      expect(SiteSetting.force_https).to eq(false)
-      expect(SiteSetting.force_https?).to eq(false)
-    end
-
-    xit "should log warnings when deprecated settings are called" do
-      deprecate_override!(["use_https", "force_https", override, "0.0.1"])
-
         logger =
           track_log_messages do
-          expect(SiteSetting.use_https).to eq(true)
+            expect(SiteSetting.old_one).to eq(false)
-          expect(SiteSetting.use_https?).to eq(true)
+            expect(SiteSetting.old_one?).to eq(false)
           end
         expect(logger.warnings.count).to eq(2)
 
-      logger = track_log_messages { SiteSetting.use_https(warn: false) }
+        logger = track_log_messages { SiteSetting.old_one(warn: false) }
         expect(logger.warnings.count).to eq(0)
       end
     end
-
-  describe "when overriding a trust level setting with a group setting" do
-    let(:override) { false }
-    let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }
-
-    before { SiteSetting.load_settings(deprecated_test) }
-
-    context "when getting an old TrustLevelSetting" do
-      before do
-        deprecate_override!(
-          ["min_trust_level_to_allow_invite", "invite_allowed_groups", override, "0.0.1"],
-        )
-      end
-
-      it "uses the minimum trust level from the trust level auto groups in the new group setting" do
-        SiteSetting.invite_allowed_groups =
-          "#{Group::AUTO_GROUPS[:trust_level_3]}|#{Group::AUTO_GROUPS[:trust_level_4]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite).to eq(TrustLevel[3])
-      end
-
-      it "returns TL4 if there are no trust level auto groups in the new group setting" do
-        SiteSetting.invite_allowed_groups = Fabricate(:group).id.to_s
-        expect(SiteSetting.min_trust_level_to_allow_invite).to eq(TrustLevel[4])
-      end
-
-      it "returns TL4 if there are only staff and admin auto groups in the new group setting" do
-        SiteSetting.invite_allowed_groups =
-          "#{Group::AUTO_GROUPS[:admins]}|#{Group::AUTO_GROUPS[:staff]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite).to eq(TrustLevel[4])
-      end
-
-      it "returns TL4 if there are no automated invite_allowed_groups" do
-        SiteSetting.invite_allowed_groups = Fabricate(:group).id.to_s
-        expect(SiteSetting.min_trust_level_to_allow_invite).to eq(TrustLevel[4])
-      end
-    end
-
-    context "when getting an old TrustLevelAndStaffSetting" do
-      before do
-        deprecate_override!(
-          [
-            "min_trust_level_to_allow_invite_tl_and_staff",
-            "invite_allowed_groups",
-            override,
-            "0.0.1",
-          ],
-          ["min_trust_level_to_allow_invite_tl_and_staff"],
-        )
-      end
-
-      it "returns moderator if there is only the moderators auto group in the new group setting" do
-        SiteSetting.invite_allowed_groups = "#{Group::AUTO_GROUPS[:moderators]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("moderator")
-      end
-
-      it "returns staff if there are staff and admin auto groups in the new group setting" do
-        SiteSetting.invite_allowed_groups =
-          "#{Group::AUTO_GROUPS[:admins]}|#{Group::AUTO_GROUPS[:staff]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("staff")
-      end
-
-      it "returns admin if there is only the admin auto group in the new group setting" do
-        SiteSetting.invite_allowed_groups = "#{Group::AUTO_GROUPS[:admins]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("admin")
-      end
-
-      it "returns the min trust level if the admin auto group as well as lower TL auto groups in the new group setting" do
-        SiteSetting.invite_allowed_groups =
-          "#{Group::AUTO_GROUPS[:admins]}|#{Group::AUTO_GROUPS[:trust_level_3]}"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq(TrustLevel[3])
-      end
-
-      it "returns admin if there are no automated invite_allowed_groups" do
-        SiteSetting.invite_allowed_groups = Fabricate(:group).id.to_s
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("admin")
-      end
-    end
-
-    context "when setting an old TrustLevelSetting" do
-      before do
-        deprecate_override!(
-          ["min_trust_level_to_allow_invite", "invite_allowed_groups", override, "0.0.1"],
-          ["min_trust_level_to_allow_invite"],
-        )
-      end
-
-      it "converts the provided trust level to the appropriate auto group" do
-        SiteSetting.min_trust_level_to_allow_invite = TrustLevel[4]
-        expect(SiteSetting.min_trust_level_to_allow_invite).to eq(TrustLevel[4])
-        expect(SiteSetting.invite_allowed_groups).to eq(Group::AUTO_GROUPS[:trust_level_4].to_s)
-      end
-
-      it "raises error with an invalid trust level" do
-        expect { SiteSetting.min_trust_level_to_allow_invite = 66 }.to raise_error(
-          Discourse::InvalidParameters,
-        )
-      end
-    end
-
-    context "when setting an old TrustLevelAndStaffSetting" do
-      before do
-        deprecate_override!(
-          [
-            "min_trust_level_to_allow_invite_tl_and_staff",
-            "invite_allowed_groups",
-            override,
-            "0.0.1",
-          ],
-          ["min_trust_level_to_allow_invite_tl_and_staff"],
-        )
-      end
-
-      it "converts the provided trust level to the appropriate auto group" do
-        SiteSetting.min_trust_level_to_allow_invite_tl_and_staff = "admin"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("admin")
-        expect(SiteSetting.invite_allowed_groups).to eq(Group::AUTO_GROUPS[:admins].to_s)
-
-        SiteSetting.min_trust_level_to_allow_invite_tl_and_staff = "staff"
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq("staff")
-        expect(SiteSetting.invite_allowed_groups).to eq(Group::AUTO_GROUPS[:staff].to_s)
-
-        SiteSetting.min_trust_level_to_allow_invite_tl_and_staff = TrustLevel[3]
-        expect(SiteSetting.min_trust_level_to_allow_invite_tl_and_staff).to eq(TrustLevel[3])
-        expect(SiteSetting.invite_allowed_groups).to eq(
-          "#{Group::AUTO_GROUPS[:admins]}|#{Group::AUTO_GROUPS[:staff]}|#{Group::AUTO_GROUPS[:trust_level_3]}",
-        )
-      end
-
-      it "raises error with an invalid trust level" do
-        expect { SiteSetting.min_trust_level_to_allow_invite_tl_and_staff = 66 }.to raise_error(
-          Discourse::InvalidParameters,
-        )
-      end
-    end
   end
 end

spec/models/trust_level_setting_spec.rb

@@ -14,33 +14,4 @@ RSpec.describe TrustLevelSetting do
       expect(value[:value]).to eq(0)
     end
   end
-
-  xdescribe ".valid_value?" do
-    let(:deprecated_test) { "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml" }
-
-    before { SiteSetting.load_settings(deprecated_test) }
-
-    it "allows all trust levels as valid values" do
-      expect(TrustLevelSetting.valid_value?(TrustLevel[0])).to eq(true)
-      expect(TrustLevelSetting.valid_value?(TrustLevel[1])).to eq(true)
-      expect(TrustLevelSetting.valid_value?(TrustLevel[2])).to eq(true)
-      expect(TrustLevelSetting.valid_value?(TrustLevel[3])).to eq(true)
-      expect(TrustLevelSetting.valid_value?(TrustLevel[4])).to eq(true)
-      expect(TrustLevelSetting.valid_value?(20)).to eq(false)
-    end
-
-    it "does not allow 'admin' or 'staff' as valid values" do
-      expect(TrustLevelSetting.valid_value?("admin")).to eq(false)
-      expect(TrustLevelSetting.valid_value?("staff")).to eq(false)
-    end
-
-    it "does not allow setting 'admin' or 'staff' as valid values" do
-      expect { SiteSetting.min_trust_level_to_allow_invite = "admin" }.to raise_error(
-        Discourse::InvalidParameters,
-      )
-      expect { SiteSetting.min_trust_level_to_allow_invite = "staff" }.to raise_error(
-        Discourse::InvalidParameters,
-      )
-    end
-  end
 end

spec/rails_helper.rb

@@ -975,6 +975,23 @@ def has_trigger?(trigger_name)
   SQL
 end
 
+def stub_deprecated_settings!(override:)
+  SiteSetting.load_settings("#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml")
+
+  stub_const(
+    SiteSettings::DeprecatedSettings,
+    "SETTINGS",
+    [["old_one", "new_one", override, "0.0.1"]],
+  ) do
+    SiteSetting.setup_deprecated_methods
+    yield
+  end
+
+  defaults = SiteSetting.defaults.instance_variable_get(:@defaults)
+  defaults.each { |_, hash| hash.delete(:old_one) }
+  defaults.each { |_, hash| hash.delete(:new_one) }
+end
+
 def silence_stdout
   STDOUT.stubs(:write)
   yield

spec/requests/admin/site_settings_controller_spec.rb

@@ -238,14 +238,7 @@ RSpec.describe Admin::SiteSettingsController do
       end
 
       it "works for deprecated settings" do
-        deprecated_test = "#{Rails.root}/spec/fixtures/site_settings/deprecated_test.yml"
+        stub_deprecated_settings!(override: true) do
-        SiteSetting.load_settings(deprecated_test)
-
-        stub_const(
-          SiteSettings::DeprecatedSettings,
-          "SETTINGS",
-          [["old_one", "new_one", true, "3.0"]],
-        ) do
           put "/admin/site_settings/old_one.json", params: { old_one: true }
 
           expect(response.status).to eq(200)