From e8d2c2579383897a1dd7f9debd359abe8ae8373d Mon Sep 17 00:00:00 2001
From: GitLab Bot <gitlab-bot@gitlab.com>
Date: Tue, 20 Jul 2021 09:55:51 +0000
Subject: Add latest changes from gitlab-org/gitlab@14-1-stable-ee

---
 spec/rubocop/code_reuse_helpers_spec.rb            |  75 ++++++-
 .../cop/database/multiple_databases_spec.rb        |  15 ++
 .../cop/gitlab/mark_used_feature_flags_spec.rb     | 233 +++++++++++++++++++++
 .../cop/migration/prevent_index_creation_spec.rb   |  50 +++++
 .../cop/migration/sidekiq_queue_migrate_spec.rb    |  47 +++++
 spec/rubocop/cop/worker_data_consistency_spec.rb   |  50 +++++
 6 files changed, 466 insertions(+), 4 deletions(-)
 create mode 100644 spec/rubocop/cop/database/multiple_databases_spec.rb
 create mode 100644 spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
 create mode 100644 spec/rubocop/cop/migration/prevent_index_creation_spec.rb
 create mode 100644 spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb
 create mode 100644 spec/rubocop/cop/worker_data_consistency_spec.rb

(limited to 'spec/rubocop')

diff --git a/spec/rubocop/code_reuse_helpers_spec.rb b/spec/rubocop/code_reuse_helpers_spec.rb
index 9337df368e3..695c152e3db 100644
--- a/spec/rubocop/code_reuse_helpers_spec.rb
+++ b/spec/rubocop/code_reuse_helpers_spec.rb
@@ -150,6 +150,31 @@ RSpec.describe RuboCop::CodeReuseHelpers do
     end
   end
 
+  describe '#in_graphql_types?' do
+    %w[
+      app/graphql/types
+      ee/app/graphql/ee/types
+      ee/app/graphql/types
+    ].each do |path|
+      it "returns true for a node in #{path}" do
+        node = build_and_parse_source('10', rails_root_join(path, 'foo.rb'))
+
+        expect(cop.in_graphql_types?(node)).to eq(true)
+      end
+    end
+
+    %w[
+      app/graphql/resolvers
+      app/foo
+    ].each do |path|
+      it "returns true for a node in #{path}" do
+        node = build_and_parse_source('10', rails_root_join(path, 'foo.rb'))
+
+        expect(cop.in_graphql_types?(node)).to eq(false)
+      end
+    end
+  end
+
   describe '#in_api?' do
     it 'returns true for a node in the API directory' do
       node = build_and_parse_source('10', rails_root_join('lib', 'api', 'foo.rb'))
@@ -164,25 +189,67 @@ RSpec.describe RuboCop::CodeReuseHelpers do
     end
   end
 
-  describe '#in_directory?' do
+  describe '#in_spec?' do
+    it 'returns true for a node in the spec directory' do
+      node = build_and_parse_source('10', rails_root_join('spec', 'foo.rb'))
+
+      expect(cop.in_spec?(node)).to eq(true)
+    end
+
+    it 'returns true for a node in the ee/spec directory' do
+      node = build_and_parse_source('10', rails_root_join('ee', 'spec', 'foo.rb'))
+
+      expect(cop.in_spec?(node)).to eq(true)
+    end
+
+    it 'returns false for a node outside the spec directory' do
+      node = build_and_parse_source('10', rails_root_join('lib', 'foo.rb'))
+
+      expect(cop.in_spec?(node)).to eq(false)
+    end
+  end
+
+  describe '#in_app_directory?' do
     it 'returns true for a directory in the CE app/ directory' do
       node = build_and_parse_source('10', rails_root_join('app', 'models', 'foo.rb'))
 
-      expect(cop.in_directory?(node, 'models')).to eq(true)
+      expect(cop.in_app_directory?(node, 'models')).to eq(true)
     end
 
     it 'returns true for a directory in the EE app/ directory' do
       node =
         build_and_parse_source('10', rails_root_join('ee', 'app', 'models', 'foo.rb'))
 
-      expect(cop.in_directory?(node, 'models')).to eq(true)
+      expect(cop.in_app_directory?(node, 'models')).to eq(true)
     end
 
     it 'returns false for a directory in the lib/ directory' do
       node =
         build_and_parse_source('10', rails_root_join('lib', 'models', 'foo.rb'))
 
-      expect(cop.in_directory?(node, 'models')).to eq(false)
+      expect(cop.in_app_directory?(node, 'models')).to eq(false)
+    end
+  end
+
+  describe '#in_lib_directory?' do
+    it 'returns true for a directory in the CE lib/ directory' do
+      node = build_and_parse_source('10', rails_root_join('lib', 'models', 'foo.rb'))
+
+      expect(cop.in_lib_directory?(node, 'models')).to eq(true)
+    end
+
+    it 'returns true for a directory in the EE lib/ directory' do
+      node =
+        build_and_parse_source('10', rails_root_join('ee', 'lib', 'models', 'foo.rb'))
+
+      expect(cop.in_lib_directory?(node, 'models')).to eq(true)
+    end
+
+    it 'returns false for a directory in the app/ directory' do
+      node =
+        build_and_parse_source('10', rails_root_join('app', 'models', 'foo.rb'))
+
+      expect(cop.in_lib_directory?(node, 'models')).to eq(false)
     end
   end
 
diff --git a/spec/rubocop/cop/database/multiple_databases_spec.rb b/spec/rubocop/cop/database/multiple_databases_spec.rb
new file mode 100644
index 00000000000..16b916d61db
--- /dev/null
+++ b/spec/rubocop/cop/database/multiple_databases_spec.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/database/multiple_databases'
+
+RSpec.describe RuboCop::Cop::Database::MultipleDatabases do
+  subject(:cop) { described_class.new }
+
+  it 'flags the use of ActiveRecord::Base.connection' do
+    expect_offense(<<~SOURCE)
+    ActiveRecord::Base.connection.inspect
+    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use methods from ActiveRecord::Base, [...]
+    SOURCE
+  end
+end
diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
new file mode 100644
index 00000000000..968cafc57d4
--- /dev/null
+++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb
@@ -0,0 +1,233 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/gitlab/mark_used_feature_flags'
+
+RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do
+  let(:defined_feature_flags) do
+    %w[a_feature_flag foo_hello foo_world baz_experiment_percentage bar_baz]
+  end
+
+  subject(:cop) { described_class.new }
+
+  before do
+    stub_const("#{described_class}::DYNAMIC_FEATURE_FLAGS", [])
+    allow(cop).to receive(:defined_feature_flags).and_return(defined_feature_flags)
+    allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return([])
+  end
+
+  def feature_flag_path(feature_flag_name)
+    File.expand_path("../../../../tmp/feature_flags/#{feature_flag_name}.used", __dir__)
+  end
+
+  shared_examples 'sets flag as used' do |method_call, flags_to_be_set|
+    it 'sets the flag as used' do
+      Array(flags_to_be_set).each do |flag_to_be_set|
+        expect(FileUtils).to receive(:touch).with(feature_flag_path(flag_to_be_set))
+      end
+
+      expect_no_offenses(<<~RUBY)
+        class Foo < ApplicationRecord
+          #{method_call}
+        end
+      RUBY
+    end
+  end
+
+  shared_examples 'does not set any flags as used' do |method_call|
+    it 'sets the flag as used' do
+      expect(FileUtils).not_to receive(:touch)
+
+      expect_no_offenses(method_call)
+    end
+  end
+
+  %w[
+    Feature.enabled?
+    Feature.disabled?
+    push_frontend_feature_flag
+  ].each do |feature_flag_method|
+    context "#{feature_flag_method} method" do
+      context 'a string feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo")|, 'foo'
+      end
+
+      context 'a symbol feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:foo)|, 'foo'
+      end
+
+      context 'an interpolated string feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated symbol feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'a string with a "/" in it' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("bar/baz")|, 'bar_baz'
+      end
+
+      context 'an interpolated string feature flag with a string prefix and suffix' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")|
+      end
+
+      context 'a dynamic string feature flag as a variable' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)|
+      end
+
+      context 'an integer feature flag' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)|
+      end
+    end
+  end
+
+  %w[
+    Feature::Gitaly.enabled?
+    Feature::Gitaly.disabled?
+  ].each do |feature_flag_method|
+    context "#{feature_flag_method} method" do
+      context 'a string feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo")|, 'gitaly_foo'
+      end
+
+      context 'a symbol feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:foo)|, 'gitaly_foo'
+      end
+
+      context 'an interpolated string feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated symbol feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated string feature flag with a string prefix and suffix' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")|
+      end
+
+      context 'a dynamic string feature flag as a variable' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)|
+      end
+
+      context 'an integer feature flag' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)|
+      end
+    end
+  end
+
+  %w[
+    experiment
+    experiment_enabled?
+    push_frontend_experiment
+    Gitlab::Experimentation.active?
+  ].each do |feature_flag_method|
+    context "#{feature_flag_method} method" do
+      context 'a string feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("baz")|, %w[baz baz_experiment_percentage]
+      end
+
+      context 'a symbol feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:baz)|, %w[baz baz_experiment_percentage]
+      end
+
+      context 'an interpolated string feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated symbol feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated string feature flag with a string prefix and suffix' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")|
+      end
+
+      context 'a dynamic string feature flag as a variable' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)|
+      end
+
+      context 'an integer feature flag' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)|
+      end
+    end
+  end
+
+  %w[
+    use_rugged?
+  ].each do |feature_flag_method|
+    context "#{feature_flag_method} method" do
+      context 'a string feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, "baz")|, 'baz'
+      end
+
+      context 'a symbol feature flag' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, :baz)|, 'baz'
+      end
+
+      context 'an interpolated string feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, "foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated symbol feature flag with a string prefix' do
+        include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, :"foo_\#{bar}")|, %w[foo_hello foo_world]
+      end
+
+      context 'an interpolated string feature flag with a string prefix and suffix' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(arg, :"foo_\#{bar}_baz")|
+      end
+
+      context 'a dynamic string feature flag as a variable' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)|
+      end
+
+      context 'an integer feature flag' do
+        include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(arg, 123)|
+      end
+    end
+  end
+
+  describe 'self.limit_feature_flag = :foo' do
+    include_examples 'sets flag as used', 'self.limit_feature_flag = :foo', 'foo'
+  end
+
+  describe 'FEATURE_FLAG = :foo' do
+    include_examples 'sets flag as used', 'FEATURE_FLAG = :foo', 'foo'
+  end
+
+  describe 'Worker `data_consistency` method' do
+    include_examples 'sets flag as used', 'data_consistency :delayed, feature_flag: :foo', 'foo'
+    include_examples 'does not set any flags as used', 'data_consistency :delayed'
+  end
+
+  describe 'Worker `deduplicate` method' do
+    include_examples 'sets flag as used', 'deduplicate :delayed, feature_flag: :foo', 'foo'
+    include_examples 'does not set any flags as used', 'deduplicate :delayed'
+  end
+
+  describe 'GraphQL `field` method' do
+    before do
+      allow(cop).to receive(:in_graphql_types?).and_return(true)
+    end
+
+    include_examples 'sets flag as used', 'field :runners, Types::Ci::RunnerType.connection_type, null: true, feature_flag: :foo', 'foo'
+    include_examples 'sets flag as used', 'field :runners, null: true, feature_flag: :foo', 'foo'
+    include_examples 'does not set any flags as used', 'field :solution'
+    include_examples 'does not set any flags as used', 'field :runners, Types::Ci::RunnerType.connection_type'
+    include_examples 'does not set any flags as used', 'field :runners, Types::Ci::RunnerType.connection_type, null: true, description: "hello world"'
+    include_examples 'does not set any flags as used', 'field :solution, type: GraphQL::STRING_TYPE, null: true, description: "URL to the vulnerabilitys details page."'
+  end
+
+  describe "tracking of usage data metrics known events happens at the beginning of inspection" do
+    let(:usage_data_counters_known_event_feature_flags) { ['an_event_feature_flag'] }
+
+    before do
+      allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return(usage_data_counters_known_event_feature_flags)
+    end
+
+    include_examples 'sets flag as used', "FEATURE_FLAG = :foo", %w[foo an_event_feature_flag]
+  end
+end
diff --git a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
new file mode 100644
index 00000000000..a3965f54bbd
--- /dev/null
+++ b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/migration/prevent_index_creation'
+
+RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
+  subject(:cop) { described_class.new }
+
+  context 'when in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    context 'when adding an index to a forbidden table' do
+      it 'registers an offense when add_index is used' do
+        expect_offense(<<~RUBY)
+          def change
+            add_index :ci_builds, :protected
+            ^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+          end
+        RUBY
+      end
+
+      it 'registers an offense when add_concurrent_index is used' do
+        expect_offense(<<~RUBY)
+          def change
+            add_concurrent_index :ci_builds, :protected
+            ^^^^^^^^^^^^^^^^^^^^ Adding new index to ci_builds is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
+          end
+        RUBY
+      end
+    end
+
+    context 'when adding an index to a regular table' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          def change
+            add_index :ci_pipelines, :locked
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'when outside of migration' do
+    it 'does not register an offense' do
+      expect_no_offenses('def change; add_index :table, :column; end')
+    end
+  end
+end
diff --git a/spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb b/spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb
new file mode 100644
index 00000000000..499351b3585
--- /dev/null
+++ b/spec/rubocop/cop/migration/sidekiq_queue_migrate_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/migration/sidekiq_queue_migrate'
+
+RSpec.describe RuboCop::Cop::Migration::SidekiqQueueMigrate do
+  subject(:cop) { described_class.new }
+
+  def source(meth = 'change')
+    "def #{meth}; sidekiq_queue_migrate 'queue', to: 'new_queue'; end"
+  end
+
+  context 'when in a regular migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+      allow(cop).to receive(:in_post_deployment_migration?).and_return(false)
+    end
+
+    %w(up down change any_other_method).each do |method_name|
+      it "registers an offense when sidekiq_queue_migrate is used in ##{method_name}" do
+        expect_offense(<<~RUBY)
+          def #{method_name}
+            sidekiq_queue_migrate 'queue', to: 'new_queue'
+            ^^^^^^^^^^^^^^^^^^^^^ `sidekiq_queue_migrate` must only be used in post-deployment migrations
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'when in a post-deployment migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+      allow(cop).to receive(:in_post_deployment_migration?).and_return(true)
+    end
+
+    it 'registers no offense' do
+      expect_no_offenses(source)
+    end
+  end
+
+  context 'when outside of a migration' do
+    it 'registers no offense' do
+      expect_no_offenses(source)
+    end
+  end
+end
diff --git a/spec/rubocop/cop/worker_data_consistency_spec.rb b/spec/rubocop/cop/worker_data_consistency_spec.rb
new file mode 100644
index 00000000000..5fa42bf2b87
--- /dev/null
+++ b/spec/rubocop/cop/worker_data_consistency_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../rubocop/cop/worker_data_consistency'
+
+RSpec.describe RuboCop::Cop::WorkerDataConsistency do
+  subject(:cop) { described_class.new }
+
+  before do
+    allow(cop)
+      .to receive(:in_worker?)
+      .and_return(true)
+  end
+
+  it 'adds an offense when not defining data_consistency' do
+    expect_offense(<<~CODE)
+      class SomeWorker
+      ^^^^^^^^^^^^^^^^ Should define data_consistency expectation.[...]
+        include ApplicationWorker
+
+        queue_namespace :pipeline_hooks
+        feature_category :continuous_integration
+        urgency :high
+      end
+    CODE
+  end
+
+  it 'adds no offense when defining data_consistency' do
+    expect_no_offenses(<<~CODE)
+      class SomeWorker
+        include ApplicationWorker
+
+        queue_namespace :pipeline_hooks
+        feature_category :continuous_integration
+        data_consistency :delayed
+        urgency :high
+      end
+    CODE
+  end
+
+  it 'adds no offense when worker is not an ApplicationWorker' do
+    expect_no_offenses(<<~CODE)
+      class SomeWorker
+        queue_namespace :pipeline_hooks
+        feature_category :continuous_integration
+        urgency :high
+      end
+    CODE
+  end
+end
-- 
cgit v1.2.3