diff options
author | Robert Speicher <robert@gitlab.com> | 2017-12-19 19:23:00 +0300 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-12-19 19:23:00 +0300 |
commit | ac22576d80b6d043c4991ad39172976566465693 (patch) | |
tree | 06b4041a313cfabab35373c765cec56566000cb0 /spec | |
parent | 4e60b4f1cdfa7dc3524d17bf313d348758950ecf (diff) | |
parent | c6edae38870a4228e3b964d647b9ef588df11f27 (diff) |
Merge branch 'zj-gitaly-pipelines-n-1' into 'master'
Improve performance of Pipelines#index.json
See merge request gitlab-org/gitlab-ce!14846
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/pipelines_controller_spec.rb | 13 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 39 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 48 | ||||
-rw-r--r-- | spec/serializers/pipeline_serializer_spec.rb | 6 |
4 files changed, 95 insertions, 11 deletions
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 1604a2da485..35ac999cc65 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -17,13 +17,10 @@ describe Projects::PipelinesController do describe 'GET index.json' do before do - branch_head = project.commit - parent = branch_head.parent - - create(:ci_empty_pipeline, status: 'pending', project: project, sha: branch_head.id) - create(:ci_empty_pipeline, status: 'running', project: project, sha: branch_head.id) - create(:ci_empty_pipeline, status: 'created', project: project, sha: parent.id) - create(:ci_empty_pipeline, status: 'success', project: project, sha: parent.id) + %w(pending running created success).each_with_index do |status, index| + sha = project.commit("HEAD~#{index}") + create(:ci_empty_pipeline, status: status, project: project, sha: sha) + end end subject do @@ -46,7 +43,7 @@ describe Projects::PipelinesController do context 'when performing gitaly calls', :request_store do it 'limits the Gitaly requests' do - expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(8) + expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index d18a5c9dfa6..cd955a5eb69 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -13,6 +13,45 @@ describe Commit do it { is_expected.to include_module(StaticModel) } end + describe '.lazy' do + set(:project) { create(:project, :repository) } + + context 'when the commits are found' do + let(:oids) do + %w( + 498214de67004b1da3d820901307bed2a68a8ef6 + c642fe9b8b9f28f9225d7ea953fe14e74748d53b + 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + 048721d90c449b244b7b4c53a9186b04330174ec + 281d3a76f31c812dbf48abce82ccf6860adedd81 + ) + end + + subject { oids.map { |oid| described_class.lazy(project, oid) } } + + it 'batches requests for commits' do + expect(project.repository).to receive(:commits_by).once.and_call_original + + subject.first.title + subject.last.title + end + + it 'maintains ordering' do + subject.each_with_index do |commit, i| + expect(commit.id).to eq(oids[i]) + end + end + end + + context 'when not found' do + it 'returns nil as commit' do + commit = described_class.lazy(project, 'deadbeef').__sync + + expect(commit).to be_nil + end + end + end + describe '#author' do it 'looks up the author in a case-insensitive way' do user = create(:user, email: commit.author_email.upcase) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index bdc430c9095..1d7069feebd 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -239,6 +239,54 @@ describe Repository do end end + 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) } + + it 'finds each commit' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(oids.size) + end + + it 'returns only Commit instances' do + expect(subject).to all( be_a(Commit) ) + end + + context 'when some commits are not found ' do + let(:oids) do + ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10) + end + + it 'returns only found commits' do + expect(subject).not_to include(nil) + expect(subject.size).to eq(10) + end + end + + context 'when no oids are passed' do + let(:oids) { [] } + + it 'does not call #batch_by_oid' do + expect(Gitlab::Git::Commit).not_to receive(:batch_by_oid) + + subject + end + 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 diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 88d347322a6..c38795ad1a1 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe PipelineSerializer do + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:serializer) do @@ -16,7 +17,7 @@ describe PipelineSerializer do end context 'when a single object is being serialized' do - let(:resource) { create(:ci_empty_pipeline) } + let(:resource) { create(:ci_empty_pipeline, project: project) } it 'serializers the pipeline object' do expect(subject[:id]).to eq resource.id @@ -24,7 +25,7 @@ describe PipelineSerializer do end context 'when multiple objects are being serialized' do - let(:resource) { create_list(:ci_pipeline, 2) } + let(:resource) { create_list(:ci_pipeline, 2, project: project) } it 'serializers the array of pipelines' do expect(subject).not_to be_empty @@ -100,7 +101,6 @@ describe PipelineSerializer do context 'number of queries' do let(:resource) { Ci::Pipeline.all } - let(:project) { create(:project) } before do # Since RequestStore.active? is true we have to allow the |