Merge branch 'gitaly-clean-up-tests' into 'master'

Clean up Gitaly tests

See merge request !12526

8 files changed, 95 insertions, 144 deletions

diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index 2d61992f595..6edc69de078 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -34,7 +34,7 @@ module Gitlab
           commit_id: commit_id,
           prefix: ref_prefix
         )
-        GitalyClient.call(@storage, :ref, :find_ref_name, request).name
+        encode!(GitalyClient.call(@storage, :ref, :find_ref_name, request).name.dup)
       end
 
       def count_tag_names
diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb
index 8b041ac69b1..66c016d14b3 100644
--- a/spec/lib/gitlab/git/blame_spec.rb
+++ b/spec/lib/gitlab/git/blame_spec.rb
@@ -20,6 +20,7 @@ describe Gitlab::Git::Blame, seed_helper: true do
       expect(data.size).to eq(95)
       expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
       expect(data.first[:line]).to eq("# Contribute to GitLab")
+      expect(data.first[:line]).to be_utf8
     end
   end
 
@@ -40,6 +41,7 @@ describe Gitlab::Git::Blame, seed_helper: true do
       expect(data.size).to eq(1)
       expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
       expect(data.first[:line]).to eq("Ä ü")
+      expect(data.first[:line]).to be_utf8
     end
   end
 
@@ -61,6 +63,7 @@ describe Gitlab::Git::Blame, seed_helper: true do
       expect(data.size).to eq(1)
       expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit)
       expect(data.first[:line]).to eq(" ")
+      expect(data.first[:line]).to be_utf8
     end
   end
 end
diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb
index 9dba4397e79..d1d7ed1d02a 100644
--- a/spec/lib/gitlab/git/branch_spec.rb
+++ b/spec/lib/gitlab/git/branch_spec.rb
@@ -48,7 +48,7 @@ describe Gitlab::Git::Branch, seed_helper: true do
       expect(Gitlab::Git::Commit).to receive(:decorate)
         .with(hash_including(attributes)).and_call_original
 
-      expect(branch.dereferenced_target.message.encoding).to be(Encoding::UTF_8)
+      expect(branch.dereferenced_target.message).to be_utf8
     end
   end
 
diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb
index d50ccb0df30..d97e85364c2 100644
--- a/spec/lib/gitlab/git/diff_spec.rb
+++ b/spec/lib/gitlab/git/diff_spec.rb
@@ -180,7 +180,7 @@ EOT
         let(:raw_patch) { @raw_diff_hash[:diff].encode(Encoding::ASCII_8BIT) }
 
         it 'encodes diff patch to UTF-8' do
-          expect(diff.diff.encoding).to eq(Encoding::UTF_8)
+          expect(diff.diff).to be_utf8
         end
       end
     end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 0cd458bf933..f1cdd86edb9 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -27,34 +27,24 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     it 'returns UTF-8' do
-      expect(repository.root_ref.encoding).to eq(Encoding.find('UTF-8'))
+      expect(repository.root_ref).to be_utf8
     end
 
-    context 'with gitaly enabled' do
-      before do
-        stub_gitaly
-      end
-
-      after do
-        Gitlab::GitalyClient.clear_stubs!
-      end
-
-      it 'gets the branch name from GitalyClient' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-        repository.root_ref
-      end
+    it 'gets the branch name from GitalyClient' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+      repository.root_ref
+    end
 
-      it 'wraps GRPC not found' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-          .and_raise(GRPC::NotFound)
-        expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
-      end
+    it 'wraps GRPC not found' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+        .and_raise(GRPC::NotFound)
+      expect { repository.root_ref }.to raise_error(Gitlab::Git::Repository::NoRepository)
+    end
 
-      it 'wraps GRPC exceptions' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
-          .and_raise(GRPC::Unknown)
-        expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
-      end
+    it 'wraps GRPC exceptions' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+        .and_raise(GRPC::Unknown)
+      expect { repository.root_ref }.to raise_error(Gitlab::Git::CommandError)
     end
   end
 
@@ -129,37 +119,27 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     it 'returns UTF-8' do
-      expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+      expect(subject.first).to be_utf8
     end
 
     it { is_expected.to include("master") }
     it { is_expected.not_to include("branch-from-space") }
 
-    context 'with gitaly enabled' do
-      before do
-        stub_gitaly
-      end
-
-      after do
-        Gitlab::GitalyClient.clear_stubs!
-      end
-
-      it 'gets the branch names from GitalyClient' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-        subject
-      end
+    it 'gets the branch names from GitalyClient' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+      subject
+    end
 
-      it 'wraps GRPC not found' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-          .and_raise(GRPC::NotFound)
-        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
-      end
+    it 'wraps GRPC not found' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+        .and_raise(GRPC::NotFound)
+      expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+    end
 
-      it 'wraps GRPC other exceptions' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
-          .and_raise(GRPC::Unknown)
-        expect { subject }.to raise_error(Gitlab::Git::CommandError)
-      end
+    it 'wraps GRPC other exceptions' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+        .and_raise(GRPC::Unknown)
+      expect { subject }.to raise_error(Gitlab::Git::CommandError)
     end
   end
 
@@ -173,7 +153,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     it 'returns UTF-8' do
-      expect(subject.first.encoding).to eq(Encoding.find('UTF-8'))
+      expect(subject.first).to be_utf8
     end
 
     describe '#last' do
@@ -183,31 +163,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to include("v1.0.0") }
     it { is_expected.not_to include("v5.0.0") }
 
-    context 'with gitaly enabled' do
-      before do
-        stub_gitaly
-      end
-
-      after do
-        Gitlab::GitalyClient.clear_stubs!
-      end
-
-      it 'gets the tag names from GitalyClient' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-        subject
-      end
+    it 'gets the tag names from GitalyClient' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+      subject
+    end
 
-      it 'wraps GRPC not found' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-          .and_raise(GRPC::NotFound)
-        expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
-      end
+    it 'wraps GRPC not found' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+        .and_raise(GRPC::NotFound)
+      expect { subject }.to raise_error(Gitlab::Git::Repository::NoRepository)
+    end
 
-      it 'wraps GRPC exceptions' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
-          .and_raise(GRPC::Unknown)
-        expect { subject }.to raise_error(Gitlab::Git::CommandError)
-      end
+    it 'wraps GRPC exceptions' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+        .and_raise(GRPC::Unknown)
+      expect { subject }.to raise_error(Gitlab::Git::CommandError)
     end
   end
 
@@ -1281,42 +1251,31 @@ describe Gitlab::Git::Repository, seed_helper: true do
       expect(@repo.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true)
     end
 
-    context 'with gitaly enabled' do
-      before do
-        stub_gitaly
-      end
-
-      after do
-        Gitlab::GitalyClient.clear_stubs!
-      end
-
-      it 'returns a Branch with UTF-8 fields' do
-        branches = @repo.local_branches.to_a
-        expect(branches.size).to be > 0
-        utf_8 = Encoding.find('utf-8')
-        branches.each do |branch|
-          expect(branch.name.encoding).to eq(utf_8)
-          expect(branch.target.encoding).to eq(utf_8) unless branch.target.nil?
-        end
+    it 'returns a Branch with UTF-8 fields' do
+      branches = @repo.local_branches.to_a
+      expect(branches.size).to be > 0
+      branches.each do |branch|
+        expect(branch.name).to be_utf8
+        expect(branch.target).to be_utf8 unless branch.target.nil?
       end
+    end
 
-      it 'gets the branches from GitalyClient' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
-          .and_return([])
-        @repo.local_branches
-      end
+    it 'gets the branches from GitalyClient' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
+        .and_return([])
+      @repo.local_branches
+    end
 
-      it 'wraps GRPC not found' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
-          .and_raise(GRPC::NotFound)
-        expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository)
-      end
+    it 'wraps GRPC not found' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
+        .and_raise(GRPC::NotFound)
+      expect { @repo.local_branches }.to raise_error(Gitlab::Git::Repository::NoRepository)
+    end
 
-      it 'wraps GRPC exceptions' do
-        expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
-          .and_raise(GRPC::Unknown)
-        expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError)
-      end
+    it 'wraps GRPC exceptions' do
+      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
+        .and_raise(GRPC::Unknown)
+      expect { @repo.local_branches }.to raise_error(Gitlab::Git::CommandError)
     end
   end
 
@@ -1395,11 +1354,4 @@ describe Gitlab::Git::Repository, seed_helper: true do
     sha = Rugged::Commit.create(repo, options)
     repo.lookup(sha)
   end
-
-  def stub_gitaly
-    allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(true)
-
-    stub = double(:stub)
-    allow(Gitaly::Ref::Stub).to receive(:new).and_return(stub)
-  end
 end
diff --git a/spec/lib/gitlab/gitaly_client/ref_spec.rb b/spec/lib/gitlab/gitaly_client/ref_spec.rb
index 8ad39a02b93..df22fcad902 100644
--- a/spec/lib/gitlab/gitaly_client/ref_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/ref_spec.rb
@@ -6,17 +6,6 @@ describe Gitlab::GitalyClient::Ref do
   let(:relative_path) { project.path_with_namespace + '.git' }
   let(:client) { described_class.new(project.repository) }
 
-  before do
-    allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)
-  end
-
-  after do
-    # When we say `expect_any_instance_of(Gitaly::Ref::Stub)` a double is created,
-    # and because GitalyClient shares stubs these will get passed from example to
-    # example, which will cause an error, so we clean the stubs after each example.
-    Gitlab::GitalyClient.clear_stubs!
-  end
-
   describe '#branch_names' do
     it 'sends a find_all_branch_names message' do
       expect_any_instance_of(Gitaly::Ref::Stub)
@@ -82,4 +71,13 @@ describe Gitlab::GitalyClient::Ref do
       expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError)
     end
   end
+
+  describe '#find_ref_name', seed_helper: true do
+    let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) }
+    let(:client) { described_class.new(repository) }
+    subject { client.find_ref_name(SeedRepo::Commit::ID, 'refs/heads/master') }
+
+    it { should be_utf8 }
+    it { should eq('refs/heads/master') }
+  end
 end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index b0635c6a90a..0a2cd8c2957 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -120,28 +120,17 @@ describe Environment, models: true do
     let(:head_commit)   { project.commit }
     let(:commit)        { project.commit.parent }
 
-    context 'Gitaly find_ref_name feature disabled' do
-      it 'returns deployment id for the environment' do
-        expect(environment.first_deployment_for(commit)).to eq deployment1
-      end
+    it 'returns deployment id for the environment' do
+      expect(environment.first_deployment_for(commit)).to eq deployment1
+    end
 
-      it 'return nil when no deployment is found' do
-        expect(environment.first_deployment_for(head_commit)).to eq nil
-      end
+    it 'return nil when no deployment is found' do
+      expect(environment.first_deployment_for(head_commit)).to eq nil
     end
 
-    # TODO: Uncomment when feature is reenabled
-    # context 'Gitaly find_ref_name feature enabled' do
-    #   before do
-    #     allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true)
-    #   end
-    #
-    #   it 'calls GitalyClient' do
-    #     expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name)
-    #
-    #     environment.first_deployment_for(commit)
-    #   end
-    # end
+    it 'returns a UTF-8 ref' do
+      expect(environment.first_deployment_for(commit).ref).to be_utf8
+    end
   end
 
   describe '#environment_type' do
diff --git a/spec/support/matchers/be_utf8.rb b/spec/support/matchers/be_utf8.rb
new file mode 100644
index 00000000000..ea806352422
--- /dev/null
+++ b/spec/support/matchers/be_utf8.rb
@@ -0,0 +1,9 @@
+RSpec::Matchers.define :be_utf8 do |_|
+  match do |actual|
+    actual.is_a?(String) && actual.encoding == Encoding.find('UTF-8')
+  end
+
+  description do
+    "be a String with encoding UTF-8"
+  end
+end