Merge pull request #2844 from hennevogel/refactoring/factory-linting

Validate factories exactly once

.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 }}

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

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

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']}\""

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

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) }

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

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

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) }

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) }

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) }

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
 

spec/models/physical_ticket_spec.rb

@@ -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

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) }

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

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

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

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

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

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

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

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) }

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 }

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

spec/support/factory_bot.rb

@@ -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