summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2017-02-23 19:05:13 +0100
committerRémy Coutable <remy@rymai.me>2017-02-23 19:06:00 +0100
commit9720bcd83df29b4dc8da241d4d632993cd3f2895 (patch)
treef718e973ac0baa0ab35016977510f289ec193c2a
parent3eb0354061b6c2ad39c90c9a3e9a5afa62e7685f (diff)
downloadgitlab-ce-9720bcd83df29b4dc8da241d4d632993cd3f2895.tar.gz
Optimize a bit Gitlab::Git::Repository#log_by_shell and its specs23062-allow-git-log-to-accept-follow-and-skip
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--lib/gitlab/git/repository.rb22
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb136
2 files changed, 74 insertions, 84 deletions
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 9c68060064a..8afbd818bbd 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -333,21 +333,21 @@ module Gitlab
offset_in_ruby = use_follow_flag && options[:offset].present?
limit += offset if offset_in_ruby
- cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path} log)
- cmd += %W(-n #{limit})
- cmd += %w(--format=%H)
- cmd += %W(--skip=#{offset}) unless offset_in_ruby
- cmd += %w(--follow) if use_follow_flag
- cmd += %w(--no-merges) if options[:skip_merges]
- cmd += %W(--after=#{options[:after].iso8601}) if options[:after]
- cmd += %W(--before=#{options[:before].iso8601}) if options[:before]
- cmd += [sha]
- cmd += %W(-- #{options[:path]}) if options[:path].present?
+ cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} log]
+ cmd << "--max-count=#{limit}"
+ cmd << '--format=%H'
+ cmd << "--skip=#{offset}" unless offset_in_ruby
+ cmd << '--follow' if use_follow_flag
+ cmd << '--no-merges' if options[:skip_merges]
+ cmd << "--after=#{options[:after].iso8601}" if options[:after]
+ cmd << "--before=#{options[:before].iso8601}" if options[:before]
+ cmd << sha
+ cmd += %W[-- #{options[:path]}] if options[:path].present?
raw_output = IO.popen(cmd) { |io| io.read }
lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines
- lines.map { |c| Rugged::Commit.new(rugged, c.strip) }
+ lines.map! { |c| Rugged::Commit.new(rugged, c.strip) }
end
def sha_from_ref(ref)
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 05594089db3..2ea5f8f68be 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -529,7 +529,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
commit_with_new_name = nil
rename_commit = nil
- before(:all) do
+ before(:context) do
# Add new commits so that there's a renamed file in the commit history
repo = Gitlab::Git::Repository.new(TEST_REPO_PATH).rugged
@@ -538,123 +538,119 @@ describe Gitlab::Git::Repository, seed_helper: true do
commit_with_new_name = new_commit_edit_new_file(repo)
end
+ after(:context) do
+ # Erase our commits so other tests get the original repo
+ repo = Gitlab::Git::Repository.new(TEST_REPO_PATH).rugged
+ repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID)
+ end
+
context "where 'follow' == true" do
- options = { ref: "master", follow: true }
+ let(:options) { { ref: "master", follow: true } }
context "and 'path' is a directory" do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding"))
- end
-
it "does not follow renames" do
- expect(log_commits).to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).not_to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding"))
+
+ aggregate_failures do
+ expect(log_commits).to include(commit_with_new_name)
+ expect(log_commits).to include(rename_commit)
+ expect(log_commits).not_to include(commit_with_old_name)
+ end
end
end
context "and 'path' is a file that matches the new filename" do
context 'without offset' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG"))
- end
-
it "follows renames" do
- expect(log_commits).to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG"))
+
+ aggregate_failures do
+ expect(log_commits).to include(commit_with_new_name)
+ expect(log_commits).to include(rename_commit)
+ expect(log_commits).to include(commit_with_old_name)
+ end
end
end
context 'with offset=1' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1))
- end
-
it "follows renames and skip the latest commit" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1))
+
+ aggregate_failures do
+ expect(log_commits).not_to include(commit_with_new_name)
+ expect(log_commits).to include(rename_commit)
+ expect(log_commits).to include(commit_with_old_name)
+ end
end
end
context 'with offset=1', 'and limit=1' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 1))
- end
-
it "follows renames, skip the latest commit and return only one commit" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).not_to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 1))
+
+ expect(log_commits).to contain_exactly(rename_commit)
end
end
context 'with offset=1', 'and limit=2' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 2))
- end
-
it "follows renames, skip the latest commit and return only two commits" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1, limit: 2))
+
+ aggregate_failures do
+ expect(log_commits).to contain_exactly(rename_commit, commit_with_old_name)
+ end
end
end
context 'with offset=2' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2))
- end
-
it "follows renames and skip the latest commit" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).not_to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2))
+
+ aggregate_failures do
+ expect(log_commits).not_to include(commit_with_new_name)
+ expect(log_commits).not_to include(rename_commit)
+ expect(log_commits).to include(commit_with_old_name)
+ end
end
end
context 'with offset=2', 'and limit=1' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 1))
- end
-
it "follows renames, skip the two latest commit and return only one commit" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).not_to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 1))
+
+ expect(log_commits).to contain_exactly(commit_with_old_name)
end
end
context 'with offset=2', 'and limit=2' do
- let(:log_commits) do
- repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 2))
- end
-
it "follows renames, skip the two latest commit and return only one commit" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).not_to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "encoding/CHANGELOG", offset: 2, limit: 2))
+
+ aggregate_failures do
+ expect(log_commits).not_to include(commit_with_new_name)
+ expect(log_commits).not_to include(rename_commit)
+ expect(log_commits).to include(commit_with_old_name)
+ end
end
end
end
context "and 'path' is a file that matches the old filename" do
- let(:log_commits) do
- repository.log(options.merge(path: "CHANGELOG"))
- end
-
it "does not follow renames" do
- expect(log_commits).not_to include(commit_with_new_name)
- expect(log_commits).to include(rename_commit)
- expect(log_commits).to include(commit_with_old_name)
+ log_commits = repository.log(options.merge(path: "CHANGELOG"))
+
+ aggregate_failures do
+ expect(log_commits).not_to include(commit_with_new_name)
+ expect(log_commits).to include(rename_commit)
+ expect(log_commits).to include(commit_with_old_name)
+ end
end
end
context "unknown ref" do
- let(:log_commits) { repository.log(options.merge(ref: 'unknown')) }
+ it "returns an empty array" do
+ log_commits = repository.log(options.merge(ref: 'unknown'))
- it "return an empty array" do
expect(log_commits).to eq([])
end
end
@@ -773,12 +769,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
end
-
- after(:all) do
- # Erase our commits so other tests get the original repo
- repo = Gitlab::Git::Repository.new(TEST_REPO_PATH).rugged
- repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID)
- end
end
describe "#commits_between" do