From e9c2776252ae5f694d04cd6f80520877f18a9fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 13 Oct 2022 03:19:06 +0200 Subject: [PATCH] Don't create schemas in model tests unless needed Creating a schema takes about 3-4 seconds on my machine, so omitting the callbacks makes tests much faster. To do so, we're using the `insert!` method added in Rails 6.0, which inserts a record without executing callbacks or validations. To make the tests look consistent, we're adding a FactoryBot strategy which uses `insert!` instead of `create!`. Note this strategy is useless in most cases because it doesn't work when models have translatable attributes or associations. However, IMHO it's worth it even if we only use it for tenants. We could also use `Tenant.insert!` instead, but then we would have to add all the mandatory attributes, and in this case the code is clearer if we only add the attributes we need for the test. --- spec/factory_bot/strategy/insert.rb | 24 +++++++++++++++++++++ spec/models/abilities/administrator_spec.rb | 19 ++++++++-------- spec/models/setting_spec.rb | 7 +++--- spec/models/tenant_spec.rb | 4 ++-- spec/spec_helper.rb | 1 + 5 files changed, 39 insertions(+), 16 deletions(-) create mode 100644 spec/factory_bot/strategy/insert.rb diff --git a/spec/factory_bot/strategy/insert.rb b/spec/factory_bot/strategy/insert.rb new file mode 100644 index 000000000..5c7468a25 --- /dev/null +++ b/spec/factory_bot/strategy/insert.rb @@ -0,0 +1,24 @@ +module FactoryBot + module Strategy + class Insert + def initialize + @strategy = FactoryBot.strategy_by_name(:attributes_for).new + end + + delegate :association, to: :@strategy + + def result(evaluation) + build_class = evaluation.instance_variable_get(:@attribute_assigner) + .instance_variable_get(:@build_class) + + timestamps = { created_at: Time.current, updated_at: Time.current }.select do |attribute, _| + build_class.has_attribute?(attribute) + end + + build_class.insert!(timestamps.merge(@strategy.result(evaluation))) + end + end + + FactoryBot.register_strategy(:insert, Insert) + end +end diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index 4b8e2376f..a34ec08e6 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -183,17 +183,16 @@ describe Abilities::Administrator do it { should be_able_to :update, Tenant } it { should_not be_able_to :destroy, Tenant } - it "does not allow administrators from other tenants to manage tenants " do - create(:tenant, schema: "subsidiary") - - Tenant.switch("subsidiary") do - admin = create(:administrator).user - - expect(admin).not_to be_able_to :create, Tenant - expect(admin).not_to be_able_to :read, Tenant - expect(admin).not_to be_able_to :update, Tenant - expect(admin).not_to be_able_to :destroy, Tenant + context "administrators from other tenants" do + before do + insert(:tenant, schema: "subsidiary") + allow(Tenant).to receive(:current_schema).and_return("subsidiary") end + + it { should_not be_able_to :create, Tenant } + it { should_not be_able_to :read, Tenant } + it { should_not be_able_to :update, Tenant } + it { should_not be_able_to :destroy, Tenant } end end end diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 13b168d14..38fc743e5 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -193,11 +193,10 @@ describe Setting do end it "returns the tenant name for other tenants" do - create(:tenant, schema: "new", name: "New Institution") + insert(:tenant, schema: "new", name: "New Institution") + allow(Tenant).to receive(:current_schema).and_return("new") - Tenant.switch("new") do - expect(Setting.default_org_name).to eq "New Institution" - end + expect(Setting.default_org_name).to eq "New Institution" end end diff --git a/spec/models/tenant_spec.rb b/spec/models/tenant_spec.rb index 0d5cdfc8b..f4e7e32c5 100644 --- a/spec/models/tenant_spec.rb +++ b/spec/models/tenant_spec.rb @@ -243,7 +243,7 @@ describe Tenant do end it "is not valid with an already existing schema" do - expect(create(:tenant, schema: "subdomainx")).to be_valid + insert(:tenant, schema: "subdomainx") expect(build(:tenant, schema: "subdomainx")).not_to be_valid end @@ -270,7 +270,7 @@ describe Tenant do end it "is not valid with an already existing name" do - expect(create(:tenant, name: "Name X")).to be_valid + insert(:tenant, name: "Name X") expect(build(:tenant, name: "Name X")).not_to be_valid end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a2f22cf87..7c941094e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,7 @@ require "email_spec" require "devise" require "knapsack_pro" +Dir["./spec/factory_bot/**/*.rb"].sort.each { |f| require f } Dir["./spec/models/concerns/*.rb"].each { |f| require f } Dir["./spec/support/**/*.rb"].sort.each { |f| require f } Dir["./spec/shared/**/*.rb"].sort.each { |f| require f }