Add Gitlab::Git::Commit unit specs

9 files changed, 282 insertions, 36 deletions

diff --git a/ruby/Gemfile b/ruby/Gemfile
index 711f505a9..44accc405 100644
--- a/ruby/Gemfile
+++ b/ruby/Gemfile
@@ -26,4 +26,5 @@ group :development, :test do
   gem 'rubocop', '~> 0.50', require: false
   gem 'rspec', require: false
   gem 'timecop', require: false
+  gem 'factory_bot', require: false
 end
diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock
index 8f7769f7a..20c316515 100644
--- a/ruby/Gemfile.lock
+++ b/ruby/Gemfile.lock
@@ -12,6 +12,8 @@ GEM
     crass (1.0.4)
     diff-lcs (1.3)
     escape_utils (1.2.1)
+    factory_bot (4.11.1)
+      activesupport (>= 3.0.0)
     faraday (0.15.3)
       multipart-post (>= 1.2, < 3)
     gemojione (3.3.0)
@@ -113,6 +115,7 @@ PLATFORMS
 DEPENDENCIES
   activesupport (~> 5.0.2)
   bundler (>= 1.16.5)
+  factory_bot
   faraday (~> 0.12)
   gitaly-proto (~> 0.116.0)
   github-linguist (~> 6.1)
diff --git a/ruby/lib/gitlab/git/commit.rb b/ruby/lib/gitlab/git/commit.rb
index 25ceb1117..c163e9e6e 100644
--- a/ruby/lib/gitlab/git/commit.rb
+++ b/ruby/lib/gitlab/git/commit.rb
@@ -161,17 +161,6 @@ module Gitlab
         Gitlab::Git::CommitStats.new(@repository, self)
       end
 
-      # Get ref names collection
-      #
-      # Ex.
-      #   commit.ref_names(repo)
-      #
-      def ref_names(repo)
-        refs(repo).map do |ref|
-          ref.sub(%r{^refs/(heads|remotes|tags)/}, "")
-        end
-      end
-
       def message
         encode! @message
       end
@@ -293,32 +282,10 @@ module Gitlab
         )
       end
 
-      # Get a collection of Gitlab::Git::Ref objects for this commit.
-      #
-      # Ex.
-      #   commit.ref(repo)
-      #
-      def refs(repo)
-        repo.refs_hash[id]
-      end
-
       def message_from_gitaly_body
         return @raw_commit.subject.dup if @raw_commit.body_size.zero?
-        return @raw_commit.body.dup if full_body_fetched_from_gitaly?
-
-        if @raw_commit.body_size > MAX_COMMIT_MESSAGE_DISPLAY_SIZE
-          "#{@raw_commit.subject}\n\n--commit message is too big".strip
-        else
-          fetch_body_from_gitaly
-        end
-      end
-
-      def full_body_fetched_from_gitaly?
-        @raw_commit.body.bytesize == @raw_commit.body_size
-      end
 
-      def fetch_body_from_gitaly
-        self.class.get_message(@repository, id)
+        @raw_commit.body.dup
       end
     end
   end
diff --git a/ruby/spec/factories/gitaly/commit.rb b/ruby/spec/factories/gitaly/commit.rb
new file mode 100644
index 000000000..5034c3d0e
--- /dev/null
+++ b/ruby/spec/factories/gitaly/commit.rb
@@ -0,0 +1,17 @@
+FactoryBot.define do
+  sequence(:gitaly_commit_id) { Digest::SHA1.hexdigest(Time.now.to_f.to_s) }
+
+  factory :gitaly_commit, class: Gitaly::GitCommit do
+    skip_create
+
+    id { generate(:gitaly_commit_id) }
+    parent_ids do
+      ids = [generate(:gitaly_commit_id), generate(:gitaly_commit_id)]
+      Google::Protobuf::RepeatedField.new(:string, ids)
+    end
+    subject { "My commit" }
+    body { subject + "\nMy body" }
+    author { build(:gitaly_commit_author) }
+    committer { build(:gitaly_commit_author) }
+  end
+end
diff --git a/ruby/spec/factories/gitaly/commit_author.rb b/ruby/spec/factories/gitaly/commit_author.rb
new file mode 100644
index 000000000..aaf634ce0
--- /dev/null
+++ b/ruby/spec/factories/gitaly/commit_author.rb
@@ -0,0 +1,9 @@
+FactoryBot.define do
+  factory :gitaly_commit_author, class: Gitaly::CommitAuthor do
+    skip_create
+
+    name { generate(:name) }
+    email { generate(:email) }
+    date { Google::Protobuf::Timestamp.new(seconds: Time.now.to_i) }
+  end
+end
diff --git a/ruby/spec/factories/sequences.rb b/ruby/spec/factories/sequences.rb
new file mode 100644
index 000000000..16a05b1ca
--- /dev/null
+++ b/ruby/spec/factories/sequences.rb
@@ -0,0 +1,4 @@
+FactoryBot.define do
+  sequence(:name) { |n| "John Doe#{n}" }
+  sequence(:email) { |n| "user#{n}@example.org" }
+end
diff --git a/ruby/spec/lib/gitlab/git/commit_spec.rb b/ruby/spec/lib/gitlab/git/commit_spec.rb
new file mode 100644
index 000000000..b6498f93a
--- /dev/null
+++ b/ruby/spec/lib/gitlab/git/commit_spec.rb
@@ -0,0 +1,219 @@
+require "spec_helper"
+
+describe Gitlab::Git::Commit, :seed_helper do
+  include TestRepo
+
+  let(:repository) { gitlab_git_from_gitaly(new_mutable_test_repo) }
+  let(:rugged_repo) { Rugged::Repository.new(repository.path) }
+  let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) }
+  let(:rugged_commit) { rugged_repo.lookup(SeedRepo::Commit::ID) }
+
+  describe "Commit info" do
+    let(:committer) do
+      {
+        email: 'mike@smith.com',
+        name: "Mike Smith",
+        time: Time.now
+      }
+    end
+    let(:author) do
+      {
+        email: 'john@smith.com',
+        name: "John Smith",
+        time: Time.now
+      }
+    end
+    let(:parents) { [rugged_repo.head.target] }
+    let(:gitlab_parents) do
+      parents.map { |c| described_class.find(repository, c.oid) }
+    end
+    let(:tree) { parents.first.tree }
+    let(:sha) do
+      Rugged::Commit.create(
+        rugged_repo,
+        author: author,
+        committer: committer,
+        tree: tree,
+        parents: parents,
+        message: "Refactoring specs",
+        update_ref: "HEAD"
+      )
+    end
+    let(:rugged_commit) { rugged_repo.lookup(sha) }
+    let(:commit) { described_class.find(repository, sha) }
+
+    it { expect(commit.short_id).to eq(rugged_commit.oid[0..10]) }
+    it { expect(commit.id).to eq(rugged_commit.oid) }
+    it { expect(commit.sha).to eq(rugged_commit.oid) }
+    it { expect(commit.safe_message).to eq(rugged_commit.message) }
+    it { expect(commit.created_at).to eq(rugged_commit.author[:time]) }
+    it { expect(commit.date).to eq(rugged_commit.committer[:time]) }
+    it { expect(commit.author_email).to eq(author[:email]) }
+    it { expect(commit.author_name).to eq(author[:name]) }
+    it { expect(commit.committer_name).to eq(committer[:name]) }
+    it { expect(commit.committer_email).to eq(committer[:email]) }
+    it { expect(commit.different_committer?).to be_truthy }
+    it { expect(commit.parents).to eq(gitlab_parents) }
+    it { expect(commit.parent_id).to eq(parents.first.oid) }
+    it { expect(commit.no_commit_message).to eq("--no commit message") }
+  end
+
+  describe "Commit info from gitaly commit" do
+    let(:subject) { "My commit".b }
+    let(:body) { subject + "My body".b }
+    let(:body_size) { body.length }
+    let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body, body_size: body_size) }
+    let(:id) { gitaly_commit.id }
+    let(:committer) { gitaly_commit.committer }
+    let(:author) { gitaly_commit.author }
+    let(:commit) { described_class.new(repository, gitaly_commit) }
+
+    it { expect(commit.short_id).to eq(id[0..10]) }
+    it { expect(commit.id).to eq(id) }
+    it { expect(commit.sha).to eq(id) }
+    it { expect(commit.safe_message).to eq(body) }
+    it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) }
+    it { expect(commit.author_email).to eq(author.email) }
+    it { expect(commit.author_name).to eq(author.name) }
+    it { expect(commit.committer_name).to eq(committer.name) }
+    it { expect(commit.committer_email).to eq(committer.email) }
+    it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) }
+
+    context 'body_size != body.size' do
+      let(:body) { "".b }
+
+      context 'zero body_size' do
+        it { expect(commit.safe_message).to eq(subject) }
+      end
+    end
+  end
+
+  context 'Class methods' do
+    describe '.find' do
+      it "returns an array of parent ids" do
+        expect(described_class.find(repository, SeedRepo::Commit::ID).parent_ids).to be_an(Array)
+      end
+
+      it "should return valid commit for tag" do
+        expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9')
+      end
+
+      it "should return nil for non-commit ids" do
+        blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb")
+        expect(described_class.find(repository, blob.id)).to be_nil
+      end
+
+      it "should return nil for parent of non-commit object" do
+        blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb")
+        expect(described_class.find(repository, "#{blob.id}^")).to be_nil
+      end
+
+      it "should return nil for nonexisting ids" do
+        expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil
+      end
+
+      context 'with broken repo' do
+        let(:repository) { gitlab_git_from_gitaly(new_broken_test_repo) }
+
+        it 'returns nil' do
+          expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil
+        end
+      end
+    end
+
+    describe '.shas_with_signatures' do
+      let(:signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e 570e7b2abdd848b95f2f578043fc23bd6f6fd24d] }
+      let(:unsigned_shas) { %w[19e2e9b4ef76b422ce1154af39a91323ccc57434 c642fe9b8b9f28f9225d7ea953fe14e74748d53b] }
+      let(:first_signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e c642fe9b8b9f28f9225d7ea953fe14e74748d53b] }
+
+      it 'has 2 signed shas' do
+        ret = described_class.shas_with_signatures(repository, signed_shas)
+        expect(ret).to eq(signed_shas)
+      end
+
+      it 'has 0 signed shas' do
+        ret = described_class.shas_with_signatures(repository, unsigned_shas)
+        expect(ret).to eq([])
+      end
+
+      it 'has 1 signed sha' do
+        ret = described_class.shas_with_signatures(repository, first_signed_shas)
+        expect(ret).to contain_exactly(first_signed_shas.first)
+      end
+    end
+  end
+
+  describe '#init_from_rugged' do
+    let(:gitlab_commit) { described_class.new(repository, rugged_commit) }
+    subject { gitlab_commit }
+
+    describe '#id' do
+      subject { super().id }
+      it { is_expected.to eq(SeedRepo::Commit::ID) }
+    end
+  end
+
+  describe '#init_from_hash' do
+    let(:commit) { described_class.new(repository, sample_commit_hash) }
+    subject { commit }
+
+    describe '#id' do
+      subject { super().id }
+      it { is_expected.to eq(sample_commit_hash[:id]) }
+    end
+
+    describe '#message' do
+      subject { super().message }
+      it { is_expected.to eq(sample_commit_hash[:message]) }
+    end
+  end
+
+  describe '#stats' do
+    subject { commit.stats }
+
+    describe '#additions' do
+      subject { super().additions }
+      it { is_expected.to eq(11) }
+    end
+
+    describe '#deletions' do
+      subject { super().deletions }
+      it { is_expected.to eq(6) }
+    end
+
+    describe '#total' do
+      subject { super().total }
+      it { is_expected.to eq(17) }
+    end
+  end
+
+  describe '#has_zero_stats?' do
+    it { expect(commit.has_zero_stats?).to eq(false) }
+  end
+
+  describe '#to_hash' do
+    let(:hash) { commit.to_hash }
+    subject { hash }
+
+    it { is_expected.to be_kind_of Hash }
+
+    describe '#keys' do
+      subject { super().keys.sort }
+      it { is_expected.to match(sample_commit_hash.keys.sort) }
+    end
+  end
+
+  def sample_commit_hash
+    {
+      author_email: "dmitriy.zaporozhets@gmail.com",
+      author_name: "Dmitriy Zaporozhets",
+      authored_date: "2012-02-27 20:51:12 +0200",
+      committed_date: "2012-02-27 20:51:12 +0200",
+      committer_email: "dmitriy.zaporozhets@gmail.com",
+      committer_name: "Dmitriy Zaporozhets",
+      id: SeedRepo::Commit::ID,
+      message: "tree css fixes",
+      parent_ids: ["874797c3a73b60d2187ed6e2fcabd289ff75171e"]
+    }
+  end
+end
diff --git a/ruby/spec/spec_helper.rb b/ruby/spec/spec_helper.rb
index be9412e5a..818c3c44c 100644
--- a/ruby/spec/spec_helper.rb
+++ b/ruby/spec/spec_helper.rb
@@ -3,7 +3,16 @@ require_relative '../lib/gitlab/git.rb'
 require_relative 'support/sentry.rb'
 require 'timecop'
 require 'test_repo_helper'
+require 'factory_bot'
 
 Dir[File.join(__dir__, 'support/helpers/*.rb')].each { |f| require f }
 
 ENV['GITALY_RUBY_GIT_BIN_PATH'] ||= 'git'
+
+RSpec.configure do |config|
+  config.include FactoryBot::Syntax::Methods
+
+  config.before(:suite) do
+    FactoryBot.find_definitions
+  end
+end
diff --git a/ruby/spec/test_repo_helper.rb b/ruby/spec/test_repo_helper.rb
index 9d8f39a9d..b3895c29d 100644
--- a/ruby/spec/test_repo_helper.rb
+++ b/ruby/spec/test_repo_helper.rb
@@ -35,13 +35,24 @@ module TestRepo
   end
 
   def new_mutable_test_repo
-    relative_path = "mutable-#{SecureRandom.hex(6)}.git"
+    relative_path = random_repository_relative_path(:mutable)
     TestRepo.clone_new_repo!(TEST_REPO_ORIGIN, File.join(DEFAULT_STORAGE_DIR, relative_path))
     Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path)
   end
 
+  def new_broken_test_repo
+    relative_path = random_repository_relative_path(:broken)
+    repo_path = File.join(DEFAULT_STORAGE_DIR, relative_path)
+    TestRepo.clone_new_repo!(TEST_REPO_ORIGIN, repo_path)
+
+    refs_path = File.join(repo_path, 'refs')
+    FileUtils.rm_r(refs_path)
+
+    Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path)
+  end
+
   def new_empty_test_repo
-    relative_path = "mutable-#{SecureRandom.hex(6)}.git"
+    relative_path = random_repository_relative_path(:mutable)
     TestRepo.init_new_repo!(File.join(DEFAULT_STORAGE_DIR, relative_path))
     Gitaly::Repository.new(storage_name: DEFAULT_STORAGE_NAME, relative_path: relative_path)
   end
@@ -78,6 +89,12 @@ module TestRepo
 
     abort "Failed to init test repo."
   end
+
+  private
+
+  def random_repository_relative_path(prefix)
+    "#{prefix}-#{SecureRandom.hex(6)}.git"
+  end
 end
 
 TestRepo.prepare_test_repository