Optimize a bit Gitlab::Git::Repository#log_by_shell and its specs

Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
Rémy Coutable 2017-02-23 19:05:13 +01:00
parent 3eb0354061
commit 9720bcd83d
2 changed files with 74 additions and 84 deletions

View file

@ -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)

View file

@ -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
context "where 'follow' == true" do
options = { ref: "master", follow: true }
context "and 'path' is a directory" do
let(:log_commits) do
repository.log(options.merge(path: "encoding"))
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
let(:options) { { ref: "master", follow: true } }
context "and 'path' is a directory" do
it "does not follow renames" do
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
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
context 'with offset=1' do
let(:log_commits) do
repository.log(options.merge(path: "encoding/CHANGELOG", offset: 1))
end
context 'with offset=1' do
it "follows renames and skip the latest commit" do
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
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
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
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