From 28553dbc05989b698777ee085aa2a357ffe576d2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 4 Jul 2017 05:58:28 +0800 Subject: Update tests due to permission changes --- spec/serializers/job_entity_spec.rb | 6 +++++- spec/serializers/pipeline_details_entity_spec.rb | 6 +++--- spec/serializers/pipeline_entity_spec.rb | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'spec/serializers') diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb index 5ca7bf2fcaf..ec30816654b 100644 --- a/spec/serializers/job_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -8,7 +8,7 @@ describe JobEntity do before do allow(request).to receive(:current_user).and_return(user) - project.add_developer(user) + project.add_master(user) end let(:entity) do @@ -90,6 +90,10 @@ describe JobEntity do end context 'when user is not allowed to trigger action' do + before do + project.team.truncate + end + it 'does not contain path to play action' do expect(subject).not_to include(:play_path) end diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index d28dec9592a..e9b24b47900 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -52,7 +52,7 @@ describe PipelineDetailsEntity do context 'user has ability to retry pipeline' do before do - project.team << [user, :developer] + project.add_master(user) end it 'retryable flag is true' do @@ -80,7 +80,7 @@ describe PipelineDetailsEntity do context 'user has ability to cancel pipeline' do before do - project.add_developer(user) + project.add_master(user) end it 'cancelable flag is true' do @@ -97,7 +97,7 @@ describe PipelineDetailsEntity do context 'when pipeline has commit statuses' do let(:pipeline) { create(:ci_empty_pipeline) } - + before do create(:generic_commit_status, pipeline: pipeline) end diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index 46650f3a80d..46433867b11 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -52,7 +52,7 @@ describe PipelineEntity do context 'user has ability to retry pipeline' do before do - project.team << [user, :developer] + project.add_master(user) end it 'contains retry path' do @@ -80,7 +80,7 @@ describe PipelineEntity do context 'user has ability to cancel pipeline' do before do - project.add_developer(user) + project.add_master(user) end it 'contains cancel path' do -- cgit v1.2.3 From 216bf78fd154005cbf8ec447bfa23f77f6b26775 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 4 Jul 2017 17:48:45 +0800 Subject: Introduce Gitlab::Cache::RequestStoreWrap So that we cache the result of UserAccess#can_push_or_merge_to_branch? in RequestStore, avoiding querying ProtectedBranch over and over for the list of pipelines (i.e. in PipelineSerializer) I don't think this is ideal because I don't like the idea of RequestStore in general, but this is the easiest way to cache it without changing the architecture. In the future we should cache more explicitly rather than this kind of global store. --- spec/serializers/pipeline_serializer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/serializers') diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 44813656aff..8dc666586c7 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -110,7 +110,7 @@ describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(57) + expect(recorded.count).to be_within(1).of(59) expect(recorded.cached_count).to eq(0) end -- cgit v1.2.3 From 7bd5e571256aff6de132b118f04224e56abf3228 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 18 Jul 2017 22:32:34 +0800 Subject: Instead of adding master, stub_not_protect_default_branch --- spec/serializers/job_entity_spec.rb | 11 ++++++++--- spec/serializers/pipeline_details_entity_spec.rb | 6 ++++-- spec/serializers/pipeline_entity_spec.rb | 6 ++++-- 3 files changed, 16 insertions(+), 7 deletions(-) (limited to 'spec/serializers') diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb index ec30816654b..026360e91a3 100644 --- a/spec/serializers/job_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -7,8 +7,10 @@ describe JobEntity do let(:request) { double('request') } before do + stub_not_protect_default_branch allow(request).to receive(:current_user).and_return(user) - project.add_master(user) + + project.add_developer(user) end let(:entity) do @@ -77,7 +79,7 @@ describe JobEntity do project.add_developer(user) create(:protected_branch, :developers_can_merge, - name: 'master', project: project) + name: job.ref, project: job.project) end it 'contains path to play action' do @@ -91,7 +93,10 @@ describe JobEntity do context 'when user is not allowed to trigger action' do before do - project.team.truncate + allow(job.project).to receive(:empty_repo?).and_return(false) + + create(:protected_branch, :no_one_can_push, + name: job.ref, project: job.project) end it 'does not contain path to play action' do diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index e9b24b47900..b990370a271 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -9,6 +9,8 @@ describe PipelineDetailsEntity do end before do + stub_not_protect_default_branch + allow(request).to receive(:current_user).and_return(user) end @@ -52,7 +54,7 @@ describe PipelineDetailsEntity do context 'user has ability to retry pipeline' do before do - project.add_master(user) + project.add_developer(user) end it 'retryable flag is true' do @@ -80,7 +82,7 @@ describe PipelineDetailsEntity do context 'user has ability to cancel pipeline' do before do - project.add_master(user) + project.add_developer(user) end it 'cancelable flag is true' do diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index 46433867b11..5b01cc4fc9e 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -5,6 +5,8 @@ describe PipelineEntity do let(:request) { double('request') } before do + stub_not_protect_default_branch + allow(request).to receive(:current_user).and_return(user) end @@ -52,7 +54,7 @@ describe PipelineEntity do context 'user has ability to retry pipeline' do before do - project.add_master(user) + project.add_developer(user) end it 'contains retry path' do @@ -80,7 +82,7 @@ describe PipelineEntity do context 'user has ability to cancel pipeline' do before do - project.add_master(user) + project.add_developer(user) end it 'contains cancel path' do -- cgit v1.2.3 From 561bc570dea970328e0c33972fcf1ed90427f2f2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 19 Jul 2017 17:53:56 +0800 Subject: Add a test for checking queries with different ref --- spec/serializers/pipeline_serializer_spec.rb | 33 +++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 6 deletions(-) (limited to 'spec/serializers') diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 8dc666586c7..262bc4acb69 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -108,14 +108,35 @@ describe PipelineSerializer do end end - it 'verifies number of queries', :request_store do - recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(59) - expect(recorded.cached_count).to eq(0) + shared_examples 'no N+1 queries' do + it 'verifies number of queries', :request_store do + recorded = ActiveRecord::QueryRecorder.new { subject } + expect(recorded.count).to be_within(1).of(59) + expect(recorded.cached_count).to eq(0) + end + end + + context 'with the same ref' do + let(:ref) { 'feature' } + + it_behaves_like 'no N+1 queries' + end + + context 'with different refs' do + def ref + @sequence ||= 0 + @sequence += 1 + "feature-#{@sequence}" + end + + it_behaves_like 'no N+1 queries' end def create_pipeline(status) - create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline| + create(:ci_empty_pipeline, + project: project, + status: status, + ref: ref).tap do |pipeline| Ci::Build::AVAILABLE_STATUSES.each do |status| create_build(pipeline, status, status) end @@ -125,7 +146,7 @@ describe PipelineSerializer do def create_build(pipeline, stage, status) create(:ci_build, :tags, :triggered, :artifacts, pipeline: pipeline, stage: stage, - name: stage, status: status) + name: stage, status: status, ref: pipeline.ref) end end end -- cgit v1.2.3