Introduce a new FactoriesInMigrationSpecs cop
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
0fff9db5ea
commit
4b035896c4
6 changed files with 122 additions and 1 deletions
40
rubocop/cop/rspec/factories_in_migration_specs.rb
Normal file
40
rubocop/cop/rspec/factories_in_migration_specs.rb
Normal file
|
@ -0,0 +1,40 @@
|
||||||
|
require_relative '../../spec_helpers'
|
||||||
|
|
||||||
|
module RuboCop
|
||||||
|
module Cop
|
||||||
|
module RSpec
|
||||||
|
# This cop checks for the usage of factories in migration specs
|
||||||
|
#
|
||||||
|
# @example
|
||||||
|
#
|
||||||
|
# # bad
|
||||||
|
# let(:user) { create(:user) }
|
||||||
|
#
|
||||||
|
# # good
|
||||||
|
# let(:users) { table(:users) }
|
||||||
|
# let(:user) { users.create!(name: 'User 1', username: 'user1') }
|
||||||
|
class FactoriesInMigrationSpecs < RuboCop::Cop::Cop
|
||||||
|
include SpecHelpers
|
||||||
|
|
||||||
|
MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze
|
||||||
|
FORBIDDEN_METHODS = %i[build build_list create create_list].freeze
|
||||||
|
|
||||||
|
def_node_search :forbidden_factory_usage?, <<~PATTERN
|
||||||
|
(send {(const nil? :FactoryBot) nil?} {#{FORBIDDEN_METHODS.map(&:inspect).join(' ')}} ...)
|
||||||
|
PATTERN
|
||||||
|
|
||||||
|
# Following is what node.children looks like on a match:
|
||||||
|
# - Without FactoryBot namespace: [nil, :build, s(:sym, :user)]
|
||||||
|
# - With FactoryBot namespace: [s(:const, nil, :FactoryBot), :build, s(:sym, :user)]
|
||||||
|
def on_send(node)
|
||||||
|
return unless in_migration_spec?(node)
|
||||||
|
return unless forbidden_factory_usage?(node)
|
||||||
|
|
||||||
|
method = node.children[1]
|
||||||
|
|
||||||
|
add_offense(node, location: :expression, message: MESSAGE % method)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -21,4 +21,5 @@ require_relative 'cop/migration/update_column_in_batches'
|
||||||
require_relative 'cop/migration/update_large_table'
|
require_relative 'cop/migration/update_large_table'
|
||||||
require_relative 'cop/project_path_helper'
|
require_relative 'cop/project_path_helper'
|
||||||
require_relative 'cop/rspec/env_assignment'
|
require_relative 'cop/rspec/env_assignment'
|
||||||
|
require_relative 'cop/rspec/factories_in_migration_specs'
|
||||||
require_relative 'cop/sidekiq_options_queue'
|
require_relative 'cop/sidekiq_options_queue'
|
||||||
|
|
|
@ -6,7 +6,18 @@ module RuboCop
|
||||||
def in_spec?(node)
|
def in_spec?(node)
|
||||||
path = node.location.expression.source_buffer.name
|
path = node.location.expression.source_buffer.name
|
||||||
|
|
||||||
!SPEC_HELPERS.include?(File.basename(path)) && path.start_with?(File.join(Dir.pwd, 'spec'))
|
!SPEC_HELPERS.include?(File.basename(path)) &&
|
||||||
|
path.start_with?(File.join(Dir.pwd, 'spec'), File.join(Dir.pwd, 'ee', 'spec'))
|
||||||
|
end
|
||||||
|
|
||||||
|
# Returns true if the given node originated from a migration spec.
|
||||||
|
def in_migration_spec?(node)
|
||||||
|
path = node.location.expression.source_buffer.name
|
||||||
|
|
||||||
|
in_spec?(node) &&
|
||||||
|
path.start_with?(
|
||||||
|
File.join(Dir.pwd, 'spec', 'migrations'),
|
||||||
|
File.join(Dir.pwd, 'ee', 'spec', 'migrations'))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
48
spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb
Normal file
48
spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb
Normal file
|
@ -0,0 +1,48 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
require 'rubocop'
|
||||||
|
require 'rubocop/rspec/support'
|
||||||
|
|
||||||
|
require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs'
|
||||||
|
|
||||||
|
describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do
|
||||||
|
include CopHelper
|
||||||
|
|
||||||
|
let(:source_file) { 'spec/migrations/foo_spec.rb' }
|
||||||
|
|
||||||
|
subject(:cop) { described_class.new }
|
||||||
|
|
||||||
|
shared_examples 'an offensive factory call' do |namespace|
|
||||||
|
%i[build build_list create create_list].each do |forbidden_method|
|
||||||
|
namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:user)"
|
||||||
|
|
||||||
|
it "registers an offense for #{namespaced_forbidden_method}" do
|
||||||
|
expect_offense(<<-RUBY)
|
||||||
|
describe 'foo' do
|
||||||
|
let(:user) { #{namespaced_forbidden_method} }
|
||||||
|
#{'^' * namespaced_forbidden_method.size} Don't use FactoryBot.#{forbidden_method} in migration specs, use `table` instead.
|
||||||
|
end
|
||||||
|
RUBY
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'in a migration spec file' do
|
||||||
|
before do
|
||||||
|
allow(cop).to receive(:in_migration_spec?).and_return(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
it_behaves_like 'an offensive factory call', ''
|
||||||
|
it_behaves_like 'an offensive factory call', 'FactoryBot.'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'outside of a migration spec file' do
|
||||||
|
it "does not register an offense" do
|
||||||
|
expect_no_offenses(<<-RUBY)
|
||||||
|
describe 'foo' do
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
end
|
||||||
|
RUBY
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -66,6 +66,7 @@ RSpec.configure do |config|
|
||||||
config.include MigrationsHelpers, :migration
|
config.include MigrationsHelpers, :migration
|
||||||
config.include StubFeatureFlags
|
config.include StubFeatureFlags
|
||||||
config.include StubENV
|
config.include StubENV
|
||||||
|
config.include ExpectOffense
|
||||||
|
|
||||||
config.infer_spec_type_from_file_location!
|
config.infer_spec_type_from_file_location!
|
||||||
|
|
||||||
|
|
20
spec/support/helpers/expect_offense.rb
Normal file
20
spec/support/helpers/expect_offense.rb
Normal file
|
@ -0,0 +1,20 @@
|
||||||
|
require 'rubocop/rspec/support'
|
||||||
|
|
||||||
|
# https://github.com/backus/rubocop-rspec/blob/master/spec/support/expect_offense.rb
|
||||||
|
# rubocop-rspec gem extension of RuboCop's ExpectOffense module.
|
||||||
|
#
|
||||||
|
# This mixin is the same as rubocop's ExpectOffense except the default
|
||||||
|
# filename ends with `_spec.rb`
|
||||||
|
module ExpectOffense
|
||||||
|
include RuboCop::RSpec::ExpectOffense
|
||||||
|
|
||||||
|
DEFAULT_FILENAME = 'example_spec.rb'.freeze
|
||||||
|
|
||||||
|
def expect_offense(source, filename = DEFAULT_FILENAME)
|
||||||
|
super
|
||||||
|
end
|
||||||
|
|
||||||
|
def expect_no_offenses(source, filename = DEFAULT_FILENAME)
|
||||||
|
super
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue