mirror of
https://github.com/discourse/discourse.git
synced 2025-06-01 02:04:53 +08:00
DEV: Update the rubocop-discourse gem
This enables cops related to RSpec `subject`. See https://github.com/discourse/rubocop-discourse/pull/32
This commit is contained in:

committed by
Loïc Guitaut

parent
8e1d049e6b
commit
0f4beab0fb
@ -3,12 +3,12 @@
|
||||
require_relative "shared_context_for_backup_restore"
|
||||
|
||||
RSpec.describe BackupRestore::DatabaseRestorer, type: :multisite do
|
||||
subject(:restorer) { BackupRestore::DatabaseRestorer.new(logger, current_db) }
|
||||
|
||||
include_context "with shared stuff"
|
||||
|
||||
let(:current_db) { RailsMultisite::ConnectionManagement.current_db }
|
||||
|
||||
subject { BackupRestore::DatabaseRestorer.new(logger, current_db) }
|
||||
|
||||
describe "#restore" do
|
||||
context "with database connection" do
|
||||
it "reconnects to the correct database" do
|
||||
|
@ -3,12 +3,12 @@
|
||||
require_relative "shared_context_for_backup_restore"
|
||||
|
||||
RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
subject(:restorer) { BackupRestore::DatabaseRestorer.new(logger, current_db) }
|
||||
|
||||
include_context "with shared stuff"
|
||||
|
||||
let(:current_db) { RailsMultisite::ConnectionManagement.current_db }
|
||||
|
||||
subject { BackupRestore::DatabaseRestorer.new(logger, current_db) }
|
||||
|
||||
describe "#restore" do
|
||||
it "executes everything in the correct order" do
|
||||
restore = sequence("restore")
|
||||
@ -18,7 +18,7 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
expect_db_migrate.in_sequence(restore)
|
||||
expect_db_reconnect.in_sequence(restore)
|
||||
|
||||
subject.restore("foo.sql")
|
||||
restorer.restore("foo.sql")
|
||||
end
|
||||
|
||||
it "stores the date of the last restore" do
|
||||
@ -146,12 +146,12 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
it "moves tables back when tables were moved" do
|
||||
BackupRestore.stubs(:can_rollback?).returns(true)
|
||||
BackupRestore.expects(:move_tables_between_schemas).with("backup", "public").never
|
||||
subject.rollback
|
||||
restorer.rollback
|
||||
|
||||
execute_stubbed_restore
|
||||
|
||||
BackupRestore.expects(:move_tables_between_schemas).with("backup", "public").once
|
||||
subject.rollback
|
||||
restorer.rollback
|
||||
end
|
||||
end
|
||||
|
||||
@ -164,7 +164,7 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
|
||||
it "doesn't try to drop function when no functions have been created" do
|
||||
Migration::BaseDropper.expects(:drop_readonly_function).never
|
||||
subject.clean_up
|
||||
restorer.clean_up
|
||||
end
|
||||
|
||||
it "creates and drops all functions when none exist" do
|
||||
@ -174,7 +174,7 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
|
||||
Migration::BaseDropper.expects(:drop_readonly_function).with(:posts, :via_email)
|
||||
Migration::BaseDropper.expects(:drop_readonly_function).with(:posts, :raw_email)
|
||||
subject.clean_up
|
||||
restorer.clean_up
|
||||
end
|
||||
|
||||
it "creates and drops only missing functions during restore" do
|
||||
@ -186,19 +186,17 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
execute_stubbed_restore(stub_readonly_functions: false)
|
||||
|
||||
Migration::BaseDropper.expects(:drop_readonly_function).with(:posts, :via_email)
|
||||
subject.clean_up
|
||||
restorer.clean_up
|
||||
end
|
||||
end
|
||||
|
||||
describe ".drop_backup_schema" do
|
||||
subject { BackupRestore::DatabaseRestorer }
|
||||
|
||||
context "when no backup schema exists" do
|
||||
it "doesn't do anything" do
|
||||
ActiveRecord::Base.connection.expects(:schema_exists?).with("backup").returns(false)
|
||||
ActiveRecord::Base.connection.expects(:drop_schema).never
|
||||
|
||||
subject.drop_backup_schema
|
||||
described_class.drop_backup_schema
|
||||
end
|
||||
end
|
||||
|
||||
@ -209,14 +207,14 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
ActiveRecord::Base.connection.expects(:drop_schema).with("backup")
|
||||
BackupMetadata.update_last_restore_date(8.days.ago)
|
||||
|
||||
subject.drop_backup_schema
|
||||
described_class.drop_backup_schema
|
||||
end
|
||||
|
||||
it "doesn't drop the schema when the last restore was recently" do
|
||||
ActiveRecord::Base.connection.expects(:drop_schema).with("backup").never
|
||||
BackupMetadata.update_last_restore_date(6.days.ago)
|
||||
|
||||
subject.drop_backup_schema
|
||||
described_class.drop_backup_schema
|
||||
end
|
||||
|
||||
it "stores the current date when there is no record of the last restore" do
|
||||
@ -225,7 +223,7 @@ RSpec.describe BackupRestore::DatabaseRestorer do
|
||||
date_string = "2020-01-08T17:38:27Z"
|
||||
freeze_time(Time.parse(date_string))
|
||||
|
||||
subject.drop_backup_schema
|
||||
described_class.drop_backup_schema
|
||||
expect(BackupMetadata.value_for(BackupMetadata::LAST_RESTORE_DATE)).to eq(date_string)
|
||||
end
|
||||
end
|
||||
|
@ -4,6 +4,8 @@ require "backup_restore/local_backup_store"
|
||||
require_relative "shared_examples_for_backup_store"
|
||||
|
||||
RSpec.describe BackupRestore::LocalBackupStore do
|
||||
subject(:store) { BackupRestore::BackupStore.create(root_directory: @root_directory) }
|
||||
|
||||
before do
|
||||
@root_directory = Dir.mktmpdir
|
||||
@paths = []
|
||||
@ -12,7 +14,6 @@ RSpec.describe BackupRestore::LocalBackupStore do
|
||||
|
||||
after { FileUtils.remove_dir(@root_directory, true) }
|
||||
|
||||
subject(:store) { BackupRestore::BackupStore.create(root_directory: @root_directory) }
|
||||
let(:expected_type) { BackupRestore::LocalBackupStore }
|
||||
|
||||
it_behaves_like "backup store"
|
||||
|
@ -5,6 +5,8 @@ require "backup_restore/s3_backup_store"
|
||||
require_relative "shared_examples_for_backup_store"
|
||||
|
||||
RSpec.describe BackupRestore::S3BackupStore do
|
||||
subject(:store) { BackupRestore::BackupStore.create(s3_options: @s3_options) }
|
||||
|
||||
before do
|
||||
@s3_client = Aws::S3::Client.new(stub_responses: true)
|
||||
@s3_options = { client: @s3_client }
|
||||
@ -86,7 +88,6 @@ RSpec.describe BackupRestore::S3BackupStore do
|
||||
SiteSetting.backup_location = BackupLocationSiteSetting::S3
|
||||
end
|
||||
|
||||
subject(:store) { BackupRestore::BackupStore.create(s3_options: @s3_options) }
|
||||
let(:expected_type) { BackupRestore::S3BackupStore }
|
||||
|
||||
it_behaves_like "backup store"
|
||||
|
@ -3,9 +3,9 @@
|
||||
require_relative "shared_context_for_backup_restore"
|
||||
|
||||
RSpec.describe BackupRestore::SystemInterface, type: :multisite do
|
||||
include_context "with shared stuff"
|
||||
subject(:system_interface) { BackupRestore::SystemInterface.new(logger) }
|
||||
|
||||
subject { BackupRestore::SystemInterface.new(logger) }
|
||||
include_context "with shared stuff"
|
||||
|
||||
describe "#flush_redis" do
|
||||
it "removes only keys from the current site in a multisite" do
|
||||
@ -24,7 +24,7 @@ RSpec.describe BackupRestore::SystemInterface, type: :multisite do
|
||||
expect(Discourse.redis.get("foo")).to eq("second-foo")
|
||||
expect(Discourse.redis.get("bar")).to eq("second-bar")
|
||||
|
||||
subject.flush_redis
|
||||
system_interface.flush_redis
|
||||
|
||||
expect(Discourse.redis.get("foo")).to be_nil
|
||||
expect(Discourse.redis.get("bar")).to be_nil
|
||||
@ -43,7 +43,7 @@ RSpec.describe BackupRestore::SystemInterface, type: :multisite do
|
||||
BackupRestore.mark_as_running!
|
||||
|
||||
expect do
|
||||
thread = subject.listen_for_shutdown_signal
|
||||
thread = system_interface.listen_for_shutdown_signal
|
||||
BackupRestore.set_shutdown_signal!
|
||||
thread.join
|
||||
end.to raise_error(SystemExit)
|
||||
|
@ -3,9 +3,9 @@
|
||||
require_relative "shared_context_for_backup_restore"
|
||||
|
||||
RSpec.describe BackupRestore::SystemInterface do
|
||||
include_context "with shared stuff"
|
||||
subject(:system_interface) { BackupRestore::SystemInterface.new(logger) }
|
||||
|
||||
subject { BackupRestore::SystemInterface.new(logger) }
|
||||
include_context "with shared stuff"
|
||||
|
||||
describe "readonly mode" do
|
||||
after { Discourse::READONLY_KEYS.each { |key| Discourse.redis.del(key) } }
|
||||
@ -13,26 +13,26 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
describe "#enable_readonly_mode" do
|
||||
it "enables readonly mode" do
|
||||
Discourse.expects(:enable_readonly_mode).once
|
||||
subject.enable_readonly_mode
|
||||
system_interface.enable_readonly_mode
|
||||
end
|
||||
|
||||
it "does not enable readonly mode when it is already in readonly mode" do
|
||||
Discourse.enable_readonly_mode
|
||||
Discourse.expects(:enable_readonly_mode).never
|
||||
subject.enable_readonly_mode
|
||||
system_interface.enable_readonly_mode
|
||||
end
|
||||
end
|
||||
|
||||
describe "#disable_readonly_mode" do
|
||||
it "disables readonly mode" do
|
||||
Discourse.expects(:disable_readonly_mode).once
|
||||
subject.disable_readonly_mode
|
||||
system_interface.disable_readonly_mode
|
||||
end
|
||||
|
||||
it "does not disable readonly mode when readonly mode was explicitly enabled" do
|
||||
Discourse.enable_readonly_mode
|
||||
Discourse.expects(:disable_readonly_mode).never
|
||||
subject.disable_readonly_mode
|
||||
system_interface.disable_readonly_mode
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -40,14 +40,14 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
describe "#mark_restore_as_running" do
|
||||
it "calls mark_restore_as_running" do
|
||||
BackupRestore.expects(:mark_as_running!).once
|
||||
subject.mark_restore_as_running
|
||||
system_interface.mark_restore_as_running
|
||||
end
|
||||
end
|
||||
|
||||
describe "#mark_restore_as_not_running" do
|
||||
it "calls mark_restore_as_not_running" do
|
||||
BackupRestore.expects(:mark_as_not_running!).once
|
||||
subject.mark_restore_as_not_running
|
||||
system_interface.mark_restore_as_not_running
|
||||
end
|
||||
end
|
||||
|
||||
@ -61,7 +61,7 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
|
||||
it "exits the process when shutdown signal is set" do
|
||||
expect do
|
||||
thread = subject.listen_for_shutdown_signal
|
||||
thread = system_interface.listen_for_shutdown_signal
|
||||
BackupRestore.set_shutdown_signal!
|
||||
thread.join
|
||||
end.to raise_error(SystemExit)
|
||||
@ -71,7 +71,7 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
BackupRestore.set_shutdown_signal!
|
||||
expect(BackupRestore.should_shutdown?).to eq(true)
|
||||
|
||||
thread = subject.listen_for_shutdown_signal
|
||||
thread = system_interface.listen_for_shutdown_signal
|
||||
expect(BackupRestore.should_shutdown?).to eq(false)
|
||||
Thread.kill(thread)
|
||||
end
|
||||
@ -82,7 +82,7 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
|
||||
it "calls pause!" do
|
||||
expect(Sidekiq.paused?).to eq(false)
|
||||
subject.pause_sidekiq("my reason")
|
||||
system_interface.pause_sidekiq("my reason")
|
||||
expect(Sidekiq.paused?).to eq(true)
|
||||
expect(Discourse.redis.get(SidekiqPauser::PAUSED_KEY)).to eq("my reason")
|
||||
end
|
||||
@ -93,15 +93,15 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
Sidekiq.pause!
|
||||
expect(Sidekiq.paused?).to eq(true)
|
||||
|
||||
subject.unpause_sidekiq
|
||||
system_interface.unpause_sidekiq
|
||||
expect(Sidekiq.paused?).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#wait_for_sidekiq" do
|
||||
it "waits 6 seconds even when there are no running Sidekiq jobs" do
|
||||
subject.expects(:sleep).with(6).once
|
||||
subject.wait_for_sidekiq
|
||||
system_interface.expects(:sleep).with(6).once
|
||||
system_interface.wait_for_sidekiq
|
||||
end
|
||||
|
||||
context "with Sidekiq workers" do
|
||||
@ -146,22 +146,26 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
end
|
||||
|
||||
it "waits up to 60 seconds for jobs running for the current site to finish" do
|
||||
subject.expects(:sleep).with(6).times(10)
|
||||
system_interface.expects(:sleep).with(6).times(10)
|
||||
create_workers
|
||||
expect { subject.wait_for_sidekiq }.to raise_error(BackupRestore::RunningSidekiqJobsError)
|
||||
expect { system_interface.wait_for_sidekiq }.to raise_error(
|
||||
BackupRestore::RunningSidekiqJobsError,
|
||||
)
|
||||
end
|
||||
|
||||
it "waits up to 60 seconds for jobs running on all sites to finish" do
|
||||
subject.expects(:sleep).with(6).times(10)
|
||||
system_interface.expects(:sleep).with(6).times(10)
|
||||
create_workers(all_sites: true)
|
||||
expect { subject.wait_for_sidekiq }.to raise_error(BackupRestore::RunningSidekiqJobsError)
|
||||
expect { system_interface.wait_for_sidekiq }.to raise_error(
|
||||
BackupRestore::RunningSidekiqJobsError,
|
||||
)
|
||||
end
|
||||
|
||||
it "ignores jobs of other sites" do
|
||||
subject.expects(:sleep).with(6).once
|
||||
system_interface.expects(:sleep).with(6).once
|
||||
create_workers(site_id: "another_site")
|
||||
|
||||
subject.wait_for_sidekiq
|
||||
system_interface.wait_for_sidekiq
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -172,7 +176,7 @@ RSpec.describe BackupRestore::SystemInterface do
|
||||
|
||||
it "doesn't unpause Sidekiq" do
|
||||
Sidekiq.pause!
|
||||
subject.flush_redis
|
||||
system_interface.flush_redis
|
||||
|
||||
expect(Sidekiq.paused?).to eq(true)
|
||||
end
|
||||
|
@ -4,9 +4,9 @@
|
||||
require_relative "shared_context_for_backup_restore"
|
||||
|
||||
RSpec.describe BackupRestore::UploadsRestorer do
|
||||
include_context "with shared stuff"
|
||||
subject(:restorer) { BackupRestore::UploadsRestorer.new(logger) }
|
||||
|
||||
subject { BackupRestore::UploadsRestorer.new(logger) }
|
||||
include_context "with shared stuff"
|
||||
|
||||
def with_temp_uploads_directory(name: "default", with_optimized: false)
|
||||
Dir.mktmpdir do |directory|
|
||||
@ -93,7 +93,7 @@ RSpec.describe BackupRestore::UploadsRestorer do
|
||||
|
||||
def setup_and_restore(directory, metadata)
|
||||
metadata.each { |d| BackupMetadata.create!(d) }
|
||||
subject.restore(directory)
|
||||
restorer.restore(directory)
|
||||
end
|
||||
|
||||
def uploads_path(database)
|
||||
@ -119,15 +119,15 @@ RSpec.describe BackupRestore::UploadsRestorer do
|
||||
let(:target_site_name) { target_site_type == multisite ? "second" : "default" }
|
||||
let(:target_hostname) { target_site_type == multisite ? "test2.localhost" : "test.localhost" }
|
||||
|
||||
shared_context "with no uploads" do
|
||||
shared_examples "with no uploads" do
|
||||
it "does nothing when temporary uploads directory is missing or empty" do
|
||||
store_class.any_instance.expects(:copy_from).never
|
||||
|
||||
Dir.mktmpdir do |directory|
|
||||
subject.restore(directory)
|
||||
restorer.restore(directory)
|
||||
|
||||
FileUtils.mkdir(File.join(directory, "uploads"))
|
||||
subject.restore(directory)
|
||||
restorer.restore(directory)
|
||||
end
|
||||
end
|
||||
end
|
||||
@ -176,7 +176,7 @@ RSpec.describe BackupRestore::UploadsRestorer do
|
||||
with_temp_uploads_directory do |directory, path|
|
||||
store_class.any_instance.expects(:copy_from).with(path).once
|
||||
|
||||
expect { subject.restore(directory) }.to change { OptimizedImage.count }.by_at_most(
|
||||
expect { restorer.restore(directory) }.to change { OptimizedImage.count }.by_at_most(
|
||||
-1,
|
||||
).and change { Jobs::CreateAvatarThumbnails.jobs.size }.by(1).and change {
|
||||
Post.where(baked_version: nil).count
|
||||
@ -190,7 +190,7 @@ RSpec.describe BackupRestore::UploadsRestorer do
|
||||
with_temp_uploads_directory(with_optimized: true) do |directory, path|
|
||||
store_class.any_instance.expects(:copy_from).with(path).once
|
||||
|
||||
expect { subject.restore(directory) }.to not_change {
|
||||
expect { restorer.restore(directory) }.to not_change {
|
||||
OptimizedImage.count
|
||||
}.and not_change { Jobs::CreateAvatarThumbnails.jobs.size }.and change {
|
||||
Post.where(baked_version: nil).count
|
||||
@ -609,14 +609,14 @@ RSpec.describe BackupRestore::UploadsRestorer do
|
||||
Discourse.stubs(:store).returns(Object.new)
|
||||
|
||||
with_temp_uploads_directory do |directory|
|
||||
expect { subject.restore(directory) }.to raise_error(BackupRestore::UploadsRestoreError)
|
||||
expect { restorer.restore(directory) }.to raise_error(BackupRestore::UploadsRestoreError)
|
||||
end
|
||||
end
|
||||
|
||||
it "raises an exception when there are multiple folders in the uploads directory" do
|
||||
with_temp_uploads_directory do |directory|
|
||||
FileUtils.mkdir_p(File.join(directory, "uploads", "foo"))
|
||||
expect { subject.restore(directory) }.to raise_error(BackupRestore::UploadsRestoreError)
|
||||
expect { restorer.restore(directory) }.to raise_error(BackupRestore::UploadsRestoreError)
|
||||
end
|
||||
end
|
||||
|
||||
|
Reference in New Issue
Block a user