DEV: Don't user before(:all)/after(:all) (#13389)

DEV: Don’t user before(:all)/after(:all) (#13389)

Leaking state and non-obvious order (before :all runs before RailsHelper.test_setup) are not worth it. A replacement PR for #13370. Fixes some flaky specs, e.g.

bin/rspec './spec/components/freedom_patches/translate_accelerator_spec.rb[1:3]' './spec/jobs/clean_up_user_export_topics_spec.rb[1:1]' --tag ~type:multisite --seed 35994

Also included:

  • DEV: No need for locale reset (we do it anyway in rails_helper in test_setup)
diff --git a/spec/components/freedom_patches/translate_accelerator_spec.rb b/spec/components/freedom_patches/translate_accelerator_spec.rb
index cd52c79..1039732 100644
--- a/spec/components/freedom_patches/translate_accelerator_spec.rb
+++ b/spec/components/freedom_patches/translate_accelerator_spec.rb
@@ -3,18 +3,14 @@
 require "rails_helper"
 
 describe "translate accelerator" do
-  before(:all) do
+  before do
     @original_i18n_load_path = I18n.load_path.dup
     I18n.load_path += Dir["#{Rails.root}/spec/fixtures/i18n/translate_accelerator.*.yml"]
     I18n.reload!
   end
 
-  after(:all) do
-    I18n.load_path = @original_i18n_load_path
-    I18n.reload!
-  end
-
   after do
+    I18n.load_path = @original_i18n_load_path
     I18n.reload!
   end
 
diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb
index ae4ee4a..0d53880 100644
--- a/spec/components/pretty_text_spec.rb
+++ b/spec/components/pretty_text_spec.rb
@@ -1319,7 +1319,7 @@ HTML
   end
 
   describe "censoring" do
-    after(:all) { Discourse.redis.flushdb }
+    after { Discourse.redis.flushdb }
 
     def expect_cooked_match(raw, expected_cooked)
       expect(PrettyText.cook(raw)).to eq(expected_cooked)
@@ -1404,7 +1404,7 @@ HTML
   end
 
   describe "watched words - replace & link" do
-    after(:all) { Discourse.redis.flushdb }
+    after { Discourse.redis.flushdb }
 
     it "replaces words with other words" do
       Fabricate(:watched_word, action: WatchedWord.actions[:replace], word: "dolor sit*", replacement: "something else")
diff --git a/spec/components/theme_settings_parser_spec.rb b/spec/components/theme_settings_parser_spec.rb
index e65169b..e24ecd2 100644
--- a/spec/components/theme_settings_parser_spec.rb
+++ b/spec/components/theme_settings_parser_spec.rb
@@ -4,7 +4,7 @@ require 'rails_helper'
 require 'theme_settings_parser'
 
 describe ThemeSettingsParser do
-  after(:all) do
+  after do
     ThemeField.destroy_all
   end
 
diff --git a/spec/jobs/vacate_legacy_prefix_backups_spec.rb b/spec/jobs/vacate_legacy_prefix_backups_spec.rb
index 4de6325..45eb0df 100644
--- a/spec/jobs/vacate_legacy_prefix_backups_spec.rb
+++ b/spec/jobs/vacate_legacy_prefix_backups_spec.rb
@@ -1,12 +1,12 @@
 # frozen_string_literal: true
 
 require "s3_helper"
-require 'rails_helper'
+require "rails_helper"
 
 describe Jobs::VacateLegacyPrefixBackups, type: :multisite do
   let(:bucket_name) { "backupbucket" }
 
-  before(:all) do
+  before do
     @s3_client = Aws::S3::Client.new(stub_responses: true)
     @s3_options = { client: @s3_client }
     @objects = []
@@ -15,9 +15,7 @@ describe Jobs::VacateLegacyPrefixBackups, type: :multisite do
     @s3_client.stub_responses(:list_objects_v2, -> (context) do
       { contents: objects_with_prefix(context) }
     end)
-  end
 
-  before do
     setup_s3
     SiteSetting.s3_backup_bucket = bucket_name
     SiteSetting.backup_location = BackupLocationSiteSetting::S3
diff --git a/spec/lib/backup_restore/local_backup_store_spec.rb b/spec/lib/backup_restore/local_backup_store_spec.rb
index 50e1a36..5247daf 100644
--- a/spec/lib/backup_restore/local_backup_store_spec.rb
+++ b/spec/lib/backup_restore/local_backup_store_spec.rb
@@ -5,19 +5,16 @@ require 'backup_restore/local_backup_store'
 require_relative 'shared_examples_for_backup_store'
 
 describe BackupRestore::LocalBackupStore do
-  before(:all) do
+  before do
     @root_directory = Dir.mktmpdir
     @paths = []
+    SiteSetting.backup_location = BackupLocationSiteSetting::LOCAL
   end
 
-  after(:all) do
+  after do
     FileUtils.remove_dir(@root_directory, true)
   end
 
-  before do
-    SiteSetting.backup_location = BackupLocationSiteSetting::LOCAL
-  end
-
   subject(:store) { BackupRestore::BackupStore.create(root_directory: @root_directory) }
   let(:expected_type) { BackupRestore::LocalBackupStore }
 
diff --git a/spec/lib/backup_restore/s3_backup_store_spec.rb b/spec/lib/backup_restore/s3_backup_store_spec.rb
index 5f6745f..c4120a3 100644
--- a/spec/lib/backup_restore/s3_backup_store_spec.rb
+++ b/spec/lib/backup_restore/s3_backup_store_spec.rb
@@ -6,7 +6,7 @@ require 'backup_restore/s3_backup_store'
 require_relative 'shared_examples_for_backup_store'
 
 describe BackupRestore::S3BackupStore do
-  before(:all) do
+  before do
     @s3_client = Aws::S3::Client.new(stub_responses: true)
     @s3_options = { client: @s3_client }
 
@@ -65,9 +65,7 @@ describe BackupRestore::S3BackupStore do
         last_modified: Time.zone.now
       }
     end)
-  end
 
-  before do
     SiteSetting.s3_backup_bucket = "s3-backup-bucket"
     SiteSetting.s3_access_key_id = "s3-access-key-id"
     SiteSetting.s3_secret_access_key = "s3-secret-access-key"
@@ -82,7 +80,7 @@ describe BackupRestore::S3BackupStore do
 
   context "S3 specific behavior" do
     before { create_backups }
-    after(:all) { remove_backups }
+    after { remove_backups }
 
     describe "#delete_old" do
       it "doesn't delete files when cleanup is disabled" do
diff --git a/spec/lib/backup_restore/shared_examples_for_backup_store.rb b/spec/lib/backup_restore/shared_examples_for_backup_store.rb
index 344fb17..679fd63 100644
--- a/spec/lib/backup_restore/shared_examples_for_backup_store.rb
+++ b/spec/lib/backup_restore/shared_examples_for_backup_store.rb
@@ -2,7 +2,7 @@
 
 shared_context "backups" do
   before { create_backups }
-  after(:all) { remove_backups }
+  after { remove_backups }
 
   # default backup files
   let(:backup1) { BackupFile.new(filename: "b.tar.gz", size: 17, last_modified: Time.parse("2018-09-13T15:10:00Z")) }
diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb
index b57a87d..47462f3 100644
--- a/spec/models/theme_field_spec.rb
+++ b/spec/models/theme_field_spec.rb
@@ -4,14 +4,10 @@
 require 'rails_helper'
 
 describe ThemeField do
-  after(:all) do
+  after do
     ThemeField.destroy_all
   end
 
-  before do
-    I18n.locale = :en
-  end
-
   describe "scope: find_by_theme_ids" do
     it "returns result in the specified order" do
       theme = Fabricate(:theme)
diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb
index 5b681a0..6d8f12d 100644
--- a/spec/models/theme_spec.rb
+++ b/spec/models/theme_spec.rb
@@ -7,10 +7,6 @@ describe Theme do
     Theme.clear_cache!
   end
 
-  before do
-    I18n.locale = :en
-  end
-
   fab! :user do
     Fabricate(:user)
   end
@@ -21,6 +17,7 @@ describe Theme do
 
   let(:theme) { Fabricate(:theme, user: user) }
   let(:child) { Fabricate(:theme, user: user, component: true) }
+
   it 'can properly clean up color schemes' do
     scheme = ColorScheme.create!(theme_id: theme.id, name: 'test')
     scheme2 = ColorScheme.create!(theme_id: theme.id, name: 'test2')

GitHub sha: e36377d9ab0cb1fcfff844c906fd564b54a70a4d

1 Like

This commit appears in #13389 which was approved by eviltrout. It was merged by CvX.