From 0104671e012b40aa5eb1a9a552bf2a4345bb35d5 Mon Sep 17 00:00:00 2001
From: Henne Vogelsang <hvogel@opensuse.org>
Date: Wed, 18 Aug 2021 13:14:44 +0200
Subject: [PATCH 1/2] Stop validating factories in specs

We do validate all of them in a before(:suite) already.
---
 spec/controllers/admin/users_controller_spec.rb |  9 ---------
 spec/models/booth_spec.rb                       |  4 ----
 spec/models/conference_spec.rb                  |  4 ----
 spec/models/email_settings_spec.rb              |  4 ----
 spec/models/event_schedule_spec.rb              |  4 ----
 spec/models/event_spec.rb                       |  4 ----
 spec/models/event_type_spec.rb                  |  4 ----
 spec/models/payment_spec.rb                     |  6 ------
 spec/models/physical_ticket_spec.rb             | 16 ----------------
 spec/models/program_spec.rb                     |  8 --------
 spec/models/registration_period_spec.rb         |  4 ----
 spec/models/registration_spec.rb                |  4 ----
 spec/models/resource_spec.rb                    |  4 ----
 spec/models/sponsor_spec.rb                     |  4 ----
 spec/models/sponsorship_level_spec.rb           |  4 ----
 spec/models/ticket_purchase_spec.rb             |  4 ----
 spec/models/ticket_spec.rb                      |  5 -----
 spec/models/track_spec.rb                       |  4 ----
 spec/models/user_spec.rb                        |  4 ----
 spec/models/vote_spec.rb                        |  4 ----
 20 files changed, 104 deletions(-)
 delete mode 100644 spec/models/physical_ticket_spec.rb

diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 51dfb04c..bf5d125e 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -38,15 +38,6 @@ describe Admin::UsersController do
       before :each do
         patch :update, params: { id: user.id, user: { name: 'new name', email: 'new_email@osem.io' } }
       end
-
-      it 'locates requested @user' do
-        expect(build(:user, id: user.id)).to eq(user)
-      end
-      it 'changes @users attributes' do
-        expect(build(
-          :user, email: 'email_new@osem.io', id: user.id).email)
-              .to eq('email_new@osem.io')
-      end
       it 'redirects to the updated user' do
         expect(response).to redirect_to admin_users_path
       end
diff --git a/spec/models/booth_spec.rb b/spec/models/booth_spec.rb
index cec4d4ac..8a501767 100644
--- a/spec/models/booth_spec.rb
+++ b/spec/models/booth_spec.rb
@@ -7,10 +7,6 @@ describe Booth do
   let!(:conference) { create(:conference) }
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:booth)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:reasoning) }
     it { is_expected.to validate_presence_of(:description) }
     it { is_expected.to validate_presence_of(:responsibles) }
diff --git a/spec/models/conference_spec.rb b/spec/models/conference_spec.rb
index 1c3ecb3b..fc593a14 100755
--- a/spec/models/conference_spec.rb
+++ b/spec/models/conference_spec.rb
@@ -1484,10 +1484,6 @@ describe Conference do
 
   describe 'validations' do
 
-    it 'has a valid factory' do
-      expect(build(:conference)).to be_valid
-    end
-
     it 'is not valid without a title' do
       should validate_presence_of(:title)
     end
diff --git a/spec/models/email_settings_spec.rb b/spec/models/email_settings_spec.rb
index 875dba6d..f79857f7 100644
--- a/spec/models/email_settings_spec.rb
+++ b/spec/models/email_settings_spec.rb
@@ -23,10 +23,6 @@ describe EmailSettings do
     }
   end
 
-  it 'has a valid factory' do
-    expect(build(:email_settings)).to be_valid
-  end
-
   describe '#get_values' do
     context 'user has name' do
       it 'returns correct key-value pairs' do
diff --git a/spec/models/event_schedule_spec.rb b/spec/models/event_schedule_spec.rb
index 56cedcd4..f111e896 100644
--- a/spec/models/event_schedule_spec.rb
+++ b/spec/models/event_schedule_spec.rb
@@ -12,10 +12,6 @@ describe EventSchedule do
   end
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:event_schedule)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:schedule) }
     it { is_expected.to validate_presence_of(:event) }
     it { is_expected.to validate_presence_of(:room) }
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 92768222..d299bedf 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -18,10 +18,6 @@ describe Event do
   end
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:event)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:title) }
     it { is_expected.to validate_presence_of(:abstract) }
     it { is_expected.to validate_presence_of(:program) }
diff --git a/spec/models/event_type_spec.rb b/spec/models/event_type_spec.rb
index 9fb65950..2b696ead 100644
--- a/spec/models/event_type_spec.rb
+++ b/spec/models/event_type_spec.rb
@@ -12,10 +12,6 @@ describe EventType do
   end
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:event_type)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:title) }
     it { is_expected.to validate_presence_of(:minimum_abstract_length) }
     it { is_expected.to validate_presence_of(:maximum_abstract_length) }
diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb
index f011a749..702ad725 100644
--- a/spec/models/payment_spec.rb
+++ b/spec/models/payment_spec.rb
@@ -13,14 +13,8 @@ describe Payment do
   end
 
   describe 'validations' do
-    it 'has a valid factory' do
-      expect(build(:payment)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:status) }
-
     it { is_expected.to validate_presence_of(:user_id) }
-
     it { is_expected.to validate_presence_of(:conference_id) }
   end
 
diff --git a/spec/models/physical_ticket_spec.rb b/spec/models/physical_ticket_spec.rb
deleted file mode 100644
index 31afd621..00000000
--- a/spec/models/physical_ticket_spec.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe PhysicalTicket do
-
-  describe 'association' do
-    it { is_expected.to belong_to :ticket_purchase }
-  end
-
-  describe 'validations' do
-    it 'has a valid factory' do
-      expect(build(:physical_ticket)).to be_valid
-    end
-  end
-end
diff --git a/spec/models/program_spec.rb b/spec/models/program_spec.rb
index e8db5880..1aa7eb29 100644
--- a/spec/models/program_spec.rb
+++ b/spec/models/program_spec.rb
@@ -25,14 +25,6 @@ describe Program do
   end
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:program)).to be_valid
-    end
-
-    it 'is valid for rating of 5' do
-      expect(build(:program, rating: 5)).to be_valid
-    end
-
     it { is_expected.to validate_numericality_of(:rating).is_greater_than_or_equal_to(0).is_less_than_or_equal_to(10).only_integer }
 
     it { is_expected.to validate_numericality_of(:schedule_interval).is_greater_than_or_equal_to(5).is_less_than_or_equal_to(60) }
diff --git a/spec/models/registration_period_spec.rb b/spec/models/registration_period_spec.rb
index 10da0684..ae33e51d 100644
--- a/spec/models/registration_period_spec.rb
+++ b/spec/models/registration_period_spec.rb
@@ -8,10 +8,6 @@ describe RegistrationPeriod do
   let!(:registration_period) { create(:registration_period, start_date: Date.today - 2, end_date: Date.today - 1, conference: conference) }
 
   describe 'validations' do
-    it 'has a valid factory' do
-      expect(build(:registration_period)).to be_valid
-    end
-
     it 'is not valid without a start_date' do
       should validate_presence_of(:start_date)
     end
diff --git a/spec/models/registration_spec.rb b/spec/models/registration_spec.rb
index d6c86838..940a6044 100644
--- a/spec/models/registration_spec.rb
+++ b/spec/models/registration_spec.rb
@@ -9,10 +9,6 @@ describe Registration do
   let!(:registration) { create(:registration, conference: conference, user: user) }
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:registration)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:user) }
 
     it 'validates uniqueness of user in scope of conference' do
diff --git a/spec/models/resource_spec.rb b/spec/models/resource_spec.rb
index d70e3144..9ef5a71a 100644
--- a/spec/models/resource_spec.rb
+++ b/spec/models/resource_spec.rb
@@ -24,10 +24,6 @@ describe Resource do
 
   it { is_expected.to allow_value(0).for(:quantity) }
 
-  it 'has a valid factory' do
-    expect(build(:resource)).to be_valid
-  end
-
   it 'is not valid with used greater than quantity' do
     resource.used = resource.quantity + 1
     expect(resource.valid?).to eq false
diff --git a/spec/models/sponsor_spec.rb b/spec/models/sponsor_spec.rb
index 7f6e3e50..fcab9330 100644
--- a/spec/models/sponsor_spec.rb
+++ b/spec/models/sponsor_spec.rb
@@ -4,10 +4,6 @@ require 'spec_helper'
 
 describe Sponsor do
   describe 'validations' do
-    it 'has a valid factory' do
-      expect(build(:sponsor)).to be_valid
-    end
-
     it 'is not valid without a name' do
       should validate_presence_of(:name)
     end
diff --git a/spec/models/sponsorship_level_spec.rb b/spec/models/sponsorship_level_spec.rb
index c6b71a88..5636c730 100644
--- a/spec/models/sponsorship_level_spec.rb
+++ b/spec/models/sponsorship_level_spec.rb
@@ -5,10 +5,6 @@ require 'spec_helper'
 describe SponsorshipLevel do
   describe 'validation' do
 
-    it 'has a valid factory' do
-      expect(build(:sponsorship_level)).to be_valid
-    end
-
     it 'is not valid without a title' do
       should validate_presence_of(:title)
     end
diff --git a/spec/models/ticket_purchase_spec.rb b/spec/models/ticket_purchase_spec.rb
index 688864c6..6c28681b 100644
--- a/spec/models/ticket_purchase_spec.rb
+++ b/spec/models/ticket_purchase_spec.rb
@@ -5,10 +5,6 @@ require 'spec_helper'
 describe TicketPurchase do
 
   describe 'validations' do
-    it 'has a valid factory' do
-      expect(build(:ticket_purchase)).to be_valid
-    end
-
     it 'is not valid without a conference_id' do
       should validate_presence_of(:conference_id)
     end
diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb
index 0dcb5cad..858e4d37 100644
--- a/spec/models/ticket_spec.rb
+++ b/spec/models/ticket_spec.rb
@@ -8,11 +8,6 @@ describe Ticket do
   let(:user) { create(:user) }
 
   describe 'validation' do
-
-    it 'has a valid factory' do
-      expect(build(:ticket)).to be_valid
-    end
-
     it 'is not valid without a title' do
       should validate_presence_of(:title)
     end
diff --git a/spec/models/track_spec.rb b/spec/models/track_spec.rb
index b58fc622..27066442 100644
--- a/spec/models/track_spec.rb
+++ b/spec/models/track_spec.rb
@@ -17,10 +17,6 @@ describe Track do
   end
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:track)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to allow_value('#ABCDEF').for(:color) }
     it { is_expected.to allow_value('#124689').for(:color) }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4c20b752..eda4d725 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -21,10 +21,6 @@ describe User do
   let(:events_registration) { create(:events_registration, event: event1, registration: registration) }
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:user)).to be_valid
-    end
-
     it { is_expected.to validate_presence_of(:email) }
     it { is_expected.to validate_presence_of(:username) }
     it { is_expected.to validate_uniqueness_of(:username).ignoring_case_sensitivity }
diff --git a/spec/models/vote_spec.rb b/spec/models/vote_spec.rb
index 39afb352..7ed45a36 100644
--- a/spec/models/vote_spec.rb
+++ b/spec/models/vote_spec.rb
@@ -6,10 +6,6 @@ describe Vote do
   let!(:vote) { create(:vote) }
 
   describe 'validation' do
-    it 'has a valid factory' do
-      expect(build(:vote)).to be_valid
-    end
-
     it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:event_id) }
 
     # This is testing the relationship instead of using the shoulda-matchers

From 7dbe2d19fe710f971aeb780794d3e38155ab7b81 Mon Sep 17 00:00:00 2001
From: Henne Vogelsang <hvogel@opensuse.org>
Date: Wed, 18 Aug 2021 14:05:19 +0200
Subject: [PATCH 2/2] Move factory linting to CI cycle

No need to do this before each and every spec or example. Speeds
up suite and make things less fragile.
---
 .github/workflows/spec.yml    |  1 +
 dotenv.example                |  4 ----
 lib/tasks/factory_bot.rake    | 11 ++++-------
 lib/tasks/migrate_config.rake |  1 -
 spec/support/factory_bot.rb   | 20 --------------------
 5 files changed, 5 insertions(+), 32 deletions(-)
 delete mode 100644 spec/support/factory_bot.rb

diff --git a/.github/workflows/spec.yml b/.github/workflows/spec.yml
index 9431e8b4..a80430d0 100644
--- a/.github/workflows/spec.yml
+++ b/.github/workflows/spec.yml
@@ -41,6 +41,7 @@ jobs:
           rm -f osem_test osem_development
           bundle exec rake db:setup --trace
           bundle exec bin/rails webdrivers:chromedriver:update
+          bundle exec rake factory_bot:lint RAILS_ENV=test
       - name: spec/${{ matrix.suite }}
         run: bundle exec rake spec:${{ matrix.suite }}
       - name: coverage upload ${{ matrix.suite }}
diff --git a/dotenv.example b/dotenv.example
index dfecb0cb..f02147af 100644
--- a/dotenv.example
+++ b/dotenv.example
@@ -115,10 +115,6 @@
 # SKYLIGHT_AUTHENTICATION=1234
 # SKYLIGHT_PUBLIC_DASHBOARD_URL='https://oss.skylight.io/app/applications/xxxxxxxxxxxx'
 
-# Disable linting of factories in the test suite.
-# Speeds up turn around times of tests
-# OSEM_FACTORY_LINT=false
-
 # How should browser tests be performed?
 # For headless Chrome (default):
 # OSEM_TEST_DRIVER=chrome_headless
diff --git a/lib/tasks/factory_bot.rake b/lib/tasks/factory_bot.rake
index 177d2f09..971ed5fc 100644
--- a/lib/tasks/factory_bot.rake
+++ b/lib/tasks/factory_bot.rake
@@ -1,17 +1,14 @@
-# frozen_string_literal: true
-
 namespace :factory_bot do
   desc "Verify that all FactoryBot factories are valid"
   task lint: :environment do
     if Rails.env.test?
-      begin
-        DatabaseCleaner.start
+      conn = ActiveRecord::Base.connection
+      conn.transaction do
         FactoryBot.lint
-      ensure
-        DatabaseCleaner.clean
+        raise ActiveRecord::Rollback
       end
     else
-      system("bundle exec rake factory_bot:lint RAILS_ENV='test'")
+      raise "\nERROR: You should not run this outside the test environment...\n\n"
     end
   end
 end
diff --git a/lib/tasks/migrate_config.rake b/lib/tasks/migrate_config.rake
index 7cd22ade..a012d323 100644
--- a/lib/tasks/migrate_config.rake
+++ b/lib/tasks/migrate_config.rake
@@ -28,7 +28,6 @@ namespace :data do
       dot_env.puts "OSEM_ICHAIN_ENABLED=\"#{CONFIG['authentication']['ichain']['enabled']}\"" if CONFIG.has_key?(:authentication)
       dot_env.puts "OSEM_TRANSIFEX_APIKEY=\"#{CONFIG['transifex_live_api_key']}\""
       dot_env.puts "OSEM_ERRBIT_HOST=\"#{CONFIG['errbit_host']}\""
-      dot_env.puts "OSEM_FACTORY_LINT=\"#{CONFIG['factory_bot_lint']}\""
       dot_env.puts "OSEM_SMTP_ADDRESS=\"#{CONFIG['mail_address']}\""
       dot_env.puts "OSEM_SMTP_PORT=\"#{CONFIG['mail_port']}\""
       dot_env.puts "OSEM_SMTP_USERNAME=\"#{CONFIG['mail_username']}\""
diff --git a/spec/support/factory_bot.rb b/spec/support/factory_bot.rb
deleted file mode 100644
index 75e2be51..00000000
--- a/spec/support/factory_bot.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-require_relative 'external_request'
-
-RSpec.configure do |config|
-
-  config.before(:suite) do
-    if ENV['OSEM_FACTORY_LINT'] != 'false'
-      DatabaseCleaner.strategy = :transaction
-      DatabaseCleaner.clean_with(:truncation)
-      begin
-        DatabaseCleaner.start
-        mock_commercial_request
-        FactoryBot.lint
-      ensure
-        DatabaseCleaner.clean
-      end
-    end
-  end
-end