Merge branch 'detect_spec_duplicate_for_ee_extension' into 'master'

Detect duplicate locations for EE specs

Closes #209052

See merge request gitlab-org/gitlab!27556

.rubocop.yml

@@ -200,6 +200,35 @@ GitlabSecurity/PublicSend:
     - 'ee/lib/**/*.rake'
     - 'ee/spec/**/*'
 
+Gitlab/DuplicateSpecLocation:
+  Exclude:
+    - ee/spec/controllers/groups_controller_spec.rb
+    - ee/spec/controllers/projects/jobs_controller_spec.rb
+    - ee/spec/helpers/auth_helper_spec.rb
+    - ee/spec/lib/gitlab/gl_repository_spec.rb
+    - ee/spec/lib/gitlab/usage_data_spec.rb
+    - ee/spec/models/namespace_spec.rb
+    - ee/spec/models/note_spec.rb
+    - ee/spec/serializers/environment_entity_spec.rb
+    - ee/spec/services/issues/create_service_spec.rb
+    - ee/spec/services/merge_requests/create_service_spec.rb
+    - ee/spec/services/merge_requests/refresh_service_spec.rb
+    - ee/spec/services/merge_requests/update_service_spec.rb
+    - ee/spec/services/system_hooks_service_spec.rb
+    - ee/spec/controllers/ee/groups_controller_spec.rb
+    - ee/spec/controllers/ee/projects/jobs_controller_spec.rb
+    - ee/spec/helpers/ee/auth_helper_spec.rb
+    - ee/spec/lib/ee/gitlab/gl_repository_spec.rb
+    - ee/spec/lib/ee/gitlab/usage_data_spec.rb
+    - ee/spec/models/ee/namespace_spec.rb
+    - ee/spec/models/ee/note_spec.rb
+    - ee/spec/serializers/ee/environment_entity_spec.rb
+    - ee/spec/services/ee/issues/create_service_spec.rb
+    - ee/spec/services/ee/merge_requests/create_service_spec.rb
+    - ee/spec/services/ee/merge_requests/refresh_service_spec.rb
+    - ee/spec/services/ee/merge_requests/update_service_spec.rb
+    - ee/spec/services/ee/system_hooks_service_spec.rb
+
 Cop/InjectEnterpriseEditionModule:
   Enabled: true
   Exclude:

rubocop/cop/gitlab/duplicate_spec_location.rb

+# frozen_string_literal: true
+
+require 'rubocop/rspec/top_level_describe'
+
+module RuboCop
+  module Cop
+    module Gitlab
+      # Cop that detects duplicate EE spec files
+      #
+      # There should not be files in both ee/spec/*/ee/my_spec.rb and ee/spec/*/my_spec.rb
+      #
+      #  # bad
+      #  ee/spec/controllers/my_spec.rb      # describe MyClass
+      #  ee/spec/controllers/ee/my_spec.rb   # describe MyClass
+      #
+      #  # good, spec for EE extension code
+      #  ee/spec/controllers/ee/my_spec.rb   # describe MyClass
+      #
+      #  # good, spec for EE only code
+      #  ee/spec/controllers/my_spec.rb      # describe MyClass
+      #
+      class DuplicateSpecLocation < RuboCop::Cop::Cop
+        include RuboCop::RSpec::TopLevelDescribe
+
+        MSG = 'Duplicate spec location in `%<path>s`.'
+
+        def on_top_level_describe(node, _args)
+          path = file_path_for_node(node).sub(/\A#{rails_root}\//, '')
+          duplicate_path = find_duplicate_path(path)
+
+          if duplicate_path && File.exist?(File.join(rails_root, duplicate_path))
+            add_offense(node, message: format(MSG, path: duplicate_path))
+          end
+        end
+
+        private
+
+        def ee_spec?(path)
+          File.fnmatch?('ee/spec/**/*.rb', path, File::FNM_PATHNAME)
+        end
+
+        def find_duplicate_path(path)
+          return unless ee_spec?(path)
+
+          if File.fnmatch?('ee/spec/**/ee/**', path)
+            path.match('\A(ee/spec/[^/]+)/ee/(.+)') do |match|
+              File.join(match[1], match[2])
+            end
+          else
+            path.match('\A(ee/spec/[^/]+)/(.+)') do |match|
+              File.join(match[1], 'ee', match[2])
+            end
+          end
+        end
+
+        def file_path_for_node(node)
+          node.location.expression.source_buffer.name
+        end
+
+        def rails_root
+          File.expand_path('../../..', __dir__)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/gitlab/duplicate_spec_location_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/gitlab/duplicate_spec_location'
+
+describe RuboCop::Cop::Gitlab::DuplicateSpecLocation do
+  include RuboCop::RSpec::ExpectOffense
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  let(:rails_root) { '../../../../' }
+
+  def full_path(path)
+    File.expand_path(File.join(rails_root, path), __dir__)
+  end
+
+  context 'Non-EE spec file' do
+    it 'registers no offenses' do
+      expect_no_offenses(<<~SOURCE.strip_indent, full_path('spec/foo_spec.rb'))
+        describe 'Foo' do
+        end
+      SOURCE
+    end
+  end
+
+  context 'Non-EE application file' do
+    it 'registers no offenses' do
+      expect_no_offenses(<<~SOURCE.strip_indent, full_path('app/models/blog_post.rb'))
+        class BlogPost
+        end
+      SOURCE
+    end
+  end
+
+  context 'EE application file' do
+    it 'registers no offenses' do
+      expect_no_offenses(<<~SOURCE.strip_indent, full_path('ee/app/models/blog_post.rb'))
+        class BlogPost
+        end
+      SOURCE
+    end
+  end
+
+  context 'EE spec file for EE only code' do
+    let(:spec_file_path) { full_path('ee/spec/controllers/foo_spec.rb') }
+
+    it 'registers no offenses' do
+      expect_no_offenses(<<~SOURCE.strip_indent, spec_file_path)
+        describe 'Foo' do
+        end
+      SOURCE
+    end
+
+    context 'there is a duplicate file' do
+      before do
+        allow(File).to receive(:exist?).and_call_original
+
+        allow(File).to receive(:exist?)
+          .with(full_path('ee/spec/controllers/ee/foo_spec.rb'))
+          .and_return(true)
+      end
+
+      it 'marks the describe as offending' do
+        expect_offense(<<~SOURCE.strip_indent, spec_file_path)
+          describe 'Foo' do
+          ^^^^^^^^^^^^^^ Duplicate spec location in `ee/spec/controllers/ee/foo_spec.rb`.
+          end
+        SOURCE
+      end
+    end
+  end
+
+  context 'EE spec file for EE extension' do
+    let(:spec_file_path) { full_path('ee/spec/controllers/ee/foo_spec.rb') }
+
+    it 'registers no offenses' do
+      expect_no_offenses(<<~SOURCE.strip_indent, spec_file_path)
+        describe 'Foo' do
+        end
+      SOURCE
+    end
+
+    context 'there is a duplicate file' do
+      before do
+        allow(File).to receive(:exist?).and_call_original
+
+        allow(File).to receive(:exist?)
+          .with(full_path('ee/spec/controllers/foo_spec.rb'))
+          .and_return(true)
+      end
+
+      it 'marks the describe as offending' do
+        expect_offense(<<~SOURCE.strip_indent, spec_file_path)
+          describe 'Foo' do
+          ^^^^^^^^^^^^^^ Duplicate spec location in `ee/spec/controllers/foo_spec.rb`.
+          end
+        SOURCE
+      end
+    end
+  end
+end