Merge branch 'zj-improve-gitaly-pb' into 'master'

Show what RPC is called in the performance bar

See merge request gitlab-org/gitlab-ce!23140

changelogs/unreleased/zj-improve-gitaly-pb.yml

+---
+title: Show what RPC is called in the performance bar
+merge_request: 23140
+author:
+type: other

lib/gitlab/git/blob.rb

 # frozen_string_literal: true
 
-# Gitaly note: JV: seems to be completely migrated (behind feature flags).
-
 module Gitlab
   module Git
     class Blob

lib/gitlab/git/repository.rb

@@ -885,12 +885,6 @@ module Gitlab
         Gitlab::GitalyClient::ConflictsService.new(self, our_commit_oid, their_commit_oid)
       end
 
-      def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)
-        wrapped_gitaly_errors do
-          Gitlab::GitalyClient.migrate(method, status: status, &block)
-        end
-      end
-
       def clean_stale_repository_files
         wrapped_gitaly_errors do
           gitaly_repository_client.cleanup if exists?

lib/gitlab/gitaly_client.rb

@@ -9,11 +9,6 @@ require 'grpc/health/v1/health_services_pb'
 module Gitlab
   module GitalyClient
     include Gitlab::Metrics::Methods
-    module MigrationStatus
-      DISABLED = 1
-      OPT_IN = 2
-      OPT_OUT = 3
-    end
 
     class TooManyInvocationsError < StandardError
       attr_reader :call_site, :invocation_count, :max_call_stack
@@ -31,7 +26,7 @@ module Gitlab
       end
     end
 
-    SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze
+    SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'
     MAXIMUM_GITALY_CALLS = 35
     CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
 
@@ -43,11 +38,6 @@ module Gitlab
 
     self.query_time = 0
 
-    define_histogram :gitaly_migrate_call_duration_seconds do
-      docstring "Gitaly migration call execution timings"
-      base_labels gitaly_enabled: nil, feature: nil
-    end
-
     define_histogram :gitaly_controller_action_duration_seconds do
       docstring "Gitaly endpoint histogram by controller and action combination"
       base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil)
@@ -126,7 +116,6 @@ module Gitlab
     def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil)
       start = Gitlab::Metrics::System.monotonic_time
       request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {}
-      @current_call_id ||= SecureRandom.uuid
 
       enforce_gitaly_request_limits(:call)
 
@@ -145,9 +134,7 @@ module Gitlab
         current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s),
         duration)
 
-      add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash)
-
-      @current_call_id = nil
+      add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc)
     end
 
     def self.handle_grpc_unavailable!(ex)
@@ -222,7 +209,7 @@ module Gitlab
       result
     end
 
-    SERVER_FEATURE_FLAGS = %w[gogit_findcommit].freeze
+    SERVER_FEATURE_FLAGS = %w[].freeze
 
     def self.server_feature_flags
       SERVER_FEATURE_FLAGS.map do |f|
@@ -237,82 +224,8 @@ module Gitlab
       params['gitaly_token'].presence || Gitlab.config.gitaly['token']
     end
 
-    # Evaluates whether a feature toggle is on or off
-    def self.feature_enabled?(feature_name, status: MigrationStatus::OPT_IN)
-      # Disabled features are always off!
-      return false if status == MigrationStatus::DISABLED
-
-      feature = Feature.get("gitaly_#{feature_name}")
-
-      # If the feature has been set, always evaluate
-      if Feature.persisted?(feature)
-        if feature.percentage_of_time_value > 0
-          # Probabilistically enable this feature
-          return Random.rand() * 100 < feature.percentage_of_time_value
-        end
-
-        return feature.enabled?
-      end
-
-      # If the feature has not been set, the default depends
-      # on it's status
-      case status
-      when MigrationStatus::OPT_OUT
-        true
-      when MigrationStatus::OPT_IN
-        opt_into_all_features? && !explicit_opt_in_required.include?(feature_name)
-      else
-        false
-      end
-    rescue => ex
-      # During application startup feature lookups in SQL can fail
-      Rails.logger.warn "exception while checking Gitaly feature status for #{feature_name}: #{ex}"
-      false
-    end
-
-    # We have a mechanism to let GitLab automatically opt in to all Gitaly
-    # features. We want to be able to exclude some features from automatic
-    # opt-in. This function has an override in EE.
-    def self.explicit_opt_in_required
-      []
-    end
-
-    # opt_into_all_features? returns true when the current environment
-    # is one in which we opt into features automatically
-    def self.opt_into_all_features?
-      Rails.env.development? || ENV["GITALY_FEATURE_DEFAULT_ON"] == "1"
-    end
-    private_class_method :opt_into_all_features?
-
-    def self.migrate(feature, status: MigrationStatus::OPT_IN)
-      # Enforce limits at both the `migrate` and `call` sites to ensure that
-      # problems are not hidden by a feature being disabled
-      enforce_gitaly_request_limits(:migrate)
-
-      is_enabled  = feature_enabled?(feature, status: status)
-      metric_name = feature.to_s
-      metric_name += "_gitaly" if is_enabled
-
-      Gitlab::Metrics.measure(metric_name) do
-        # Some migrate calls wrap other migrate calls
-        allow_n_plus_1_calls do
-          feature_stack = Thread.current[:gitaly_feature_stack] ||= []
-          feature_stack.unshift(feature)
-          begin
-            start = Gitlab::Metrics::System.monotonic_time
-            @current_call_id = SecureRandom.uuid
-            call_details = { id: @current_call_id }
-            yield is_enabled
-          ensure
-            total_time = Gitlab::Metrics::System.monotonic_time - start
-            gitaly_migrate_call_duration_seconds.observe({ gitaly_enabled: is_enabled, feature: feature }, total_time)
-            feature_stack.shift
-            Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty?
-
-            add_call_details(call_details.merge(feature: feature, duration: total_time))
-          end
-        end
-      end
+    def self.feature_enabled?(feature_name)
+      Feature.enabled?("gitaly_#{feature_name}")
     end
 
     # Ensures that Gitaly is not being abuse through n+1 misuse etc
@@ -368,38 +281,20 @@ module Gitlab
     end
     private_class_method :decrement_call_count
 
-    # Returns an estimate of the number of Gitaly calls made for this
-    # request
+    # Returns the of the number of Gitaly calls made for this request
     def self.get_request_count
-      return 0 unless Gitlab::SafeRequestStore.active?
-
-      gitaly_migrate_count = get_call_count("gitaly_migrate_actual")
-      gitaly_call_count = get_call_count("gitaly_call_actual")
-
-      # Using the maximum of migrate and call_count will provide an
-      # indicator of how many Gitaly calls will be made, even
-      # before a feature is enabled. This provides us with a single
-      # metric, but not an exact number, but this tradeoff is acceptable
-      if gitaly_migrate_count > gitaly_call_count
-        gitaly_migrate_count
-      else
-        gitaly_call_count
-      end
+      get_call_count("gitaly_call_actual")
     end
 
     def self.reset_counts
       return unless Gitlab::SafeRequestStore.active?
 
-      %w[migrate call].each do |call_site|
-        Gitlab::SafeRequestStore["gitaly_#{call_site}_actual"] = 0
-        Gitlab::SafeRequestStore["gitaly_#{call_site}_permitted"] = 0
-      end
+      Gitlab::SafeRequestStore["gitaly_call_actual"] = 0
+      Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0
     end
 
     def self.add_call_details(details)
-      id = details.delete(:id)
-
-      return unless id && Gitlab::SafeRequestStore[:peek_enabled]
+      return unless Gitlab::SafeRequestStore[:peek_enabled]
 
       Gitlab::SafeRequestStore['gitaly_call_details'] ||= {}
       Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {}

spec/features/projects/wiki/user_updates_wiki_page_spec.rb

 require 'spec_helper'
 
 describe 'User updates wiki page' do
-  shared_examples 'wiki page user update' do
   let(:user) { create(:user) }
 
   before do
@@ -16,7 +15,7 @@ describe 'User updates wiki page' do
     end
 
     context 'in a user namespace' do
-        let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
+      let(:project) { create(:project, :wiki_repo) }
 
       it 'redirects back to the home edit page' do
         page.within(:css, '.wiki-form .form-actions') do
@@ -72,7 +71,7 @@ describe 'User updates wiki page' do
     end
 
     context 'in a user namespace' do
-        let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
+      let(:project) { create(:project, :wiki_repo) }
 
       it 'updates a page' do
         # Commit message field should have correct value.
@@ -151,7 +150,7 @@ describe 'User updates wiki page' do
   end
 
   context 'when the page is in a subdir' do
-      let!(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
+    let!(:project) { create(:project, :wiki_repo) }
     let(:project_wiki) { create(:project_wiki, project: project, user: project.creator) }
     let(:page_name) { 'page_name' }
     let(:page_dir) { "foo/bar/#{page_name}" }
@@ -161,7 +160,7 @@ describe 'User updates wiki page' do
       visit(project_wiki_edit_path(project, wiki_page))
     end
 
-      it 'moves the page to the root folder', :skip_gitaly_mock do
+    it 'moves the page to the root folder' do
       fill_in(:wiki_title, with: "/#{page_name}")
 
       click_button('Save changes')
@@ -221,13 +220,4 @@ describe 'User updates wiki page' do
 
     it_behaves_like 'wiki file attachments'
   end
-  end
-
-  context 'when Gitaly is enabled' do
-    it_behaves_like 'wiki page user update'
-  end
-
-  context 'when Gitaly is disabled', :skip_gitaly_mock do
-    it_behaves_like 'wiki page user update'
-  end
 end

spec/features/projects/wiki/user_views_wiki_page_spec.rb

 require 'spec_helper'
 
 describe 'User views a wiki page' do
-  shared_examples 'wiki page user view' do
   include WikiHelpers
 
   let(:user) { create(:user) }
@@ -162,13 +161,4 @@ describe 'User views a wiki page' do
 
     expect(page).to have_content('Home · Create Page')
   end
-  end
-
-  context 'when Gitaly is enabled' do
-    it_behaves_like 'wiki page user view'
-  end
-
-  context 'when Gitaly is disabled', :skip_gitaly_mock do
-    it_behaves_like 'wiki page user view'
-  end
 end

spec/lib/extracts_path_spec.rb

@@ -205,7 +205,6 @@ describe ExtractsPath do
   end
 
   describe '#lfs_blob_ids' do
-    shared_examples '#lfs_blob_ids' do
     let(:tag) { @project.repository.add_tag(@project.owner, 'my-annotated-tag', 'master', 'test tag') }
     let(:ref) { tag.target }
     let(:params) { { ref: ref, path: 'README.md' } }
@@ -220,13 +219,4 @@ describe ExtractsPath do
       expect(lfs_blob_ids).to eq([])
     end
   end
-
-    context 'when gitaly is enabled' do
-      it_behaves_like '#lfs_blob_ids'
-    end
-
-    context 'when gitaly is disabled', :skip_gitaly_mock do
-      it_behaves_like '#lfs_blob_ids'
-    end
-  end
 end

spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb

@@ -37,17 +37,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
     let(:stub_path) { '.gitignore' }
   end
 
-  shared_examples 'initializes a DiffCollection' do
   it 'returns a valid instance of a DiffCollection' do
     expect(diff_files).to be_a(Gitlab::Git::DiffCollection)
   end
-  end
-
-  context 'with Gitaly disabled', :disable_gitaly do
-    it_behaves_like 'initializes a DiffCollection'
-  end
-
-  context 'with Gitaly enabled' do
-    it_behaves_like 'initializes a DiffCollection'
-  end
 end

spec/lib/gitlab/git/blob_spec.rb

@@ -128,7 +128,7 @@ describe Gitlab::Git::Blob, :seed_helper do
     end
   end
 
-  shared_examples 'finding blobs by ID' do
+  describe '.raw' do
     let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) }
     let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) }
 
@@ -166,16 +166,6 @@ describe Gitlab::Git::Blob, :seed_helper do
     end
   end
 
-  describe '.raw' do
-    context 'when the blob_raw Gitaly feature is enabled' do
-      it_behaves_like 'finding blobs by ID'
-    end
-
-    context 'when the blob_raw Gitaly feature is disabled', :skip_gitaly_mock do
-      it_behaves_like 'finding blobs by ID'
-    end
-  end
-
   describe '.batch' do
     let(:blob_references) do
       [

spec/lib/gitlab/git/commit_spec.rb

@@ -183,7 +183,6 @@ describe Gitlab::Git::Commit, :seed_helper do
       end
     end
 
-    shared_examples '.where' do
     context 'path is empty string' do
       subject do
         commits = described_class.where(
@@ -279,15 +278,6 @@ describe Gitlab::Git::Commit, :seed_helper do
       it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") }
       it { is_expected.not_to include(SeedRepo::Commit::ID) }
     end
-    end
-
-    describe '.where with gitaly' do
-      it_should_behave_like '.where'
-    end
-
-    describe '.where without gitaly', :skip_gitaly_mock do
-      it_should_behave_like '.where'
-    end
 
     describe '.between' do
       subject do
@@ -508,7 +498,7 @@ describe Gitlab::Git::Commit, :seed_helper do
     end
   end
 
-  shared_examples '#stats' do
+  describe '#stats' do
     subject { commit.stats }
 
     describe '#additions' do
@@ -527,14 +517,6 @@ describe Gitlab::Git::Commit, :seed_helper do
     end
   end
 
-  describe '#stats with gitaly on' do
-    it_should_behave_like '#stats'
-  end
-
-  describe '#stats with gitaly disabled', :skip_gitaly_mock do
-    it_should_behave_like '#stats'
-  end
-
   describe '#has_zero_stats?' do
     it { expect(commit.has_zero_stats?).to eq(false) }
   end
@@ -577,28 +559,18 @@ describe Gitlab::Git::Commit, :seed_helper do
       commit_ids.map { |id| described_class.get_message(repository, id) }
     end
 
-    shared_examples 'getting commit messages' do
     it 'gets commit messages' do
       expect(subject).to contain_exactly(
         "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n",
         "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"
       )
     end
-    end
-
-    context 'when Gitaly commit_messages feature is enabled' do
-      it_behaves_like 'getting commit messages'
 
     it 'gets messages in one batch', :request_store do
       expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
     end
   end
 
-    context 'when Gitaly commit_messages feature is disabled', :disable_gitaly do
-      it_behaves_like 'getting commit messages'
-    end
-  end
-
   def sample_commit_hash
     {
       author_email: "dmitriy.zaporozhets@gmail.com",

spec/lib/gitlab/git/tag_spec.rb

@@ -3,7 +3,7 @@ require "spec_helper"
 describe Gitlab::Git::Tag, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
 
-  shared_examples 'Gitlab::Git::Repository#tags' do
+  describe '#tags' do
     describe 'first tag' do
       let(:tag) { repository.tags.first }
 
@@ -25,14 +25,6 @@ describe Gitlab::Git::Tag, :seed_helper do
     it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) }
   end
 
-  context 'when Gitaly tags feature is enabled' do
-    it_behaves_like 'Gitlab::Git::Repository#tags'
-  end
-
-  context 'when Gitaly tags feature is disabled', :skip_gitaly_mock do
-    it_behaves_like 'Gitlab::Git::Repository#tags'
-  end
-
   describe '.get_message' do
     let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] }
 
@@ -40,26 +32,16 @@ describe Gitlab::Git::Tag, :seed_helper do
       tag_ids.map { |id| described_class.get_message(repository, id) }
     end
 
-    shared_examples 'getting tag messages' do
     it 'gets tag messages' do
       expect(subject[0]).to eq("Release\n")
       expect(subject[1]).to eq("Version 1.1.0\n")
     end
-    end
-
-    context 'when Gitaly tag_messages feature is enabled' do
-      it_behaves_like 'getting tag messages'
 
     it 'gets messages in one batch', :request_store do
       expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
     end
   end
 
-    context 'when Gitaly tag_messages feature is disabled', :disable_gitaly do
-      it_behaves_like 'getting tag messages'
-    end
-  end
-
   describe 'tag into from Gitaly tag' do
     context 'message_size != message.size' do
       let(:gitaly_tag) { build(:gitaly_tag, message: ''.b, message_size: message_size) }

spec/lib/gitlab/git/tree_spec.rb

@@ -80,18 +80,8 @@ describe Gitlab::Git::Tree, :seed_helper do
   end
 
   describe '#where' do
-    shared_examples '#where' do
     it 'returns an empty array when called with an invalid ref' do
       expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([])
     end
   end
-
-    context 'with gitaly' do
-      it_behaves_like '#where'
-    end
-
-    context 'without gitaly', :skip_gitaly_mock do
-      it_behaves_like '#where'
-    end
-  end
 end

spec/lib/gitlab/gitaly_client_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 # We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want
 # those stubs while testing the GitalyClient itself.
-describe Gitlab::GitalyClient, skip_gitaly_mock: true do
+describe Gitlab::GitalyClient do
   describe '.stub_class' do
     it 'returns the gRPC health check stub' do
       expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub)
@@ -191,105 +191,16 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do
     let(:feature_name) { 'my_feature' }
     let(:real_feature_name) { "gitaly_#{feature_name}" }
 
-    context 'when Gitaly is disabled' do
     before do
-        allow(described_class).to receive(:enabled?).and_return(false)
+      allow(Feature).to receive(:enabled?).and_return(false)
     end
 
     it 'returns false' do
+      expect(Feature).to receive(:enabled?).with(real_feature_name)
       expect(described_class.feature_enabled?(feature_name)).to be(false)
     end
   end
 
-    context 'when the feature status is DISABLED' do
-      let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::DISABLED }
-
-      it 'returns false' do
-        expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-      end
-    end
-
-    context 'when the feature_status is OPT_IN' do
-      let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_IN }
-
-      context "when the feature flag hasn't been set" do
-        it 'returns false' do
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-        end
-      end
-
-      context "when the feature flag is set to disable" do
-        before do
-          Feature.get(real_feature_name).disable
-        end
-
-        it 'returns false' do
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-        end
-      end
-
-      context "when the feature flag is set to enable" do
-        before do
-          Feature.get(real_feature_name).enable
-        end
-
-        it 'returns true' do
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true)
-        end
-      end
-
-      context "when the feature flag is set to a percentage of time" do
-        before do
-          Feature.get(real_feature_name).enable_percentage_of_time(70)
-        end
-
-        it 'bases the result on pseudo-random numbers' do
-          expect(Random).to receive(:rand).and_return(0.3)
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true)
-
-          expect(Random).to receive(:rand).and_return(0.8)
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-        end
-      end
-
-      context "when a feature is not persisted" do
-        it 'returns false when opt_into_all_features is off' do
-          allow(Feature).to receive(:persisted?).and_return(false)
-          allow(described_class).to receive(:opt_into_all_features?).and_return(false)
-
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-        end
-
-        it 'returns true when the override is on' do
-          allow(Feature).to receive(:persisted?).and_return(false)
-          allow(described_class).to receive(:opt_into_all_features?).and_return(true)
-
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true)
-        end
-      end
-    end
-
-    context 'when the feature_status is OPT_OUT' do
-      let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_OUT }
-
-      context "when the feature flag hasn't been set" do
-        it 'returns true' do
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true)
-        end
-      end
-
-      context "when the feature flag is set to disable" do
-        before do
-          Feature.get(real_feature_name).disable
-        end
-
-        it 'returns false' do
-          expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false)
-        end
-      end
-    end
-  end
-
   describe 'timeouts' do
     context 'with default values' do
       before do

spec/models/merge_request_spec.rb

@@ -30,7 +30,6 @@ describe MergeRequest do
   end
 
   describe '#squash_in_progress?' do
-    shared_examples 'checking whether a squash is in progress' do
     let(:repo_path) do
       Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         subject.source_project.repository.path
@@ -66,15 +65,6 @@ describe MergeRequest do
     end
   end
 
-    context 'when Gitaly squash_in_progress is enabled' do
-      it_behaves_like 'checking whether a squash is in progress'
-    end
-
-    context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do
-      it_behaves_like 'checking whether a squash is in progress'
-    end
-  end
-
   describe '#squash?' do
     let(:merge_request) { build(:merge_request, squash: squash) }
     subject { merge_request.squash? }
@@ -2587,14 +2577,6 @@ describe MergeRequest do
         expect(subject.rebase_in_progress?).to be_falsey
       end
     end
-
-    context 'when Gitaly rebase_in_progress is enabled' do
-      it_behaves_like 'checking whether a rebase is in progress'
-    end
-
-    context 'when Gitaly rebase_in_progress is enabled', :disable_gitaly do
-      it_behaves_like 'checking whether a rebase is in progress'
-    end
   end
 
   describe '#allow_collaboration' do

spec/models/project_wiki_spec.rb

@@ -130,7 +130,6 @@ describe ProjectWiki do
   end
 
   describe "#find_page" do
-    shared_examples 'finding a wiki page' do
     before do
       create_page("index page", "This is an awesome Gollum Wiki")
     end
@@ -181,15 +180,6 @@ describe ProjectWiki do
     end
   end
 
-    context 'when Gitaly wiki_find_page is enabled' do
-      it_behaves_like 'finding a wiki page'
-    end
-
-    context 'when Gitaly wiki_find_page is disabled', :skip_gitaly_mock do
-      it_behaves_like 'finding a wiki page'
-    end
-  end
-
   describe '#find_sidebar' do
     before do
       create_page(described_class::SIDEBAR, 'This is an awesome Sidebar')
@@ -207,7 +197,6 @@ describe ProjectWiki do
   end
 
   describe '#find_file' do
-    shared_examples 'finding a wiki file' do
     let(:image) { File.open(Rails.root.join('spec', 'fixtures', 'big-image.png')) }
 
     before do
@@ -242,17 +231,7 @@ describe ProjectWiki do
     end
   end
 
-    context 'when Gitaly wiki_find_file is enabled' do
-      it_behaves_like 'finding a wiki file'
-    end
-
-    context 'when Gitaly wiki_find_file is disabled', :skip_gitaly_mock do
-      it_behaves_like 'finding a wiki file'
-    end
-  end
-
   describe "#create_page" do
-    shared_examples 'creating a wiki page' do
     after do
       destroy_page(subject.pages.first.page)
     end
@@ -295,15 +274,6 @@ describe ProjectWiki do
     end
   end
 
-    context 'when Gitaly wiki_write_page is enabled' do
-      it_behaves_like 'creating a wiki page'
-    end
-
-    context 'when Gitaly wiki_write_page is disabled', :skip_gitaly_mock do
-      it_behaves_like 'creating a wiki page'
-    end
-  end
-
   describe "#update_page" do
     before do
       create_page("update-page", "some content")
@@ -351,7 +321,6 @@ describe ProjectWiki do
   end
 
   describe "#delete_page" do
-    shared_examples 'deleting a wiki page' do
     before do
       create_page("index", "some content")
       @page = subject.wiki.page(title: "index")
@@ -380,15 +349,6 @@ describe ProjectWiki do
     end
   end
 
-    context 'when Gitaly wiki_delete_page is enabled' do
-      it_behaves_like 'deleting a wiki page'
-    end
-
-    context 'when Gitaly wiki_delete_page is disabled', :skip_gitaly_mock do
-      it_behaves_like 'deleting a wiki page'
-    end
-  end
-
   describe '#create_repo!' do
     let(:project) { create(:project) }
 

spec/models/repository_spec.rb

@@ -38,7 +38,6 @@ describe Repository do
   end
 
   describe '#branch_names_contains' do
-    shared_examples '#branch_names_contains' do
     set(:project) { create(:project, :repository) }
     let(:repository) { project.repository }
 
@@ -57,32 +56,13 @@ describe Repository do
     end
   end
 
-    context 'when gitaly is enabled' do
-      it_behaves_like '#branch_names_contains'
-    end
-
-    context 'when gitaly is disabled', :skip_gitaly_mock do
-      it_behaves_like '#branch_names_contains'
-    end
-  end
-
   describe '#tag_names_contains' do
-    shared_examples '#tag_names_contains' do
     subject { repository.tag_names_contains(sample_commit.id) }
 
     it { is_expected.to include('v1.1.0') }
     it { is_expected.not_to include('v1.0.0') }
   end
 
-    context 'when gitaly is enabled' do
-      it_behaves_like '#tag_names_contains'
-    end
-
-    context 'when gitaly is enabled', :skip_gitaly_mock do
-      it_behaves_like '#tag_names_contains'
-    end
-  end
-
   describe 'tags_sorted_by' do
     context 'name_desc' do
       subject { repository.tags_sorted_by('name_desc').map(&:name) }
@@ -238,7 +218,6 @@ describe Repository do
   end
 
   describe '#last_commit_for_path' do
-    shared_examples 'getting last commit for path' do
     subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id }
 
     it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') }
@@ -252,17 +231,7 @@ describe Repository do
     end
   end
 
-    context 'when Gitaly feature last_commit_for_path is enabled' do
-      it_behaves_like 'getting last commit for path'
-    end
-
-    context 'when Gitaly feature last_commit_for_path is disabled', :skip_gitaly_mock do
-      it_behaves_like 'getting last commit for path'
-    end
-  end
-
   describe '#last_commit_id_for_path' do
-    shared_examples 'getting last commit ID for path' do
     subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') }
 
     it "returns last commit id for a given path" do
@@ -286,15 +255,6 @@ describe Repository do
     end
   end
 
-    context 'when Gitaly feature last_commit_for_path is enabled' do
-      it_behaves_like 'getting last commit ID for path'
-    end
-
-    context 'when Gitaly feature last_commit_for_path is disabled', :skip_gitaly_mock do
-      it_behaves_like 'getting last commit ID for path'
-    end
-  end
-
   describe '#commits' do
     context 'when neither the all flag nor a ref are specified' do
       it 'returns every commit from default branch' do
@@ -374,8 +334,6 @@ describe Repository do
 
   describe '#commits_by' do
     set(:project) { create(:project, :repository) }
-
-    shared_examples 'batch commits fetching' do
     let(:oids) { TestEnv::BRANCH_SHA.values }
 
     subject { project.repository.commits_by(oids: oids) }
@@ -411,17 +369,7 @@ describe Repository do
     end
   end
 
-    context 'when Gitaly list_commits_by_oid is enabled' do
-      it_behaves_like 'batch commits fetching'
-    end
-
-    context 'when Gitaly list_commits_by_oid is enabled', :disable_gitaly do
-      it_behaves_like 'batch commits fetching'
-    end
-  end
-
   describe '#find_commits_by_message' do
-    shared_examples 'finding commits by message' do
     it 'returns commits with messages containing a given string' do
       commit_ids = repository.find_commits_by_message('submodule').map(&:id)
 
@@ -438,15 +386,6 @@ describe Repository do
 
       expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
     end
-    end
-
-    context 'when Gitaly commits_by_message feature is enabled' do
-      it_behaves_like 'finding commits by message'
-    end
-
-    context 'when Gitaly commits_by_message feature is disabled', :skip_gitaly_mock do
-      it_behaves_like 'finding commits by message'
-    end
 
     describe 'when storage is broken', :broken_storage do
       it 'should raise a storage error' do
@@ -1328,10 +1267,8 @@ describe Repository do
 
   describe '#merge' do
     let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) }
-
     let(:message) { 'Test \r\n\r\n message' }
 
-    shared_examples '#merge' do
     it 'merges the code and returns the commit id' do
       expect(merge_commit).to be_present
       expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
@@ -1348,15 +1285,6 @@ describe Repository do
 
       expect(repository.commit(merge_commit_id).message).to eq(message.delete("\r"))
     end
-    end
-
-    context 'with gitaly' do
-      it_behaves_like '#merge'
-    end
-
-    context 'without gitaly', :skip_gitaly_mock do
-      it_behaves_like '#merge'
-    end
 
     def merge(repository, user, merge_request, message)
       repository.merge(user, merge_request.diff_head_sha, merge_request, message)
@@ -1392,7 +1320,6 @@ describe Repository do
   end
 
   describe '#revert' do
-    shared_examples 'reverting a commit' do
     let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') }
     let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
     let(:message) { 'revert message' }
@@ -1428,17 +1355,7 @@ describe Repository do
     end
   end
 
-    context 'when Gitaly revert feature is enabled' do
-      it_behaves_like 'reverting a commit'
-    end
-
-    context 'when Gitaly revert feature is disabled', :disable_gitaly do
-      it_behaves_like 'reverting a commit'
-    end
-  end
-
   describe '#cherry_pick' do
-    shared_examples 'cherry-picking a commit' do
     let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') }
     let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
     let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') }
@@ -1477,15 +1394,6 @@ describe Repository do
     end
   end
 
-    context 'when Gitaly cherry_pick feature is enabled' do
-      it_behaves_like 'cherry-picking a commit'
-    end
-
-    context 'when Gitaly cherry_pick feature is disabled', :disable_gitaly do
-      it_behaves_like 'cherry-picking a commit'
-    end
-  end
-
   describe '#before_delete' do
     describe 'when a repository does not exist' do
       before do
@@ -2190,7 +2098,6 @@ describe Repository do
     let(:commit) { repository.commit }
     let(:ancestor) { commit.parents.first }
 
-    shared_examples '#ancestor?' do
     it 'it is an ancestor' do
       expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
     end
@@ -2211,15 +2118,6 @@ describe Repository do
     end
   end
 
-    context 'with Gitaly enabled' do
-      it_behaves_like('#ancestor?')
-    end
-
-    context 'with Gitaly disabled', :skip_gitaly_mock do
-      it_behaves_like('#ancestor?')
-    end
-  end
-
   describe '#archive_metadata' do
     let(:ref) { 'master' }
     let(:storage_path) { '/tmp' }

spec/models/wiki_page_spec.rb

@@ -200,7 +200,6 @@ describe WikiPage do
   end
 
   describe '#create' do
-    shared_examples 'create method' do
     context 'with valid attributes' do
       it 'raises an error if a page with the same path already exists' do
         create_page('New Page', 'content')
@@ -222,17 +221,7 @@ describe WikiPage do
     end
   end
 
-    context 'when Gitaly is enabled' do
-      it_behaves_like 'create method'
-    end
-
-    context 'when Gitaly is disabled', :skip_gitaly_mock do
-      it_behaves_like 'create method'
-    end
-  end
-
   describe "#update" do
-    shared_examples 'update method' do
     before do
       create_page("Update", "content")
       @page = wiki.find_page("Update")
@@ -331,7 +320,7 @@ describe WikiPage do
           @page = wiki.find_page('foo/Existing Page')
         end
 
-          it 'moves the page to the root folder if the title is preceded by /', :skip_gitaly_mock do
+        it 'moves the page to the root folder if the title is preceded by /' do
           expect(@page.slug).to eq 'foo/Existing-Page'
           expect(@page.update(title: '/Existing Page', content: 'new_content')).to be_truthy
           expect(@page.slug).to eq 'Existing-Page'
@@ -368,15 +357,6 @@ describe WikiPage do
     end
   end
 
-    context 'when Gitaly is enabled' do
-      it_behaves_like 'update method'
-    end
-
-    context 'when Gitaly is disabled', :skip_gitaly_mock do
-      it_behaves_like 'update method'
-    end
-  end
-
   describe "#destroy" do
     before do
       create_page("Delete Page", "content")
@@ -394,7 +374,6 @@ describe WikiPage do
   end
 
   describe "#versions" do
-    shared_examples 'wiki page versions' do
     let(:page) { wiki.find_page("Update") }
 
     before do
@@ -416,15 +395,6 @@ describe WikiPage do
     end
   end
 
-    context 'when Gitaly is enabled' do
-      it_behaves_like 'wiki page versions'
-    end
-
-    context 'when Gitaly is disabled', :disable_gitaly do
-      it_behaves_like 'wiki page versions'
-    end
-  end
-
   describe "#title" do
     before do
       create_page("Title", "content")
@@ -555,7 +525,6 @@ describe WikiPage do
   end
 
   describe '#formatted_content' do
-    shared_examples 'fetching page formatted content' do
     it 'returns processed content of the page' do
       subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" })
       page = wiki.find_page('RDoc')
@@ -566,15 +535,6 @@ describe WikiPage do
     end
   end
 
-    context 'when Gitaly wiki_page_formatted_data is enabled' do
-      it_behaves_like 'fetching page formatted content'
-    end
-
-    context 'when Gitaly wiki_page_formatted_data is disabled', :disable_gitaly do
-      it_behaves_like 'fetching page formatted content'
-    end
-  end
-
   describe '#hook_attrs' do
     it 'adds absolute urls for images in the content' do
       create_page("test page", "test![WikiPage_Image](/uploads/abc/WikiPage_Image.png)")

spec/support/gitaly.rb

-RSpec.configure do |config|
-  config.before(:each) do |example|
-    if example.metadata[:disable_gitaly]
-      # Use 'and_wrap_original' to make sure the arguments are valid
-      allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original { |m, *args| m.call(*args) && false }
-    else
-      next if example.metadata[:skip_gitaly_mock]
-
-      # Use 'and_wrap_original' to make sure the arguments are valid
-      allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original do |m, *args|
-        m.call(*args)
-        !Gitlab::GitalyClient.explicit_opt_in_required.include?(args.first)
-      end
-    end
-  end
-end