From 9e146233d489894fac32c0d2e81bb3224b6f4ff3 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Apr 2018 18:59:07 +0900 Subject: Add job_trace_chunk_spec with small fixes --- spec/models/ci/job_trace_chunk_spec.rb | 312 +++++++++++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 spec/models/ci/job_trace_chunk_spec.rb (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb new file mode 100644 index 00000000000..fa316159e1a --- /dev/null +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -0,0 +1,312 @@ +require 'spec_helper' + +describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do + set(:job) { create(:ci_build, :running) } + let(:chunk_index) { 0 } + let(:data_store) { :redis } + let(:raw_data) { nil } + let(:job_trace_chunk) do + described_class.new(job: job, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) + end + + describe '#data' do + subject { job_trace_chunk.data } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + before do + job_trace_chunk.send(:redis_set_data, 'Sample data in redis') + end + + it { is_expected.to eq('Sample data in redis') } + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + + it { is_expected.to eq('Sample data in db') } + end + + context 'when data_store is others' do + before do + job_trace_chunk.send(:write_attribute, :data_store, -1) + end + + it { expect { subject }.to raise_error('Unsupported data store') } + end + end + + describe '#set_data' do + subject { job_trace_chunk.set_data(value) } + + let(:value) { 'Sample data' } + + context 'when value bytesize is bigger than CHUNK_SIZE' do + let(:value) { 'a' * (described_class::CHUNK_SIZE + 1) } + + it { expect { subject }.to raise_error('too much data') } + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + + it do + expect(job_trace_chunk.send(:redis_data)).to be_nil + + subject + + expect(job_trace_chunk.send(:redis_data)).to eq(value) + end + + context 'when fullfilled chunk size' do + let(:value) { 'a' * described_class::CHUNK_SIZE } + + it 'schedules stashing data' do + expect(StashTraceChunkWorker).to receive(:perform_async).once + + subject + end + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + it 'sets data' do + expect(job_trace_chunk.raw_data).to be_nil + + subject + + expect(job_trace_chunk.raw_data).to eq(value) + expect(job_trace_chunk.persisted?).to be_truthy + end + + context 'when raw_data is not changed' do + it 'does not execute UPDATE' do + expect(job_trace_chunk.raw_data).to be_nil + job_trace_chunk.save! + + # First set + expect(ActiveRecord::QueryRecorder.new { subject }.count).to be > 0 + expect(job_trace_chunk.raw_data).to eq(value) + expect(job_trace_chunk.persisted?).to be_truthy + + # Second set + job_trace_chunk.reload + expect(ActiveRecord::QueryRecorder.new { subject }.count).to be(0) + end + end + + context 'when fullfilled chunk size' do + it 'does not schedule stashing data' do + expect(StashTraceChunkWorker).not_to receive(:perform_async) + + subject + end + end + end + + context 'when data_store is others' do + before do + job_trace_chunk.send(:write_attribute, :data_store, -1) + end + + it { expect { subject }.to raise_error('Unsupported data store') } + end + end + + describe '#truncate' do + subject { job_trace_chunk.truncate(offset) } + + shared_examples_for 'truncates' do + context 'when offset is negative' do + let(:offset) { -1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:offset) { data.bytesize + 1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is 10' do + let(:offset) { 10 } + + it 'truncates' do + subject + + expect(job_trace_chunk.data).to eq(data.byteslice(0, offset)) + end + end + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + let(:data) { 'Sample data in redis' } + + before do + job_trace_chunk.send(:redis_set_data, data) + end + + it_behaves_like 'truncates' + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it_behaves_like 'truncates' + end + end + + describe '#append' do + subject { job_trace_chunk.append(new_data, offset) } + + let(:new_data) { 'Sample new data' } + let(:offset) { 0 } + let(:total_data) { data + new_data } + + shared_examples_for 'appends' do + context 'when offset is negative' do + let(:offset) { -1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:offset) { data.bytesize + 1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:new_data) { 'a' * (described_class::CHUNK_SIZE + 1) } + + it { expect { subject }.to raise_error('Outside of chunk size') } + end + + context 'when offset is EOF' do + let(:offset) { data.bytesize } + + it 'appends' do + subject + + expect(job_trace_chunk.data).to eq(total_data) + end + end + + context 'when offset is 10' do + let(:offset) { 10 } + + it 'appends' do + subject + + expect(job_trace_chunk.data).to eq(data.byteslice(0, offset) + new_data) + end + end + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + let(:data) { 'Sample data in redis' } + + before do + job_trace_chunk.send(:redis_set_data, data) + end + + it_behaves_like 'appends' + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it_behaves_like 'appends' + end + end + + describe '#size' do + subject { job_trace_chunk.size } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + context 'when data exists' do + let(:data) { 'Sample data in redis' } + + before do + job_trace_chunk.send(:redis_set_data, data) + end + + it { is_expected.to eq(data.bytesize) } + end + + context 'when data exists' do + it { is_expected.to eq(0) } + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + context 'when data exists' do + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it { is_expected.to eq(data.bytesize) } + end + + context 'when data does not exist' do + it { is_expected.to eq(0) } + end + end + end + + describe '#use_database!' do + subject { job_trace_chunk.use_database! } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + context 'when data exists' do + let(:data) { 'Sample data in redis' } + + before do + job_trace_chunk.send(:redis_set_data, data) + end + + it 'stashes the data' do + expect(job_trace_chunk.data_store).to eq('redis') + expect(job_trace_chunk.send(:redis_data)).to eq(data) + expect(job_trace_chunk.raw_data).to be_nil + + subject + + expect(job_trace_chunk.data_store).to eq('db') + expect(job_trace_chunk.send(:redis_data)).to be_nil + expect(job_trace_chunk.raw_data).to eq(data) + end + end + + context 'when data does not exist' do + it 'does not call UPDATE' do + expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) + end + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + it 'does not call UPDATE' do + expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) + end + end + end +end -- cgit v1.2.1 From 242b4afa8fd1ffd0686038ba955f8ad41dc95383 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Apr 2018 20:39:35 +0900 Subject: Chnaged Chunk size to 128kb. Add spec. --- spec/models/ci/job_trace_chunk_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index fa316159e1a..d3fbec9170b 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -9,6 +9,12 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do described_class.new(job: job, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end + describe 'CHUNK_SIZE' do + it 'Chunk size can not be changed without special care' do + expect(described_class::CHUNK_SIZE).to eq(128.kilobytes) + end + end + describe '#data' do subject { job_trace_chunk.data } -- cgit v1.2.1 From f8f62ea5b6e81162cfb8781730040e54993cd847 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Apr 2018 21:15:18 +0900 Subject: Rename to SwapTraceChunkWorker from StashTraceChunkWorker --- spec/models/ci/job_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index d3fbec9170b..6ad4b321c3f 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -70,7 +70,7 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do let(:value) { 'a' * described_class::CHUNK_SIZE } it 'schedules stashing data' do - expect(StashTraceChunkWorker).to receive(:perform_async).once + expect(SwapTraceChunkWorker).to receive(:perform_async).once subject end @@ -107,7 +107,7 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do context 'when fullfilled chunk size' do it 'does not schedule stashing data' do - expect(StashTraceChunkWorker).not_to receive(:perform_async) + expect(SwapTraceChunkWorker).not_to receive(:perform_async) subject end -- cgit v1.2.1 From b94c84e597c02a6c38cb8f5233c77dc0b15a7c4e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Apr 2018 23:43:21 +0900 Subject: Add spec for ChunkedIO --- spec/models/ci/job_trace_chunk_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index 6ad4b321c3f..cb993885fa1 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -5,6 +5,7 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do let(:chunk_index) { 0 } let(:data_store) { :redis } let(:raw_data) { nil } + let(:job_trace_chunk) do described_class.new(job: job, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end -- cgit v1.2.1 From 47b01f592134d7a4e55d6832633436e2d098d4cc Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 6 Apr 2018 21:29:39 +0900 Subject: Add test for after_destroy :redis_delete_data hook --- spec/models/ci/job_trace_chunk_spec.rb | 61 ++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index cb993885fa1..0daf5ca25c3 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -316,4 +316,65 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do end end end + + describe 'deletes data in redis after chunk record destroyed' do + let(:project) { create(:project) } + + before do + pipeline = create(:ci_pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + end + + shared_examples_for 'deletes all job_trace_chunk and data in redis' do + it do + project.builds.each do |build| + Gitlab::Redis::SharedState.with do |redis| + redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + expect(redis.exists(key)).to be_truthy + end + end + end + + expect(described_class.count).not_to eq(0) + + subject + + expect(described_class.count).to eq(0) + + project.builds.each do |build| + Gitlab::Redis::SharedState.with do |redis| + redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + expect(redis.exists(key)).to be_falsey + end + end + end + end + end + + context 'when job_trace_chunk is destroyed' do + let(:subject) do + project.builds.each { |build| build.chunks.destroy_all } + end + + it_behaves_like 'deletes all job_trace_chunk and data in redis' + end + + context 'when job is destroyed' do + let(:subject) do + project.builds.destroy_all + end + + it_behaves_like 'deletes all job_trace_chunk and data in redis' + end + + context 'when project is destroyed' do + let(:subject) do + project.destroy! + end + + it_behaves_like 'deletes all job_trace_chunk and data in redis' + end + end end -- cgit v1.2.1 From 76485cbf8ba555c929fd2f54ca2051a382760f20 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 7 Apr 2018 00:08:35 +0900 Subject: Add ExclusiveLock in Ci::JobTraceChunk --- spec/models/ci/job_trace_chunk_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index 0daf5ca25c3..eb240de188f 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -317,6 +317,17 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do end end + describe 'ExclusiveLock' do + before do + allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil } + stub_const('Ci::JobTraceChunk::LOCK_RETRY', 1) + end + + it 'raise an error' do + expect { job_trace_chunk.append('ABC', 0) }.to raise_error('Failed to obtain write lock') + end + end + describe 'deletes data in redis after chunk record destroyed' do let(:project) { create(:project) } -- cgit v1.2.1 From 9a07bc819f137a3077784a33e1b36bf6797d9547 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 1 Mar 2017 13:07:24 +0100 Subject: add misssing scope specs --- spec/models/ci/runner_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ab170e6351c..529f200b43a 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -49,6 +49,25 @@ describe Ci::Runner do end end + describe 'scopes' do + describe 'owned_or_shared' do + it 'returns the specific project runner' do + specific_project = create :project + other_project = create :project + specific_runner = create :ci_runner, :specific, projects: [specific_project] + other_runner = create :ci_runner, :specific, projects: [other_project] + + expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] + end + + it 'returns the shared projects' do + runner = create :ci_runner, :shared + + expect(described_class.owned_or_shared(0)).to eq [runner] + end + end + end + describe '#display_name' do it 'returns the description if it has a value' do runner = FactoryBot.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') -- cgit v1.2.1 From 295184f6a5ff0b98340c32e0cc715dafa4d9b60c Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 1 Mar 2017 17:19:12 +0100 Subject: include group runners in scope --- spec/models/ci/runner_spec.rb | 50 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 529f200b43a..477540fb3b0 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -52,19 +52,61 @@ describe Ci::Runner do describe 'scopes' do describe 'owned_or_shared' do it 'returns the specific project runner' do + # own specific_project = create :project - other_project = create :project specific_runner = create :ci_runner, :specific, projects: [specific_project] - other_runner = create :ci_runner, :specific, projects: [other_project] + + # other + other_project = create :project + create :ci_runner, :specific, projects: [other_project] expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] end - it 'returns the shared projects' do - runner = create :ci_runner, :shared + it 'returns the shared project runner' do + project = create :project + runner = create :ci_runner, :shared, projects: [project] expect(described_class.owned_or_shared(0)).to eq [runner] end + + it 'returns the specific group runner' do + # own + specific_group = create :group + specific_project = create :project, group: specific_group + specific_runner = create :ci_runner, :specific, groups: [specific_group] + + # other + other_group = create :group + create :project, group: other_group + create :ci_runner, :specific, groups: [other_group] + + expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] + end + + it 'returns the shared group runner' do + group = create :group + runner = create :ci_runner, :shared, groups: [group] + + expect(described_class.owned_or_shared(0)).to eq [runner] + end + + it 'returns a globally shared, a project specific and a group specific runner' do + # group specific + group = create :group + project = create :project, group: group + group_runner = create :ci_runner, :specific, groups: [group] + + # project specific + project_runner = create :ci_runner, :specific, projects: [project] + + # globally shared + shared_runner = create :ci_runner, :shared + + expect(described_class.owned_or_shared(project.id)).to match_array [ + group_runner, project_runner, shared_runner + ] + end end end -- cgit v1.2.1 From 14c8dbc66537e59d9a01fe5e8ad64ba559254f14 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 6 Sep 2017 11:26:55 +0200 Subject: drop 'scopes' context from specs --- spec/models/ci/runner_spec.rb | 88 +++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 45 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 477540fb3b0..5a851a3d559 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -49,64 +49,62 @@ describe Ci::Runner do end end - describe 'scopes' do - describe 'owned_or_shared' do - it 'returns the specific project runner' do - # own - specific_project = create :project - specific_runner = create :ci_runner, :specific, projects: [specific_project] + describe '.owned_or_shared' do + it 'returns the specific project runner' do + # own + specific_project = create :project + specific_runner = create :ci_runner, :specific, projects: [specific_project] - # other - other_project = create :project - create :ci_runner, :specific, projects: [other_project] + # other + other_project = create :project + create :ci_runner, :specific, projects: [other_project] - expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] - end + expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] + end - it 'returns the shared project runner' do - project = create :project - runner = create :ci_runner, :shared, projects: [project] + it 'returns the shared project runner' do + project = create :project + runner = create :ci_runner, :shared, projects: [project] - expect(described_class.owned_or_shared(0)).to eq [runner] - end + expect(described_class.owned_or_shared(0)).to eq [runner] + end - it 'returns the specific group runner' do - # own - specific_group = create :group - specific_project = create :project, group: specific_group - specific_runner = create :ci_runner, :specific, groups: [specific_group] + it 'returns the specific group runner' do + # own + specific_group = create :group + specific_project = create :project, group: specific_group + specific_runner = create :ci_runner, :specific, groups: [specific_group] - # other - other_group = create :group - create :project, group: other_group - create :ci_runner, :specific, groups: [other_group] + # other + other_group = create :group + create :project, group: other_group + create :ci_runner, :specific, groups: [other_group] - expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] - end + expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] + end - it 'returns the shared group runner' do - group = create :group - runner = create :ci_runner, :shared, groups: [group] + it 'returns the shared group runner' do + group = create :group + runner = create :ci_runner, :shared, groups: [group] - expect(described_class.owned_or_shared(0)).to eq [runner] - end + expect(described_class.owned_or_shared(0)).to eq [runner] + end - it 'returns a globally shared, a project specific and a group specific runner' do - # group specific - group = create :group - project = create :project, group: group - group_runner = create :ci_runner, :specific, groups: [group] + it 'returns a globally shared, a project specific and a group specific runner' do + # group specific + group = create :group + project = create :project, group: group + group_runner = create :ci_runner, :specific, groups: [group] - # project specific - project_runner = create :ci_runner, :specific, projects: [project] + # project specific + project_runner = create :ci_runner, :specific, projects: [project] - # globally shared - shared_runner = create :ci_runner, :shared + # globally shared + shared_runner = create :ci_runner, :shared - expect(described_class.owned_or_shared(project.id)).to match_array [ - group_runner, project_runner, shared_runner - ] - end + expect(described_class.owned_or_shared(project.id)).to match_array [ + group_runner, project_runner, shared_runner + ] end end -- cgit v1.2.1 From 7fbdd17cbcd19086694f575884191a6d137838dc Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 7 Sep 2017 15:49:29 +0200 Subject: authorize group runners on user --- spec/models/user_spec.rb | 64 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 12 deletions(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 35db7616efb..f384b688889 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1785,14 +1785,12 @@ describe User do describe '#ci_authorized_runners' do let(:user) { create(:user) } - let(:runner) { create(:ci_runner) } + let(:runner_1) { create(:ci_runner) } + let(:runner_2) { create(:ci_runner) } - before do - project.runners << runner - end - - context 'without any projects' do - let(:project) { create(:project) } + context 'without any projects nor groups' do + let!(:project) { create(:project, runners: [runner_1]) } + let!(:group) { create(:group) } it 'does not load' do expect(user.ci_authorized_runners).to be_empty @@ -1801,10 +1799,38 @@ describe User do context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let(:project) { create(:project, namespace: namespace) } + let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_authorized_runners).to contain_exactly(runner_1) + end + end + + context 'with personal group runner' do + let!(:project) { create(:project, runners: [runner_1]) } + let!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_authorized_runners).to contain_exactly(runner_2) + end + end + + context 'with personal project and group runner' do + let(:namespace) { create(:namespace, owner: user) } + let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + + let!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_authorized_runners).to contain_exactly(runner_1, runner_2) end end @@ -1815,7 +1841,7 @@ describe User do end it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_authorized_runners).to contain_exactly(runner_1) end end @@ -1832,7 +1858,21 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let(:project) { create(:project, group: group) } + let!(:project) { create(:project, group: group, runners: [runner_1]) } + + def add_user(access) + group.add_user(user, access) + end + + it_behaves_like :member + end + + context 'with groups runners' do + let!(:group) do + create(:group, runners: [runner_1]).tap do |group| + group.add_owner(user) + end + end def add_user(access) group.add_user(user, access) @@ -1842,7 +1882,7 @@ describe User do end context 'with other projects runners' do - let(:project) { create(:project) } + let!(:project) { create(:project, runners: [runner_1]) } def add_user(access) project.add_role(user, access) -- cgit v1.2.1 From 4b1b2f3b104df455d5d3265adca92dd09e079ee9 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 25 Sep 2017 15:28:33 +0200 Subject: add Ci::Runner#group? method --- spec/models/ci/runner_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 5a851a3d559..b9aafa63493 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -642,4 +642,28 @@ describe Ci::Runner do expect(described_class.search(runner.description.upcase)).to eq([runner]) end end + + describe 'group?' do + it 'returns false when the runner is a project runner' do + project = create :project + runner = create(:ci_runner, description: 'Project runner').tap do |r| + create :ci_runner_project, runner: r, project: project + end + + expect(runner.group?).to be false + end + + it 'returns false when the runner is a shared runner' do + runner = create :ci_runner, :shared, description: 'Shared runner' + + expect(runner.group?).to be false + end + + it 'returns true when the runner is assigned to a group' do + group = create :group + runner = create :ci_runner, description: 'Group runner', groups: [group] + + expect(runner.group?).to be true + end + end end -- cgit v1.2.1 From 81c0c57acd0f065bc5b80902ee664256d4c3241f Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 25 Sep 2017 16:46:03 +0200 Subject: exclude group runners on projects that disabled it --- spec/models/ci/runner_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index b9aafa63493..933bd6e5f23 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -83,6 +83,14 @@ describe Ci::Runner do expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] end + it 'does not return the group runner if the project has group runners disabled' do + specific_group = create :group + specific_project = create :project, group: specific_group, group_runners_enabled: false + create :ci_runner, :specific, groups: [specific_group] + + expect(described_class.owned_or_shared(specific_project.id)).to be_empty + end + it 'returns the shared group runner' do group = create :group runner = create :ci_runner, :shared, groups: [group] -- cgit v1.2.1 From d6167a9214b3a3c13850cdac9895c9d7577ddf25 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 4 Oct 2017 09:27:27 +0200 Subject: split up Ci::Runner.owned_or_shared scope --- spec/models/ci/runner_spec.rb | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 933bd6e5f23..a073af7c4b2 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -49,7 +49,23 @@ describe Ci::Runner do end end - describe '.owned_or_shared' do + describe '.shared' do + it 'returns the shared group runner' do + group = create :group + runner = create :ci_runner, :shared, groups: [group] + + expect(described_class.shared).to eq [runner] + end + + it 'returns the shared project runner' do + project = create :project + runner = create :ci_runner, :shared, projects: [project] + + expect(described_class.shared).to eq [runner] + end + end + + describe '.belonging_to_project' do it 'returns the specific project runner' do # own specific_project = create :project @@ -59,16 +75,11 @@ describe Ci::Runner do other_project = create :project create :ci_runner, :specific, projects: [other_project] - expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] - end - - it 'returns the shared project runner' do - project = create :project - runner = create :ci_runner, :shared, projects: [project] - - expect(described_class.owned_or_shared(0)).to eq [runner] + expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] end + end + describe '.belonging_to_group' do it 'returns the specific group runner' do # own specific_group = create :group @@ -80,7 +91,7 @@ describe Ci::Runner do create :project, group: other_group create :ci_runner, :specific, groups: [other_group] - expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner] + expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] end it 'does not return the group runner if the project has group runners disabled' do @@ -88,16 +99,11 @@ describe Ci::Runner do specific_project = create :project, group: specific_group, group_runners_enabled: false create :ci_runner, :specific, groups: [specific_group] - expect(described_class.owned_or_shared(specific_project.id)).to be_empty - end - - it 'returns the shared group runner' do - group = create :group - runner = create :ci_runner, :shared, groups: [group] - - expect(described_class.owned_or_shared(0)).to eq [runner] + expect(described_class.belonging_to_group(specific_project.id)).to be_empty end + end + describe '.owned_or_shared' do it 'returns a globally shared, a project specific and a group specific runner' do # group specific group = create :group -- cgit v1.2.1 From 8dad45a82228a6f1c87f919063d96c8b20a567e2 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 4 Oct 2017 13:55:34 +0200 Subject: add method CI::Runner.project? --- spec/models/ci/runner_spec.rb | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index a073af7c4b2..308db9e8e68 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -660,9 +660,7 @@ describe Ci::Runner do describe 'group?' do it 'returns false when the runner is a project runner' do project = create :project - runner = create(:ci_runner, description: 'Project runner').tap do |r| - create :ci_runner_project, runner: r, project: project - end + runner = create :ci_runner, description: 'Project runner', projects: [project] expect(runner.group?).to be false end @@ -680,4 +678,26 @@ describe Ci::Runner do expect(runner.group?).to be true end end + + describe 'project?' do + it 'returns false when the runner is a group prunner' do + group = create :group + runner = create :ci_runner, description: 'Group runner', groups: [group] + + expect(runner.project?).to be false + end + + it 'returns false when the runner is a shared runner' do + runner = create :ci_runner, :shared, description: 'Shared runner' + + expect(runner.project?).to be false + end + + it 'returns true when the runner is assigned to a project' do + project = create :project + runner = create :ci_runner, description: 'Group runner', projects: [project] + + expect(runner.project?).to be true + end + end end -- cgit v1.2.1 From d588adff1a3ce87355c8b5ac09a77e6fc63fe89a Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 4 Oct 2017 15:51:23 +0200 Subject: don't filter group runners by project flag the scope `Ci::Runner.belonging_to_group` does not filter out the runners where the projects has `#group_runners_enabled` set to false anymore. it didn't show up in the runners UI anymore when group runners were disabled. this was confusing. the flag is only relevant when selecting appropriate runner for a build. --- spec/models/ci/runner_spec.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 308db9e8e68..0cc52acd44d 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -93,14 +93,6 @@ describe Ci::Runner do expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] end - - it 'does not return the group runner if the project has group runners disabled' do - specific_group = create :group - specific_project = create :project, group: specific_group, group_runners_enabled: false - create :ci_runner, :specific, groups: [specific_group] - - expect(described_class.belonging_to_group(specific_project.id)).to be_empty - end end describe '.owned_or_shared' do -- cgit v1.2.1 From 8d61d33d37f7b8f99eae73e4ba0b48fbe35a80dc Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 4 Oct 2017 17:11:53 +0200 Subject: use .owned_or_shared for #assignable_for? instead of having the explicit logic duplicated from the scope we can use the scope instead. --- spec/models/ci/runner_spec.rb | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0cc52acd44d..1456c44dbf6 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -236,6 +236,13 @@ describe Ci::Runner do build.project.runners << runner end + context 'a different runner' do + it 'cannot handle builds' do + other_runner = create :ci_runner + expect(other_runner.can_pick?(build)).to be_falsey + end + end + context 'when runner does not have tags' do it 'can handle builds without tags' do expect(runner.can_pick?(build)).to be_truthy @@ -277,7 +284,7 @@ describe Ci::Runner do context 'when runner cannot pick untagged jobs' do before do - runner.run_untagged = false + runner.update_attributes!(run_untagged: false) end it 'cannot handle builds without tags' do @@ -290,7 +297,7 @@ describe Ci::Runner do context 'when runner is shared' do before do - runner.is_shared = true + runner.update_attributes!(is_shared: true) build.project.runners = [] end @@ -300,7 +307,7 @@ describe Ci::Runner do context 'when runner is locked' do before do - runner.locked = true + runner.update_attributes!(locked: true) end it 'can handle builds' do @@ -325,6 +332,17 @@ describe Ci::Runner do expect(runner.can_pick?(build)).to be_falsey end end + + context 'when runner is assigned to a group' do + before do + build.project.runners = [] + runner.groups << create(:group, projects: [build.project]) + end + + it 'can handle builds' do + expect(runner.can_pick?(build)).to be_truthy + end + end end context 'when access_level of runner is not_protected' do -- cgit v1.2.1 From a5f5a27df5d3055612fc8c2686ef3b9ab20bd85e Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 5 Oct 2017 13:53:18 +0200 Subject: include group runners in Project#any_runners? --- spec/models/project_spec.rb | 87 +++++++++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 26 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2675c2f52c1..9657e5011b1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1139,44 +1139,79 @@ describe Project do end describe '#any_runners' do - let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) } - let(:specific_runner) { create(:ci_runner) } - let(:shared_runner) { create(:ci_runner, :shared) } + context 'shared runners' do + let(:project) { create :project, shared_runners_enabled: shared_runners_enabled } + let(:specific_runner) { create :ci_runner } + let(:shared_runner) { create :ci_runner, :shared } - context 'for shared runners disabled' do - let(:shared_runners_enabled) { false } + context 'for shared runners disabled' do + let(:shared_runners_enabled) { false } - it 'has no runners available' do - expect(project.any_runners?).to be_falsey - end + it 'has no runners available' do + expect(project.any_runners?).to be_falsey + end - it 'has a specific runner' do - project.runners << specific_runner - expect(project.any_runners?).to be_truthy - end + it 'has a specific runner' do + project.runners << specific_runner + expect(project.any_runners?).to be_truthy + end + + it 'has a shared runner, but they are prohibited to use' do + shared_runner + expect(project.any_runners?).to be_falsey + end - it 'has a shared runner, but they are prohibited to use' do - shared_runner - expect(project.any_runners?).to be_falsey + it 'checks the presence of specific runner' do + project.runners << specific_runner + expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + end end - it 'checks the presence of specific runner' do - project.runners << specific_runner - expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy + context 'for shared runners enabled' do + let(:shared_runners_enabled) { true } + + it 'has a shared runner' do + shared_runner + expect(project.any_runners?).to be_truthy + end + + it 'checks the presence of shared runner' do + shared_runner + expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + end end end - context 'for shared runners enabled' do - let(:shared_runners_enabled) { true } + context 'group runners' do + let(:project) { create :project, group_runners_enabled: group_runners_enabled } + let(:group) { create :group, projects: [project] } + let(:group_runner) { create :ci_runner, groups: [group] } + + context 'for group runners disabled' do + let(:group_runners_enabled) { false } - it 'has a shared runner' do - shared_runner - expect(project.any_runners?).to be_truthy + it 'has no runners available' do + expect(project.any_runners?).to be_falsey + end + + it 'has a group runner, but they are prohibited to use' do + group_runner + expect(project.any_runners?).to be_falsey + end end - it 'checks the presence of shared runner' do - shared_runner - expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy + context 'for group runners enabled' do + let(:group_runners_enabled) { true } + + it 'has a group runner' do + group_runner + expect(project.any_runners?).to be_truthy + end + + it 'checks the presence of group runner' do + group_runner + expect(project.any_runners? { |runner| runner == group_runner }).to be_truthy + end end end end -- cgit v1.2.1 From 9b836b83bcabb4b977afc0e06897c4f1509215b0 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 5 Oct 2017 20:53:24 +0200 Subject: support group hierarchies for group runners --- spec/models/ci/runner_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 1456c44dbf6..ba0db43a1e7 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -93,6 +93,15 @@ describe Ci::Runner do expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] end + + it 'returns the group runner from a parent group' do + parent_group = create :group + group = create :group, parent: parent_group + project = create :project, group: group + runner = create :ci_runner, :specific, groups: [parent_group] + + expect(described_class.belonging_to_group(project.id)).to eq [runner] + end end describe '.owned_or_shared' do -- cgit v1.2.1 From a2a7ad291f64a5db74c1bc21fb556e6e8862d0f3 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 8 Nov 2017 10:21:54 +0100 Subject: add project_settings (Project#settings) --- spec/models/project_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9657e5011b1..e0f442a334c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -115,6 +115,26 @@ describe Project do expect(subject.boards.size).to eq 1 end end + + describe '#settings' do + it 'creates lazily a settings record when the project does not have one associated' do + project = create :project + expect(ProjectSettings.count).to eq 0 + + expect(project.settings).to be_a ProjectSettings + + expect(ProjectSettings.count).to eq 1 + end + + it 'returns the associated record when the project has one associated' do + project = create :project, settings: create(:project_settings) + expect(ProjectSettings.count).to eq 1 + + expect(project.settings).to be_a ProjectSettings + + expect(ProjectSettings.count).to eq 1 + end + end end describe 'modules' do -- cgit v1.2.1 From dd785467393610a73da6e9fd8413bca685d9356c Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 8 Nov 2017 13:08:13 +0100 Subject: project#group_runner_enabled -> project_settings --- spec/models/project_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e0f442a334c..ed28cd50e4d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3640,4 +3640,16 @@ describe Project do it { is_expected.not_to be_valid } end end + + describe '#toggle_settings!' do + it 'toggles the value on #settings' do + project = create :project, group_runners_enabled: false + + expect(project.group_runners_enabled).to be false + + project.toggle_settings!(:group_runners_enabled) + + expect(project.group_runners_enabled).to be true + end + end end -- cgit v1.2.1 From 316ccb64a738d376de57b9df76dbec732f0d9eff Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 9 Nov 2017 15:30:08 +0100 Subject: add `active` scope only once, inline methods --- spec/models/project_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ed28cd50e4d..9c78f7b8bd6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -63,7 +63,6 @@ describe Project do it { is_expected.to have_many(:build_trace_section_names)} it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } - it { is_expected.to have_many(:active_runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:pages_domains) } -- cgit v1.2.1 From 1acd8eb740dd070a5290d8a36c03e1b6f9691dba Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 7 Dec 2017 17:21:16 +0100 Subject: ci runners: assigned to either projects or group --- spec/models/ci/runner_spec.rb | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ba0db43a1e7..fb724f682a5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -19,6 +19,63 @@ describe Ci::Runner do end end end + + context 'either_projects_or_group' do + it 'disallows assigning to a group if already assigned to a group' do + group = create(:group) + runner = create(:ci_runner, groups: [group]) + + runner.groups << build(:group) + + expect(runner).not_to be_valid + expect(runner.errors.full_messages).to eq ['Runner can only be assigned to one group'] + end + + it 'disallows assigning to a group if already assigned to a project' do + project = create(:project) + runner = create(:ci_runner, projects: [project]) + + runner.groups << build(:group) + + expect(runner).not_to be_valid + expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + end + + it 'disallows assigning to a project if already assigned to a group' do + group = create(:group) + runner = create(:ci_runner, groups: [group]) + + runner.projects << build(:project) + + expect(runner).not_to be_valid + expect(runner.errors.full_messages).to eq ['Runner can only be assigned either to projects or to a group'] + end + + it 'allows assigning to a group if not assigned to a group nor a project' do + runner = create(:ci_runner) + + runner.groups << build(:group) + + expect(runner).to be_valid + end + + it 'allows assigning to a project if not assigned to a group nor a project' do + runner = create(:ci_runner) + + runner.projects << build(:project) + + expect(runner).to be_valid + end + + it 'allows assigning to a project if already assigned to a project' do + project = create(:project) + runner = create(:ci_runner, projects: [project]) + + runner.projects << build(:project) + + expect(runner).to be_valid + end + end end describe '#access_level' do -- cgit v1.2.1 From 9bed8de9100a394257a4a55e8b87bcfd015f0fbd Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 19 Dec 2017 15:12:21 +0100 Subject: simplify runner selection don't differentiate between the different runner types, instead we rely on the Runner model to provide the available projects. scheduling is now applied to all runners equally. --- spec/models/ci/runner_spec.rb | 67 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index fb724f682a5..512a490d289 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -776,4 +776,71 @@ describe Ci::Runner do expect(runner.project?).to be true end end + + describe '#accessible_projects' do + let!(:shared_runner) { create(:ci_runner, :shared) } + let!(:shared_project) { create(:project, shared_runners_enabled: true) } + + let!(:project_runner) { create(:ci_runner) } + let!(:project_project) { create(:project, runners: [project_runner], shared_runners_enabled: false) } + + let!(:group_runner) { create(:ci_runner) } + + let!(:parent_group) { create(:group) } + let!(:parent_group_project) do + create(:project, group: parent_group, shared_runners_enabled: false) + end + + let!(:group) { create :group, runners: [group_runner], parent: parent_group } + let!(:group_project) do + create(:project, group: group, shared_runners_enabled: false) + end + + let!(:nested_group_project) do + nested_group = create :group, parent: group + create(:project, group: nested_group, shared_runners_enabled: false) + end + + it 'returns the project with a shared runner' do + expect(shared_runner.reload.accessible_projects).to eq [shared_project] + end + + it 'returns the project with a project runner' do + expect(project_runner.reload.accessible_projects).to eq [project_project] + end + + it 'returns the projects with a group and nested group runner' do + expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project] + end + + context 'deleted' do + before do + shared_project.update_attributes!(pending_delete: true) + project_project.update_attributes!(pending_delete: true) + group_project.update_attributes!(pending_delete: true) + nested_group_project.update_attributes!(pending_delete: true) + end + + it 'returns no projects' do + expect(shared_runner.reload.accessible_projects).to be_empty + expect(project_runner.reload.accessible_projects).to be_empty + expect(group_runner.reload.accessible_projects).to be_empty + end + end + + context 'builds disabled' do + before do + shared_project.update_attributes!(builds_enabled: false) + project_project.update_attributes!(builds_enabled: false) + group_project.update_attributes!(builds_enabled: false) + nested_group_project.update_attributes!(builds_enabled: false) + end + + it 'returns no projects' do + expect(shared_runner.reload.accessible_projects).to be_empty + expect(project_runner.reload.accessible_projects).to be_empty + expect(group_runner.reload.accessible_projects).to be_empty + end + end + end end -- cgit v1.2.1 From c585004b59e5fbd5e925dacb7259916240d1cf5a Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 19 Dec 2017 16:40:19 +0100 Subject: restrict projects ci controller to project runners --- spec/models/ci/runner_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 512a490d289..3e85e3c92e3 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -136,6 +136,21 @@ describe Ci::Runner do end end + describe '.belonging_to_any_project' do + it 'returns the specific project runner' do + # project + project_project = create :project + project_runner = create :ci_runner, :specific, projects: [project_project] + + # group + group = create :group + create :project, group: group + create :ci_runner, :specific, groups: [group] + + expect(described_class.belonging_to_any_project).to eq [project_runner] + end + end + describe '.belonging_to_group' do it 'returns the specific group runner' do # own -- cgit v1.2.1 From 9447e5c27d8f840eaf4eee9635a5149ab36d93b6 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Mon, 26 Feb 2018 16:20:29 +0100 Subject: extract method to adhere to "tell, don't ask" --- spec/models/ci/runner_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 3e85e3c92e3..23e18efea70 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -858,4 +858,30 @@ describe Ci::Runner do end end end + + describe '#invalidate_build_cache!' do + context 'runner can pick the build' do + it 'calls #tick_runner_queue' do + ci_build = build :ci_build + runner = build :ci_runner + allow(runner).to receive(:can_pick?).with(ci_build).and_return(true) + + expect(runner).to receive(:tick_runner_queue) + + runner.invalidate_build_cache!(ci_build) + end + end + + context 'runner cannot pick the build' do + it 'does not call #tick_runner_queue' do + ci_build = build :ci_build + runner = build :ci_runner + allow(runner).to receive(:can_pick?).with(ci_build).and_return(false) + + expect(runner).not_to receive(:tick_runner_queue) + + runner.invalidate_build_cache!(ci_build) + end + end + end end -- cgit v1.2.1 From ac6eb51b806823c9e953b30f537014a4b329828b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 24 Apr 2018 14:16:42 +0900 Subject: Rename SwapTraceChunkWorker to BuildTraceSwapChunkWorker --- spec/models/ci/job_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb index eb240de188f..b01dfd097e6 100644 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ b/spec/models/ci/job_trace_chunk_spec.rb @@ -71,7 +71,7 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do let(:value) { 'a' * described_class::CHUNK_SIZE } it 'schedules stashing data' do - expect(SwapTraceChunkWorker).to receive(:perform_async).once + expect(BuildTraceSwapChunkWorker).to receive(:perform_async).once subject end @@ -108,7 +108,7 @@ describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do context 'when fullfilled chunk size' do it 'does not schedule stashing data' do - expect(SwapTraceChunkWorker).not_to receive(:perform_async) + expect(BuildTraceSwapChunkWorker).not_to receive(:perform_async) subject end -- cgit v1.2.1 From 92cbf9453a4435976bfe77cafd0f8c5f57833e59 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 26 Apr 2018 10:39:04 +0800 Subject: Switch to using ProjectCiCdSetting for group_runners_enabled and remove ProjectSettings --- spec/models/project_spec.rb | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 265941acbe7..7392b75e37c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -123,26 +123,6 @@ describe Project do expect(subject.boards.size).to eq 1 end end - - describe '#settings' do - it 'creates lazily a settings record when the project does not have one associated' do - project = create :project - expect(ProjectSettings.count).to eq 0 - - expect(project.settings).to be_a ProjectSettings - - expect(ProjectSettings.count).to eq 1 - end - - it 'returns the associated record when the project has one associated' do - project = create :project, settings: create(:project_settings) - expect(ProjectSettings.count).to eq 1 - - expect(project.settings).to be_a ProjectSettings - - expect(ProjectSettings.count).to eq 1 - end - end end describe 'modules' do @@ -3595,13 +3575,13 @@ describe Project do end end - describe '#toggle_settings!' do + describe '#toggle_ci_cd_settings!' do it 'toggles the value on #settings' do project = create :project, group_runners_enabled: false expect(project.group_runners_enabled).to be false - project.toggle_settings!(:group_runners_enabled) + project.toggle_ci_cd_settings!(:group_runners_enabled) expect(project.group_runners_enabled).to be true end -- cgit v1.2.1 From 9d6fe7bfdf9ff3f68ee73baa0e3d0aa7df13c351 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 26 Apr 2018 15:06:04 +0900 Subject: Refactoring ci_job_trace to ci_build_trace --- spec/models/ci/build_trace_chunk_spec.rb | 391 +++++++++++++++++++++++++++++++ spec/models/ci/job_trace_chunk_spec.rb | 391 ------------------------------- 2 files changed, 391 insertions(+), 391 deletions(-) create mode 100644 spec/models/ci/build_trace_chunk_spec.rb delete mode 100644 spec/models/ci/job_trace_chunk_spec.rb (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb new file mode 100644 index 00000000000..988e23146d2 --- /dev/null +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -0,0 +1,391 @@ +require 'spec_helper' + +describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do + set(:build) { create(:ci_build, :running) } + let(:chunk_index) { 0 } + let(:data_store) { :redis } + let(:raw_data) { nil } + + let(:build_trace_chunk) do + described_class.new(build: build, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) + end + + describe 'CHUNK_SIZE' do + it 'Chunk size can not be changed without special care' do + expect(described_class::CHUNK_SIZE).to eq(128.kilobytes) + end + end + + describe '#data' do + subject { build_trace_chunk.data } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + before do + build_trace_chunk.send(:redis_set_data, 'Sample data in redis') + end + + it { is_expected.to eq('Sample data in redis') } + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + + it { is_expected.to eq('Sample data in db') } + end + + context 'when data_store is others' do + before do + build_trace_chunk.send(:write_attribute, :data_store, -1) + end + + it { expect { subject }.to raise_error('Unsupported data store') } + end + end + + describe '#set_data' do + subject { build_trace_chunk.set_data(value) } + + let(:value) { 'Sample data' } + + context 'when value bytesize is bigger than CHUNK_SIZE' do + let(:value) { 'a' * (described_class::CHUNK_SIZE + 1) } + + it { expect { subject }.to raise_error('too much data') } + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + + it do + expect(build_trace_chunk.send(:redis_data)).to be_nil + + subject + + expect(build_trace_chunk.send(:redis_data)).to eq(value) + end + + context 'when fullfilled chunk size' do + let(:value) { 'a' * described_class::CHUNK_SIZE } + + it 'schedules stashing data' do + expect(BuildTraceSwapChunkWorker).to receive(:perform_async).once + + subject + end + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + it 'sets data' do + expect(build_trace_chunk.raw_data).to be_nil + + subject + + expect(build_trace_chunk.raw_data).to eq(value) + expect(build_trace_chunk.persisted?).to be_truthy + end + + context 'when raw_data is not changed' do + it 'does not execute UPDATE' do + expect(build_trace_chunk.raw_data).to be_nil + build_trace_chunk.save! + + # First set + expect(ActiveRecord::QueryRecorder.new { subject }.count).to be > 0 + expect(build_trace_chunk.raw_data).to eq(value) + expect(build_trace_chunk.persisted?).to be_truthy + + # Second set + build_trace_chunk.reload + expect(ActiveRecord::QueryRecorder.new { subject }.count).to be(0) + end + end + + context 'when fullfilled chunk size' do + it 'does not schedule stashing data' do + expect(BuildTraceSwapChunkWorker).not_to receive(:perform_async) + + subject + end + end + end + + context 'when data_store is others' do + before do + build_trace_chunk.send(:write_attribute, :data_store, -1) + end + + it { expect { subject }.to raise_error('Unsupported data store') } + end + end + + describe '#truncate' do + subject { build_trace_chunk.truncate(offset) } + + shared_examples_for 'truncates' do + context 'when offset is negative' do + let(:offset) { -1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:offset) { data.bytesize + 1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is 10' do + let(:offset) { 10 } + + it 'truncates' do + subject + + expect(build_trace_chunk.data).to eq(data.byteslice(0, offset)) + end + end + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + let(:data) { 'Sample data in redis' } + + before do + build_trace_chunk.send(:redis_set_data, data) + end + + it_behaves_like 'truncates' + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it_behaves_like 'truncates' + end + end + + describe '#append' do + subject { build_trace_chunk.append(new_data, offset) } + + let(:new_data) { 'Sample new data' } + let(:offset) { 0 } + let(:total_data) { data + new_data } + + shared_examples_for 'appends' do + context 'when offset is negative' do + let(:offset) { -1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:offset) { data.bytesize + 1 } + + it { expect { subject }.to raise_error('Offset is out of bound') } + end + + context 'when offset is bigger than data size' do + let(:new_data) { 'a' * (described_class::CHUNK_SIZE + 1) } + + it { expect { subject }.to raise_error('Outside of chunk size') } + end + + context 'when offset is EOF' do + let(:offset) { data.bytesize } + + it 'appends' do + subject + + expect(build_trace_chunk.data).to eq(total_data) + end + end + + context 'when offset is 10' do + let(:offset) { 10 } + + it 'appends' do + subject + + expect(build_trace_chunk.data).to eq(data.byteslice(0, offset) + new_data) + end + end + end + + context 'when data_store is redis' do + let(:data_store) { :redis } + let(:data) { 'Sample data in redis' } + + before do + build_trace_chunk.send(:redis_set_data, data) + end + + it_behaves_like 'appends' + end + + context 'when data_store is database' do + let(:data_store) { :db } + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it_behaves_like 'appends' + end + end + + describe '#size' do + subject { build_trace_chunk.size } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + context 'when data exists' do + let(:data) { 'Sample data in redis' } + + before do + build_trace_chunk.send(:redis_set_data, data) + end + + it { is_expected.to eq(data.bytesize) } + end + + context 'when data exists' do + it { is_expected.to eq(0) } + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + context 'when data exists' do + let(:raw_data) { 'Sample data in db' } + let(:data) { raw_data } + + it { is_expected.to eq(data.bytesize) } + end + + context 'when data does not exist' do + it { is_expected.to eq(0) } + end + end + end + + describe '#use_database!' do + subject { build_trace_chunk.use_database! } + + context 'when data_store is redis' do + let(:data_store) { :redis } + + context 'when data exists' do + let(:data) { 'Sample data in redis' } + + before do + build_trace_chunk.send(:redis_set_data, data) + end + + it 'stashes the data' do + expect(build_trace_chunk.data_store).to eq('redis') + expect(build_trace_chunk.send(:redis_data)).to eq(data) + expect(build_trace_chunk.raw_data).to be_nil + + subject + + expect(build_trace_chunk.data_store).to eq('db') + expect(build_trace_chunk.send(:redis_data)).to be_nil + expect(build_trace_chunk.raw_data).to eq(data) + end + end + + context 'when data does not exist' do + it 'does not call UPDATE' do + expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) + end + end + end + + context 'when data_store is database' do + let(:data_store) { :db } + + it 'does not call UPDATE' do + expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) + end + end + end + + describe 'ExclusiveLock' do + before do + allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil } + stub_const('Ci::BuildTraceChunk::LOCK_RETRY', 1) + end + + it 'raise an error' do + expect { build_trace_chunk.append('ABC', 0) }.to raise_error('Failed to obtain write lock') + end + end + + describe 'deletes data in redis after chunk record destroyed' do + let(:project) { create(:project) } + + before do + pipeline = create(:ci_pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + end + + shared_examples_for 'deletes all build_trace_chunk and data in redis' do + it do + project.builds.each do |build| + Gitlab::Redis::SharedState.with do |redis| + redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + expect(redis.exists(key)).to be_truthy + end + end + end + + expect(described_class.count).not_to eq(0) + + subject + + expect(described_class.count).to eq(0) + + project.builds.each do |build| + Gitlab::Redis::SharedState.with do |redis| + redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + expect(redis.exists(key)).to be_falsey + end + end + end + end + end + + context 'when build_trace_chunk is destroyed' do + let(:subject) do + project.builds.each { |build| build.chunks.destroy_all } + end + + it_behaves_like 'deletes all build_trace_chunk and data in redis' + end + + context 'when build is destroyed' do + let(:subject) do + project.builds.destroy_all + end + + it_behaves_like 'deletes all build_trace_chunk and data in redis' + end + + context 'when project is destroyed' do + let(:subject) do + project.destroy! + end + + it_behaves_like 'deletes all build_trace_chunk and data in redis' + end + end +end diff --git a/spec/models/ci/job_trace_chunk_spec.rb b/spec/models/ci/job_trace_chunk_spec.rb deleted file mode 100644 index b01dfd097e6..00000000000 --- a/spec/models/ci/job_trace_chunk_spec.rb +++ /dev/null @@ -1,391 +0,0 @@ -require 'spec_helper' - -describe Ci::JobTraceChunk, :clean_gitlab_redis_shared_state do - set(:job) { create(:ci_build, :running) } - let(:chunk_index) { 0 } - let(:data_store) { :redis } - let(:raw_data) { nil } - - let(:job_trace_chunk) do - described_class.new(job: job, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) - end - - describe 'CHUNK_SIZE' do - it 'Chunk size can not be changed without special care' do - expect(described_class::CHUNK_SIZE).to eq(128.kilobytes) - end - end - - describe '#data' do - subject { job_trace_chunk.data } - - context 'when data_store is redis' do - let(:data_store) { :redis } - - before do - job_trace_chunk.send(:redis_set_data, 'Sample data in redis') - end - - it { is_expected.to eq('Sample data in redis') } - end - - context 'when data_store is database' do - let(:data_store) { :db } - let(:raw_data) { 'Sample data in db' } - - it { is_expected.to eq('Sample data in db') } - end - - context 'when data_store is others' do - before do - job_trace_chunk.send(:write_attribute, :data_store, -1) - end - - it { expect { subject }.to raise_error('Unsupported data store') } - end - end - - describe '#set_data' do - subject { job_trace_chunk.set_data(value) } - - let(:value) { 'Sample data' } - - context 'when value bytesize is bigger than CHUNK_SIZE' do - let(:value) { 'a' * (described_class::CHUNK_SIZE + 1) } - - it { expect { subject }.to raise_error('too much data') } - end - - context 'when data_store is redis' do - let(:data_store) { :redis } - - it do - expect(job_trace_chunk.send(:redis_data)).to be_nil - - subject - - expect(job_trace_chunk.send(:redis_data)).to eq(value) - end - - context 'when fullfilled chunk size' do - let(:value) { 'a' * described_class::CHUNK_SIZE } - - it 'schedules stashing data' do - expect(BuildTraceSwapChunkWorker).to receive(:perform_async).once - - subject - end - end - end - - context 'when data_store is database' do - let(:data_store) { :db } - - it 'sets data' do - expect(job_trace_chunk.raw_data).to be_nil - - subject - - expect(job_trace_chunk.raw_data).to eq(value) - expect(job_trace_chunk.persisted?).to be_truthy - end - - context 'when raw_data is not changed' do - it 'does not execute UPDATE' do - expect(job_trace_chunk.raw_data).to be_nil - job_trace_chunk.save! - - # First set - expect(ActiveRecord::QueryRecorder.new { subject }.count).to be > 0 - expect(job_trace_chunk.raw_data).to eq(value) - expect(job_trace_chunk.persisted?).to be_truthy - - # Second set - job_trace_chunk.reload - expect(ActiveRecord::QueryRecorder.new { subject }.count).to be(0) - end - end - - context 'when fullfilled chunk size' do - it 'does not schedule stashing data' do - expect(BuildTraceSwapChunkWorker).not_to receive(:perform_async) - - subject - end - end - end - - context 'when data_store is others' do - before do - job_trace_chunk.send(:write_attribute, :data_store, -1) - end - - it { expect { subject }.to raise_error('Unsupported data store') } - end - end - - describe '#truncate' do - subject { job_trace_chunk.truncate(offset) } - - shared_examples_for 'truncates' do - context 'when offset is negative' do - let(:offset) { -1 } - - it { expect { subject }.to raise_error('Offset is out of bound') } - end - - context 'when offset is bigger than data size' do - let(:offset) { data.bytesize + 1 } - - it { expect { subject }.to raise_error('Offset is out of bound') } - end - - context 'when offset is 10' do - let(:offset) { 10 } - - it 'truncates' do - subject - - expect(job_trace_chunk.data).to eq(data.byteslice(0, offset)) - end - end - end - - context 'when data_store is redis' do - let(:data_store) { :redis } - let(:data) { 'Sample data in redis' } - - before do - job_trace_chunk.send(:redis_set_data, data) - end - - it_behaves_like 'truncates' - end - - context 'when data_store is database' do - let(:data_store) { :db } - let(:raw_data) { 'Sample data in db' } - let(:data) { raw_data } - - it_behaves_like 'truncates' - end - end - - describe '#append' do - subject { job_trace_chunk.append(new_data, offset) } - - let(:new_data) { 'Sample new data' } - let(:offset) { 0 } - let(:total_data) { data + new_data } - - shared_examples_for 'appends' do - context 'when offset is negative' do - let(:offset) { -1 } - - it { expect { subject }.to raise_error('Offset is out of bound') } - end - - context 'when offset is bigger than data size' do - let(:offset) { data.bytesize + 1 } - - it { expect { subject }.to raise_error('Offset is out of bound') } - end - - context 'when offset is bigger than data size' do - let(:new_data) { 'a' * (described_class::CHUNK_SIZE + 1) } - - it { expect { subject }.to raise_error('Outside of chunk size') } - end - - context 'when offset is EOF' do - let(:offset) { data.bytesize } - - it 'appends' do - subject - - expect(job_trace_chunk.data).to eq(total_data) - end - end - - context 'when offset is 10' do - let(:offset) { 10 } - - it 'appends' do - subject - - expect(job_trace_chunk.data).to eq(data.byteslice(0, offset) + new_data) - end - end - end - - context 'when data_store is redis' do - let(:data_store) { :redis } - let(:data) { 'Sample data in redis' } - - before do - job_trace_chunk.send(:redis_set_data, data) - end - - it_behaves_like 'appends' - end - - context 'when data_store is database' do - let(:data_store) { :db } - let(:raw_data) { 'Sample data in db' } - let(:data) { raw_data } - - it_behaves_like 'appends' - end - end - - describe '#size' do - subject { job_trace_chunk.size } - - context 'when data_store is redis' do - let(:data_store) { :redis } - - context 'when data exists' do - let(:data) { 'Sample data in redis' } - - before do - job_trace_chunk.send(:redis_set_data, data) - end - - it { is_expected.to eq(data.bytesize) } - end - - context 'when data exists' do - it { is_expected.to eq(0) } - end - end - - context 'when data_store is database' do - let(:data_store) { :db } - - context 'when data exists' do - let(:raw_data) { 'Sample data in db' } - let(:data) { raw_data } - - it { is_expected.to eq(data.bytesize) } - end - - context 'when data does not exist' do - it { is_expected.to eq(0) } - end - end - end - - describe '#use_database!' do - subject { job_trace_chunk.use_database! } - - context 'when data_store is redis' do - let(:data_store) { :redis } - - context 'when data exists' do - let(:data) { 'Sample data in redis' } - - before do - job_trace_chunk.send(:redis_set_data, data) - end - - it 'stashes the data' do - expect(job_trace_chunk.data_store).to eq('redis') - expect(job_trace_chunk.send(:redis_data)).to eq(data) - expect(job_trace_chunk.raw_data).to be_nil - - subject - - expect(job_trace_chunk.data_store).to eq('db') - expect(job_trace_chunk.send(:redis_data)).to be_nil - expect(job_trace_chunk.raw_data).to eq(data) - end - end - - context 'when data does not exist' do - it 'does not call UPDATE' do - expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) - end - end - end - - context 'when data_store is database' do - let(:data_store) { :db } - - it 'does not call UPDATE' do - expect(ActiveRecord::QueryRecorder.new { subject }.count).to eq(0) - end - end - end - - describe 'ExclusiveLock' do - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil } - stub_const('Ci::JobTraceChunk::LOCK_RETRY', 1) - end - - it 'raise an error' do - expect { job_trace_chunk.append('ABC', 0) }.to raise_error('Failed to obtain write lock') - end - end - - describe 'deletes data in redis after chunk record destroyed' do - let(:project) { create(:project) } - - before do - pipeline = create(:ci_pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) - end - - shared_examples_for 'deletes all job_trace_chunk and data in redis' do - it do - project.builds.each do |build| - Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| - expect(redis.exists(key)).to be_truthy - end - end - end - - expect(described_class.count).not_to eq(0) - - subject - - expect(described_class.count).to eq(0) - - project.builds.each do |build| - Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| - expect(redis.exists(key)).to be_falsey - end - end - end - end - end - - context 'when job_trace_chunk is destroyed' do - let(:subject) do - project.builds.each { |build| build.chunks.destroy_all } - end - - it_behaves_like 'deletes all job_trace_chunk and data in redis' - end - - context 'when job is destroyed' do - let(:subject) do - project.builds.destroy_all - end - - it_behaves_like 'deletes all job_trace_chunk and data in redis' - end - - context 'when project is destroyed' do - let(:subject) do - project.destroy! - end - - it_behaves_like 'deletes all job_trace_chunk and data in redis' - end - end -end -- cgit v1.2.1 From 0f9f5e82ad5c4898e7c206ff0d574a3e141f3433 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 26 Apr 2018 14:43:17 +0800 Subject: Fix spec/models/user_spec.rb for #ci_authorized_runners --- spec/models/user_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 79447a65e94..df2e547ce28 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1897,7 +1897,7 @@ describe User do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } - let(:project) { create(:project, group: subgroup) } + let!(:project) { create(:project, group: subgroup, runners: [runner_1]) } def add_user(access) group.add_user(user, access) -- cgit v1.2.1 From ff219759303dc03f66bb1a160aafa2fca7ac9308 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Fri, 27 Apr 2018 08:05:46 +0800 Subject: Tag runner_spec tests for subgroups with nested_groups --- spec/models/ci/runner_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 23e18efea70..1df11fdedf4 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -166,7 +166,7 @@ describe Ci::Runner do expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] end - it 'returns the group runner from a parent group' do + it 'returns the group runner from a parent group', :nested_groups do parent_group = create :group group = create :group, parent: parent_group project = create :project, group: group @@ -824,7 +824,7 @@ describe Ci::Runner do expect(project_runner.reload.accessible_projects).to eq [project_project] end - it 'returns the projects with a group and nested group runner' do + it 'returns the projects with a group and nested group runner', :nested_groups do expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project] end -- cgit v1.2.1 From 329d535bbcf8ceafd3b000c5cd224173f441310b Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Fri, 27 Apr 2018 08:56:13 +0800 Subject: Add extra spec for Project#any_runners? to test block properly --- spec/models/project_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 7392b75e37c..ab0694e6890 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1165,6 +1165,11 @@ describe Project do project.runners << specific_runner expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy end + + it 'returns false if match cannot be found' do + project.runners << specific_runner + expect(project.any_runners? { false }).to be_falsey + end end context 'for shared runners enabled' do @@ -1179,6 +1184,11 @@ describe Project do shared_runner expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy end + + it 'returns false if match cannot be found' do + shared_runner + expect(project.any_runners? { false }).to be_falsey + end end end @@ -1212,6 +1222,11 @@ describe Project do group_runner expect(project.any_runners? { |runner| runner == group_runner }).to be_truthy end + + it 'returns false if match cannot be found' do + group_runner + expect(project.any_runners? { false }).to be_falsey + end end end end -- cgit v1.2.1 From 8d8139862aee97d7fadc0563e7df9842f5bd46ac Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Fri, 27 Apr 2018 09:15:54 +0800 Subject: Rename `runner.belonging_to_group(project.id) -> runner.belonging_to_parent_group_of_project(project.id)` --- spec/models/ci/runner_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 1df11fdedf4..fbf9539e698 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -151,7 +151,7 @@ describe Ci::Runner do end end - describe '.belonging_to_group' do + describe '.belonging_to_parent_group_of_project' do it 'returns the specific group runner' do # own specific_group = create :group @@ -163,7 +163,7 @@ describe Ci::Runner do create :project, group: other_group create :ci_runner, :specific, groups: [other_group] - expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner] + expect(described_class.belonging_to_parent_group_of_project(specific_project.id)).to eq [specific_runner] end it 'returns the group runner from a parent group', :nested_groups do @@ -172,7 +172,7 @@ describe Ci::Runner do project = create :project, group: group runner = create :ci_runner, :specific, groups: [parent_group] - expect(described_class.belonging_to_group(project.id)).to eq [runner] + expect(described_class.belonging_to_parent_group_of_project(project.id)).to eq [runner] end end -- cgit v1.2.1 From 270b5867306c9f38cfa73402a4b00bc8fd982719 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 27 Apr 2018 23:32:35 +0900 Subject: Fix static analysys for build_trace_chunk_spec.rb:8 --- spec/models/ci/build_trace_chunk_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 988e23146d2..81e58eca3ed 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -5,7 +5,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:chunk_index) { 0 } let(:data_store) { :redis } let(:raw_data) { nil } - + let(:build_trace_chunk) do described_class.new(build: build, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end -- cgit v1.2.1 From 23c8e198463d566d2e8d2351c315741903035a64 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 30 Apr 2018 14:52:29 +0900 Subject: Fix spec. Revert #truncate in stream (But still prevent redandant calls) --- spec/models/ci/build_trace_chunk_spec.rb | 43 +++++++++++++++----------------- 1 file changed, 20 insertions(+), 23 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 81e58eca3ed..a122ee84b3c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -46,7 +46,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end describe '#set_data' do - subject { build_trace_chunk.set_data(value) } + subject { build_trace_chunk.send(:set_data, value) } let(:value) { 'Sample data' } @@ -131,13 +131,17 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when offset is negative' do let(:offset) { -1 } - it { expect { subject }.to raise_error('Offset is out of bound') } + it { expect { subject }.to raise_error('Offset is out of range') } end context 'when offset is bigger than data size' do let(:offset) { data.bytesize + 1 } - it { expect { subject }.to raise_error('Offset is out of bound') } + it do + expect_any_instance_of(described_class).not_to receive(:append) { } + + subject + end end context 'when offset is 10' do @@ -182,19 +186,19 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when offset is negative' do let(:offset) { -1 } - it { expect { subject }.to raise_error('Offset is out of bound') } + it { expect { subject }.to raise_error('Offset is out of range') } end context 'when offset is bigger than data size' do let(:offset) { data.bytesize + 1 } - it { expect { subject }.to raise_error('Offset is out of bound') } + it { expect { subject }.to raise_error('Offset is out of range') } end context 'when offset is bigger than data size' do let(:new_data) { 'a' * (described_class::CHUNK_SIZE + 1) } - it { expect { subject }.to raise_error('Outside of chunk size') } + it { expect { subject }.to raise_error('Chunk size overflow') } end context 'when offset is EOF' do @@ -320,7 +324,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do describe 'ExclusiveLock' do before do allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain) { nil } - stub_const('Ci::BuildTraceChunk::LOCK_RETRY', 1) + stub_const('Ci::BuildTraceChunk::WRITE_LOCK_RETRY', 1) end it 'raise an error' do @@ -333,30 +337,31 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do before do pipeline = create(:ci_pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) - create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + @build_ids = [] + @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id + @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id + @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id end shared_examples_for 'deletes all build_trace_chunk and data in redis' do it do - project.builds.each do |build| + @build_ids.each do |build_id| Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + redis.scan_each(match: "gitlab:ci:trace:#{build_id}:chunks:?") do |key| expect(redis.exists(key)).to be_truthy end end end - expect(described_class.count).not_to eq(0) + expect(described_class.count).to eq(3) subject expect(described_class.count).to eq(0) - project.builds.each do |build| + @build_ids.each do |build_id| Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build.id}:chunks:?") do |key| + redis.scan_each(match: "gitlab:ci:trace:#{build_id}:chunks:?") do |key| expect(redis.exists(key)).to be_falsey end end @@ -364,14 +369,6 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - context 'when build_trace_chunk is destroyed' do - let(:subject) do - project.builds.each { |build| build.chunks.destroy_all } - end - - it_behaves_like 'deletes all build_trace_chunk and data in redis' - end - context 'when build is destroyed' do let(:subject) do project.builds.destroy_all -- cgit v1.2.1 From 87740df2ba7153439c30544f299b235632717738 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Mon, 30 Apr 2018 09:39:47 +0400 Subject: Revert fair scheduling for all builds Per discussion in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9646#note_65730532 this logic is being optimized elsewhere and it will simplify things if we make less changes to this code right now. --- spec/models/ci/runner_spec.rb | 67 ------------------------------------------- 1 file changed, 67 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index fbf9539e698..d6ce97a9b28 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -792,73 +792,6 @@ describe Ci::Runner do end end - describe '#accessible_projects' do - let!(:shared_runner) { create(:ci_runner, :shared) } - let!(:shared_project) { create(:project, shared_runners_enabled: true) } - - let!(:project_runner) { create(:ci_runner) } - let!(:project_project) { create(:project, runners: [project_runner], shared_runners_enabled: false) } - - let!(:group_runner) { create(:ci_runner) } - - let!(:parent_group) { create(:group) } - let!(:parent_group_project) do - create(:project, group: parent_group, shared_runners_enabled: false) - end - - let!(:group) { create :group, runners: [group_runner], parent: parent_group } - let!(:group_project) do - create(:project, group: group, shared_runners_enabled: false) - end - - let!(:nested_group_project) do - nested_group = create :group, parent: group - create(:project, group: nested_group, shared_runners_enabled: false) - end - - it 'returns the project with a shared runner' do - expect(shared_runner.reload.accessible_projects).to eq [shared_project] - end - - it 'returns the project with a project runner' do - expect(project_runner.reload.accessible_projects).to eq [project_project] - end - - it 'returns the projects with a group and nested group runner', :nested_groups do - expect(group_runner.reload.accessible_projects).to eq [group_project, nested_group_project] - end - - context 'deleted' do - before do - shared_project.update_attributes!(pending_delete: true) - project_project.update_attributes!(pending_delete: true) - group_project.update_attributes!(pending_delete: true) - nested_group_project.update_attributes!(pending_delete: true) - end - - it 'returns no projects' do - expect(shared_runner.reload.accessible_projects).to be_empty - expect(project_runner.reload.accessible_projects).to be_empty - expect(group_runner.reload.accessible_projects).to be_empty - end - end - - context 'builds disabled' do - before do - shared_project.update_attributes!(builds_enabled: false) - project_project.update_attributes!(builds_enabled: false) - group_project.update_attributes!(builds_enabled: false) - nested_group_project.update_attributes!(builds_enabled: false) - end - - it 'returns no projects' do - expect(shared_runner.reload.accessible_projects).to be_empty - expect(project_runner.reload.accessible_projects).to be_empty - expect(group_runner.reload.accessible_projects).to be_empty - end - end - end - describe '#invalidate_build_cache!' do context 'runner can pick the build' do it 'calls #tick_runner_queue' do -- cgit v1.2.1 From d5344617a8b57e4d6d15f22ad3d09d5af82100fe Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 30 Apr 2018 15:27:05 +0900 Subject: Fix spec when parent record is destroyed --- spec/models/ci/build_trace_chunk_spec.rb | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index a122ee84b3c..cab5db9ca06 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -332,25 +332,20 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - describe 'deletes data in redis after chunk record destroyed' do + describe 'deletes data in redis after a parent record destroyed' do let(:project) { create(:project) } before do pipeline = create(:ci_pipeline, project: project) - @build_ids = [] - @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id - @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id - @build_ids << create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project).id + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) + create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) end shared_examples_for 'deletes all build_trace_chunk and data in redis' do it do - @build_ids.each do |build_id| - Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build_id}:chunks:?") do |key| - expect(redis.exists(key)).to be_truthy - end - end + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.count).to eq(3) end expect(described_class.count).to eq(3) @@ -359,12 +354,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(described_class.count).to eq(0) - @build_ids.each do |build_id| - Gitlab::Redis::SharedState.with do |redis| - redis.scan_each(match: "gitlab:ci:trace:#{build_id}:chunks:?") do |key| - expect(redis.exists(key)).to be_falsey - end - end + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.count).to eq(0) end end end -- cgit v1.2.1 From 5652ff953cba9773edbcb677908fe3f18b103be3 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Mon, 30 Apr 2018 10:43:29 +0400 Subject: Rename Runner#group? -> #assigned_to_group? and Runner#project? -> #assigned_to_project? --- spec/models/ci/runner_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index d6ce97a9b28..fb9dcce9a7c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -748,47 +748,47 @@ describe Ci::Runner do end end - describe 'group?' do + describe 'assigned_to_group?' do it 'returns false when the runner is a project runner' do project = create :project runner = create :ci_runner, description: 'Project runner', projects: [project] - expect(runner.group?).to be false + expect(runner.assigned_to_group?).to be false end it 'returns false when the runner is a shared runner' do runner = create :ci_runner, :shared, description: 'Shared runner' - expect(runner.group?).to be false + expect(runner.assigned_to_group?).to be false end it 'returns true when the runner is assigned to a group' do group = create :group runner = create :ci_runner, description: 'Group runner', groups: [group] - expect(runner.group?).to be true + expect(runner.assigned_to_group?).to be true end end - describe 'project?' do + describe 'assigned_to_project?' do it 'returns false when the runner is a group prunner' do group = create :group runner = create :ci_runner, description: 'Group runner', groups: [group] - expect(runner.project?).to be false + expect(runner.assigned_to_project?).to be false end it 'returns false when the runner is a shared runner' do runner = create :ci_runner, :shared, description: 'Shared runner' - expect(runner.project?).to be false + expect(runner.assigned_to_project?).to be false end it 'returns true when the runner is assigned to a project' do project = create :project runner = create :ci_runner, description: 'Group runner', projects: [project] - expect(runner.project?).to be true + expect(runner.assigned_to_project?).to be true end end -- cgit v1.2.1 From e98438d4efdf3aebabe5938979f4924cf7cb47a8 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Mon, 30 Apr 2018 17:10:07 +0400 Subject: Revert changes to User#ci_authorized_users to defer group runner API changes to later --- spec/models/user_spec.rb | 66 ++++++++++-------------------------------------- 1 file changed, 13 insertions(+), 53 deletions(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index df2e547ce28..3f2eb58f009 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1787,12 +1787,14 @@ describe User do describe '#ci_authorized_runners' do let(:user) { create(:user) } - let(:runner_1) { create(:ci_runner) } - let(:runner_2) { create(:ci_runner) } + let(:runner) { create(:ci_runner) } - context 'without any projects nor groups' do - let!(:project) { create(:project, runners: [runner_1]) } - let!(:group) { create(:group) } + before do + project.runners << runner + end + + context 'without any projects' do + let(:project) { create(:project) } it 'does not load' do expect(user.ci_authorized_runners).to be_empty @@ -1801,38 +1803,10 @@ describe User do context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + let(:project) { create(:project, namespace: namespace) } it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner_1) - end - end - - context 'with personal group runner' do - let!(:project) { create(:project, runners: [runner_1]) } - let!(:group) do - create(:group, runners: [runner_2]).tap do |group| - group.add_owner(user) - end - end - - it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner_2) - end - end - - context 'with personal project and group runner' do - let(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } - - let!(:group) do - create(:group, runners: [runner_2]).tap do |group| - group.add_owner(user) - end - end - - it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner_1, runner_2) + expect(user.ci_authorized_runners).to contain_exactly(runner) end end @@ -1843,7 +1817,7 @@ describe User do end it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner_1) + expect(user.ci_authorized_runners).to contain_exactly(runner) end end @@ -1860,21 +1834,7 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let!(:project) { create(:project, group: group, runners: [runner_1]) } - - def add_user(access) - group.add_user(user, access) - end - - it_behaves_like :member - end - - context 'with groups runners' do - let!(:group) do - create(:group, runners: [runner_1]).tap do |group| - group.add_owner(user) - end - end + let(:project) { create(:project, group: group) } def add_user(access) group.add_user(user, access) @@ -1884,7 +1844,7 @@ describe User do end context 'with other projects runners' do - let!(:project) { create(:project, runners: [runner_1]) } + let(:project) { create(:project) } def add_user(access) project.add_role(user, access) @@ -1897,7 +1857,7 @@ describe User do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } - let!(:project) { create(:project, group: subgroup, runners: [runner_1]) } + let(:project) { create(:project, group: subgroup) } def add_user(access) group.add_user(user, access) -- cgit v1.2.1 From 0e5c1a89f0b6ecf88fc340194d292fccbde99782 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Tue, 1 May 2018 13:44:35 +0400 Subject: Fix spec/features/admin/admin_runners_spec.rb + test style improvements --- spec/models/ci/runner_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index fb9dcce9a7c..fa540f8d4fd 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -21,8 +21,9 @@ describe Ci::Runner do end context 'either_projects_or_group' do + let(:group) { create(:group) } + it 'disallows assigning to a group if already assigned to a group' do - group = create(:group) runner = create(:ci_runner, groups: [group]) runner.groups << build(:group) @@ -42,7 +43,6 @@ describe Ci::Runner do end it 'disallows assigning to a project if already assigned to a group' do - group = create(:group) runner = create(:ci_runner, groups: [group]) runner.projects << build(:project) @@ -189,9 +189,9 @@ describe Ci::Runner do # globally shared shared_runner = create :ci_runner, :shared - expect(described_class.owned_or_shared(project.id)).to match_array [ + expect(described_class.owned_or_shared(project.id)).to contain_exactly( group_runner, project_runner, shared_runner - ] + ) end end -- cgit v1.2.1 From 6ed91266d021c2058697d12a0123bec9b00548ad Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 2 May 2018 14:27:28 +0900 Subject: Skip truncate when offset == size. Fix static analysys. --- spec/models/ci/build_trace_chunk_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index cab5db9ca06..b0ede29669e 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -345,7 +345,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do shared_examples_for 'deletes all build_trace_chunk and data in redis' do it do Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.count).to eq(3) + expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.size).to eq(3) end expect(described_class.count).to eq(3) @@ -355,14 +355,16 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(described_class.count).to eq(0) Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.count).to eq(0) + expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.size).to eq(0) end end end - context 'when build is destroyed' do + context 'when traces are archived' do let(:subject) do - project.builds.destroy_all + project.builds.each do |build| + build.success! + end end it_behaves_like 'deletes all build_trace_chunk and data in redis' -- cgit v1.2.1 From abe87373ab209c144cf684b57b12262a8df60540 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 2 May 2018 16:20:56 +0900 Subject: Enable feature flag on build_trace_chunk_spec --- spec/models/ci/build_trace_chunk_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index b0ede29669e..99fbba4afa9 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -10,6 +10,10 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do described_class.new(build: build, chunk_index: chunk_index, data_store: data_store, raw_data: raw_data) end + before do + stub_feature_flags(ci_enable_live_trace: true) + end + describe 'CHUNK_SIZE' do it 'Chunk size can not be changed without special care' do expect(described_class::CHUNK_SIZE).to eq(128.kilobytes) -- cgit v1.2.1 From 950df8babf07d7135b1f7cf95586d40f0ca36ff6 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 2 May 2018 16:52:54 +0900 Subject: Rename BuildTraceSwapChunkWorker to BuildTraceChunkFlushToDBWorker --- spec/models/ci/build_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 99fbba4afa9..4a58971a441 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -75,7 +75,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:value) { 'a' * described_class::CHUNK_SIZE } it 'schedules stashing data' do - expect(BuildTraceSwapChunkWorker).to receive(:perform_async).once + expect(BuildTraceChunkFlushToDBWorker).to receive(:perform_async).once subject end @@ -112,7 +112,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when fullfilled chunk size' do it 'does not schedule stashing data' do - expect(BuildTraceSwapChunkWorker).not_to receive(:perform_async) + expect(BuildTraceChunkFlushToDBWorker).not_to receive(:perform_async) subject end -- cgit v1.2.1 From 7d626c41ccb4531dceeb1d1025c067bac3c63c4d Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 2 May 2018 21:22:08 +0900 Subject: Fix BuildTraceChunkFlushToDbWorker name --- spec/models/ci/build_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 4a58971a441..dbeee74857c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -75,7 +75,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:value) { 'a' * described_class::CHUNK_SIZE } it 'schedules stashing data' do - expect(BuildTraceChunkFlushToDBWorker).to receive(:perform_async).once + expect(BuildTraceChunkFlushToDbWorker).to receive(:perform_async).once subject end @@ -112,7 +112,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when fullfilled chunk size' do it 'does not schedule stashing data' do - expect(BuildTraceChunkFlushToDBWorker).not_to receive(:perform_async) + expect(BuildTraceChunkFlushToDbWorker).not_to receive(:perform_async) subject end -- cgit v1.2.1 From 9ad20891435f1f51ae6e8e951b84131480d1ab97 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 2 May 2018 22:14:55 +0900 Subject: Fix Redis scan_each wildcard expression in spec --- spec/models/ci/build_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index dbeee74857c..991f501f633 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -349,7 +349,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do shared_examples_for 'deletes all build_trace_chunk and data in redis' do it do Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.size).to eq(3) + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(3) end expect(described_class.count).to eq(3) @@ -359,7 +359,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(described_class.count).to eq(0) Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:?:chunks:?").to_a.size).to eq(0) + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) end end end -- cgit v1.2.1 From 2261188f48dff25c5bfbbca739c5f570849155f4 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Wed, 2 May 2018 16:42:12 +0200 Subject: Rename Runner#invalidate_build_cache -> Runner#pick_build --- spec/models/ci/runner_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index fa540f8d4fd..6ad37417623 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -792,7 +792,7 @@ describe Ci::Runner do end end - describe '#invalidate_build_cache!' do + describe '#pick_build!' do context 'runner can pick the build' do it 'calls #tick_runner_queue' do ci_build = build :ci_build @@ -801,7 +801,7 @@ describe Ci::Runner do expect(runner).to receive(:tick_runner_queue) - runner.invalidate_build_cache!(ci_build) + runner.pick_build!(ci_build) end end @@ -813,7 +813,7 @@ describe Ci::Runner do expect(runner).not_to receive(:tick_runner_queue) - runner.invalidate_build_cache!(ci_build) + runner.pick_build!(ci_build) end end end -- cgit v1.2.1 From 1f7f29b7321c9cba5526ab991246f3178330b9cd Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 3 May 2018 08:47:41 +0200 Subject: Style changes to spec/models/ci/runner_spec.rb --- spec/models/ci/runner_spec.rb | 70 +++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 35 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 6ad37417623..744972deb4d 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -107,16 +107,17 @@ describe Ci::Runner do end describe '.shared' do + let(:group) { create(:group) } + let(:project) { create(:project) } + it 'returns the shared group runner' do - group = create :group - runner = create :ci_runner, :shared, groups: [group] + runner = create(:ci_runner, :shared, groups: [group]) expect(described_class.shared).to eq [runner] end it 'returns the shared project runner' do - project = create :project - runner = create :ci_runner, :shared, projects: [project] + runner = create(:ci_runner, :shared, projects: [project]) expect(described_class.shared).to eq [runner] end @@ -125,12 +126,12 @@ describe Ci::Runner do describe '.belonging_to_project' do it 'returns the specific project runner' do # own - specific_project = create :project - specific_runner = create :ci_runner, :specific, projects: [specific_project] + specific_project = create(:project) + specific_runner = create(:ci_runner, :specific, projects: [specific_project]) # other - other_project = create :project - create :ci_runner, :specific, projects: [other_project] + other_project = create(:project) + create(:ci_runner, :specific, projects: [other_project]) expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] end @@ -139,55 +140,54 @@ describe Ci::Runner do describe '.belonging_to_any_project' do it 'returns the specific project runner' do # project - project_project = create :project - project_runner = create :ci_runner, :specific, projects: [project_project] + project_project = create(:project) + project_runner = create(:ci_runner, :specific, projects: [project_project]) # group - group = create :group - create :project, group: group - create :ci_runner, :specific, groups: [group] + group = create(:group) + create(:project, group: group) + create(:ci_runner, :specific, groups: [group]) expect(described_class.belonging_to_any_project).to eq [project_runner] end end describe '.belonging_to_parent_group_of_project' do - it 'returns the specific group runner' do - # own - specific_group = create :group - specific_project = create :project, group: specific_group - specific_runner = create :ci_runner, :specific, groups: [specific_group] - - # other - other_group = create :group - create :project, group: other_group - create :ci_runner, :specific, groups: [other_group] + let(:project) { create(:project, group: group) } + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :specific, groups: [group]) } + let!(:unrelated_group) { create(:group) } + let!(:unrelated_project) { create(:project, group: unrelated_group) } + let!(:unrelated_runner) { create(:ci_runner, :specific, groups: [unrelated_group]) } - expect(described_class.belonging_to_parent_group_of_project(specific_project.id)).to eq [specific_runner] + it 'returns the specific group runner' do + expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) end - it 'returns the group runner from a parent group', :nested_groups do - parent_group = create :group - group = create :group, parent: parent_group - project = create :project, group: group - runner = create :ci_runner, :specific, groups: [parent_group] + context 'with a parent group with a runner', :nested_groups do + let(:runner) { create(:ci_runner, :specific, groups: [parent_group]) } + let(:project) { create(:project, group: group) } + let(:group) { create(:group, parent: parent_group) } + let(:parent_group) { create(:group) } - expect(described_class.belonging_to_parent_group_of_project(project.id)).to eq [runner] + it 'returns the group runner from the parent group' do + expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) + end end end describe '.owned_or_shared' do it 'returns a globally shared, a project specific and a group specific runner' do # group specific - group = create :group - project = create :project, group: group - group_runner = create :ci_runner, :specific, groups: [group] + group = create(:group) + project = create(:project, group: group) + group_runner = create(:ci_runner, :specific, groups: [group]) # project specific - project_runner = create :ci_runner, :specific, projects: [project] + project_runner = create(:ci_runner, :specific, projects: [project]) # globally shared - shared_runner = create :ci_runner, :shared + shared_runner = create(:ci_runner, :shared) expect(described_class.owned_or_shared(project.id)).to contain_exactly( group_runner, project_runner, shared_runner -- cgit v1.2.1 From a6c9db61779c71fd0a7f0f317fba13f8931ab954 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 3 May 2018 09:38:19 +0200 Subject: More style improvements to spec/models/ci/runner_spec.rb --- spec/models/ci/runner_spec.rb | 81 +++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 41 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 744972deb4d..21d7d616a3c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -309,7 +309,9 @@ describe Ci::Runner do describe '#can_pick?' do let(:pipeline) { create(:ci_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, tag_list: tag_list, run_untagged: run_untagged) } + let(:tag_list) { [] } + let(:run_untagged) { true } subject { runner.can_pick?(build) } @@ -319,7 +321,7 @@ describe Ci::Runner do context 'a different runner' do it 'cannot handle builds' do - other_runner = create :ci_runner + other_runner = create(:ci_runner) expect(other_runner.can_pick?(build)).to be_falsey end end @@ -337,9 +339,7 @@ describe Ci::Runner do end context 'when runner has tags' do - before do - runner.tag_list = %w(bb cc) - end + let(:tag_list) { %w(bb cc) } shared_examples 'tagged build picker' do it 'can handle build with matching tags' do @@ -364,9 +364,7 @@ describe Ci::Runner do end context 'when runner cannot pick untagged jobs' do - before do - runner.update_attributes!(run_untagged: false) - end + let(:run_untagged) { false } it 'cannot handle builds without tags' do expect(runner.can_pick?(build)).to be_falsey @@ -377,8 +375,9 @@ describe Ci::Runner do end context 'when runner is shared' do + let(:runner) { create(:ci_runner, :shared) } + before do - runner.update_attributes!(is_shared: true) build.project.runners = [] end @@ -387,9 +386,7 @@ describe Ci::Runner do end context 'when runner is locked' do - before do - runner.update_attributes!(locked: true) - end + let(:runner) { create(:ci_runner, :shared, locked: true) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -748,55 +745,57 @@ describe Ci::Runner do end end - describe 'assigned_to_group?' do - it 'returns false when the runner is a project runner' do - project = create :project - runner = create :ci_runner, description: 'Project runner', projects: [project] + describe '#assigned_to_group?' do + subject { runner.assigned_to_group? } + + context 'when project runner' do + let(:runner) { create(:ci_runner, description: 'Project runner', projects: [project]) } + let(:project) { create(:project) } - expect(runner.assigned_to_group?).to be false + it { is_expected.to be_falsey } end - it 'returns false when the runner is a shared runner' do - runner = create :ci_runner, :shared, description: 'Shared runner' + context 'when shared runner' do + let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } - expect(runner.assigned_to_group?).to be false + it { is_expected.to be_falsey } end - it 'returns true when the runner is assigned to a group' do - group = create :group - runner = create :ci_runner, description: 'Group runner', groups: [group] + context 'when group runner' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } - expect(runner.assigned_to_group?).to be true + it { is_expected.to be_truthy } end end - describe 'assigned_to_project?' do - it 'returns false when the runner is a group prunner' do - group = create :group - runner = create :ci_runner, description: 'Group runner', groups: [group] + describe '#assigned_to_project?' do + subject { runner.assigned_to_project? } - expect(runner.assigned_to_project?).to be false + context 'when group runner' do + let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:group) { create(:group) } + it { is_expected.to be_falsey } end - it 'returns false when the runner is a shared runner' do - runner = create :ci_runner, :shared, description: 'Shared runner' - - expect(runner.assigned_to_project?).to be false + context 'when shared runner' do + let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + it { is_expected.to be_falsey } end - it 'returns true when the runner is assigned to a project' do - project = create :project - runner = create :ci_runner, description: 'Group runner', projects: [project] + context 'when project runner' do + let(:runner) { create(:ci_runner, description: 'Group runner', projects: [project]) } + let(:project) { create(:project) } - expect(runner.assigned_to_project?).to be true + it { is_expected.to be_truthy } end end describe '#pick_build!' do context 'runner can pick the build' do it 'calls #tick_runner_queue' do - ci_build = build :ci_build - runner = build :ci_runner + ci_build = build(:ci_build) + runner = build(:ci_runner) allow(runner).to receive(:can_pick?).with(ci_build).and_return(true) expect(runner).to receive(:tick_runner_queue) @@ -807,8 +806,8 @@ describe Ci::Runner do context 'runner cannot pick the build' do it 'does not call #tick_runner_queue' do - ci_build = build :ci_build - runner = build :ci_runner + ci_build = build(:ci_build) + runner = build(:ci_runner) allow(runner).to receive(:can_pick?).with(ci_build).and_return(false) expect(runner).not_to receive(:tick_runner_queue) -- cgit v1.2.1 From 67f25c6259553e30e921de3d4d72d3e97d06d327 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 3 May 2018 09:44:45 +0200 Subject: Style improvements to spec/models/project_spec.rb --- spec/models/project_spec.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ab0694e6890..08e42b61910 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1138,7 +1138,7 @@ describe Project do end end - describe '#any_runners' do + describe '#any_runners?' do context 'shared runners' do let(:project) { create :project, shared_runners_enabled: shared_runners_enabled } let(:specific_runner) { create :ci_runner } @@ -1153,21 +1153,25 @@ describe Project do it 'has a specific runner' do project.runners << specific_runner + expect(project.any_runners?).to be_truthy end it 'has a shared runner, but they are prohibited to use' do shared_runner + expect(project.any_runners?).to be_falsey end it 'checks the presence of specific runner' do project.runners << specific_runner + expect(project.any_runners? { |runner| runner == specific_runner }).to be_truthy end it 'returns false if match cannot be found' do project.runners << specific_runner + expect(project.any_runners? { false }).to be_falsey end end @@ -1177,16 +1181,19 @@ describe Project do it 'has a shared runner' do shared_runner + expect(project.any_runners?).to be_truthy end it 'checks the presence of shared runner' do shared_runner + expect(project.any_runners? { |runner| runner == shared_runner }).to be_truthy end it 'returns false if match cannot be found' do shared_runner + expect(project.any_runners? { false }).to be_falsey end end @@ -1206,6 +1213,7 @@ describe Project do it 'has a group runner, but they are prohibited to use' do group_runner + expect(project.any_runners?).to be_falsey end end @@ -1215,16 +1223,19 @@ describe Project do it 'has a group runner' do group_runner + expect(project.any_runners?).to be_truthy end it 'checks the presence of group runner' do group_runner + expect(project.any_runners? { |runner| runner == group_runner }).to be_truthy end it 'returns false if match cannot be found' do group_runner + expect(project.any_runners? { false }).to be_falsey end end @@ -3592,7 +3603,7 @@ describe Project do describe '#toggle_ci_cd_settings!' do it 'toggles the value on #settings' do - project = create :project, group_runners_enabled: false + project = create(:project, group_runners_enabled: false) expect(project.group_runners_enabled).to be false -- cgit v1.2.1 From 5a7fbb8d89404f27a4467db7f2c619cc56f91142 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Thu, 3 May 2018 08:52:51 +0000 Subject: [Rails5] Fix params passed to MergeWorker --- spec/models/merge_request_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index becb146422e..5a9aa7c7d1b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1213,7 +1213,7 @@ describe MergeRequest do it 'enqueues MergeWorker job and updates merge_jid' do merge_request = create(:merge_request) user_id = double(:user_id) - params = double(:params) + params = {} merge_jid = 'hash-123' expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do -- cgit v1.2.1 From 49cbe576229b7e4003575e04006cc4132c3c0060 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 3 May 2018 10:59:38 +0200 Subject: Remove Runner#belonging_to_any_project since this is no longer needed --- spec/models/ci/runner_spec.rb | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 21d7d616a3c..cc4d4e5e4ae 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -137,21 +137,6 @@ describe Ci::Runner do end end - describe '.belonging_to_any_project' do - it 'returns the specific project runner' do - # project - project_project = create(:project) - project_runner = create(:ci_runner, :specific, projects: [project_project]) - - # group - group = create(:group) - create(:project, group: group) - create(:ci_runner, :specific, groups: [group]) - - expect(described_class.belonging_to_any_project).to eq [project_runner] - end - end - describe '.belonging_to_parent_group_of_project' do let(:project) { create(:project, group: group) } let(:group) { create(:group) } -- cgit v1.2.1 From e9e4bd23ee548253f290832646e3580b852cf1a1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 3 May 2018 20:53:48 +0900 Subject: Add spec for FastDestroyAll --- spec/models/concerns/fast_destroy_all_spec.rb | 69 +++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 spec/models/concerns/fast_destroy_all_spec.rb (limited to 'spec/models') diff --git a/spec/models/concerns/fast_destroy_all_spec.rb b/spec/models/concerns/fast_destroy_all_spec.rb new file mode 100644 index 00000000000..66fb5b23d31 --- /dev/null +++ b/spec/models/concerns/fast_destroy_all_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe FastDestroyAll, :clean_gitlab_redis_shared_state do + let(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + + before do + stub_feature_flags(ci_enable_live_trace: true) + end + + describe 'Forbid #destroy and #destroy_all' do + let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } + let(:trace_chunks) { build.trace_chunks } + + it 'does not delete database rows and associted external data' do + expect(trace_chunks.first).to be_a(described_class) + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) + expect(trace_chunks.count).to eq(1) + + expect { trace_chunks.first.destroy }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`') + expect { trace_chunks.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`') + + expect(trace_chunks.count).to eq(1) + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) + end + end + end + + describe '.fast_destroy_all' do + let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } + let(:trace_chunks) { build.trace_chunks } + + it 'deletes database rows and associted external data' do + expect(trace_chunks.first).to be_a(described_class) + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) + expect(trace_chunks.count).to eq(1) + + expect { build.trace_chunks.fast_destroy_all }.not_to raise_error + + expect(trace_chunks.count).to eq(0) + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) + end + end + end + + describe '.use_fast_destroy' do + let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } + let(:trace_chunks) { build.trace_chunks } + + it 'performs cascading delete with fast_destroy_all' do + expect(trace_chunks.first).to be_a(described_class) + expect(project).to be_a(described_class::Helpers) + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) + expect(trace_chunks.count).to eq(1) + + project.destroy + + expect(trace_chunks.count).to eq(0) + expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) + end + end + end +end -- cgit v1.2.1 From 812dd06d512ab7774b375ce45aa9235aafc99911 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 4 May 2018 17:02:08 +0900 Subject: Introduce Redis helpers. Rename BuildTraceChunkFlushToDbWorker to Ci::BuildTraceChunkFlushWorker. --- spec/models/ci/build_trace_chunk_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 991f501f633..118b72da11c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -75,7 +75,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do let(:value) { 'a' * described_class::CHUNK_SIZE } it 'schedules stashing data' do - expect(BuildTraceChunkFlushToDbWorker).to receive(:perform_async).once + expect(Ci::BuildTraceChunkFlushWorker).to receive(:perform_async).once subject end @@ -112,7 +112,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when fullfilled chunk size' do it 'does not schedule stashing data' do - expect(BuildTraceChunkFlushToDbWorker).not_to receive(:perform_async) + expect(Ci::BuildTraceChunkFlushWorker).not_to receive(:perform_async) subject end -- cgit v1.2.1 From 28284c14973a59d5a7f0f8d2862da7f61b101640 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 4 May 2018 17:42:37 +0900 Subject: Add validation and skip logic at #truncate --- spec/models/ci/build_trace_chunk_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 118b72da11c..46d09dff52c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -141,11 +141,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do context 'when offset is bigger than data size' do let(:offset) { data.bytesize + 1 } - it do - expect_any_instance_of(described_class).not_to receive(:append) { } - - subject - end + it { expect { subject }.to raise_error('Offset is out of range') } end context 'when offset is 10' do -- cgit v1.2.1 From 3d6d0a09b65f032bbe1bd5ad4736dd764195bbe1 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Tue, 24 Apr 2018 18:28:04 +0200 Subject: Store application wide terms This allows admins to define terms in the application settings. Every time the terms are adjusted, a new version is stored and becomes the 'active' version. This allows tracking which specific version was accepted by a user. --- spec/models/application_setting/term_spec.rb | 15 +++++++++++++++ spec/models/application_setting_spec.rb | 15 +++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 spec/models/application_setting/term_spec.rb (limited to 'spec/models') diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb new file mode 100644 index 00000000000..1eddf3c56ff --- /dev/null +++ b/spec/models/application_setting/term_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe ApplicationSetting::Term do + describe 'validations' do + it { is_expected.to validate_presence_of(:terms) } + end + + describe '.latest' do + it 'finds the latest terms' do + terms = create(:term) + + expect(described_class.latest).to eq(terms) + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index ae2d34750a7..10d6109cae7 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -301,6 +301,21 @@ describe ApplicationSetting do expect(subject).to be_invalid end end + + describe 'enforcing terms' do + it 'requires the terms to present when enforcing users to accept' do + subject.enforce_terms = true + + expect(subject).to be_invalid + end + + it 'is valid when terms are created' do + create(:term) + subject.enforce_terms = true + + expect(subject).to be_valid + end + end end describe '.current' do -- cgit v1.2.1 From 82eeb72c8c03727540b902d40e7e657d0a5ecb4c Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Wed, 25 Apr 2018 10:55:53 +0200 Subject: Add model to track users accepting agreements --- spec/models/term_agreement_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 spec/models/term_agreement_spec.rb (limited to 'spec/models') diff --git a/spec/models/term_agreement_spec.rb b/spec/models/term_agreement_spec.rb new file mode 100644 index 00000000000..a59bf119692 --- /dev/null +++ b/spec/models/term_agreement_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' + +describe TermAgreement do + describe 'validations' do + it { is_expected.to validate_presence_of(:term) } + it { is_expected.to validate_presence_of(:user) } + end +end -- cgit v1.2.1 From 5627f28cb886342b2555b38747458a6bee196ebe Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 4 May 2018 21:19:57 +0900 Subject: Added shared exmaple for fast_destroy_all concern --- spec/models/ci/build_trace_chunk_spec.rb | 15 ++++++ spec/models/concerns/fast_destroy_all_spec.rb | 69 --------------------------- 2 files changed, 15 insertions(+), 69 deletions(-) delete mode 100644 spec/models/concerns/fast_destroy_all_spec.rb (limited to 'spec/models') diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 46d09dff52c..cbcf1e55979 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -14,6 +14,21 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do stub_feature_flags(ci_enable_live_trace: true) end + context 'FastDestroyAll' do + let(:parent) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: parent) } + let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: parent) } + let(:subjects) { build.trace_chunks } + + it_behaves_like 'fast destroyable' + + def external_data_counter + Gitlab::Redis::SharedState.with do |redis| + redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size + end + end + end + describe 'CHUNK_SIZE' do it 'Chunk size can not be changed without special care' do expect(described_class::CHUNK_SIZE).to eq(128.kilobytes) diff --git a/spec/models/concerns/fast_destroy_all_spec.rb b/spec/models/concerns/fast_destroy_all_spec.rb deleted file mode 100644 index 66fb5b23d31..00000000000 --- a/spec/models/concerns/fast_destroy_all_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -require 'spec_helper' - -describe FastDestroyAll, :clean_gitlab_redis_shared_state do - let(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project) } - - before do - stub_feature_flags(ci_enable_live_trace: true) - end - - describe 'Forbid #destroy and #destroy_all' do - let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } - let(:trace_chunks) { build.trace_chunks } - - it 'does not delete database rows and associted external data' do - expect(trace_chunks.first).to be_a(described_class) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) - expect(trace_chunks.count).to eq(1) - - expect { trace_chunks.first.destroy }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`') - expect { trace_chunks.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`') - - expect(trace_chunks.count).to eq(1) - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) - end - end - end - - describe '.fast_destroy_all' do - let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } - let(:trace_chunks) { build.trace_chunks } - - it 'deletes database rows and associted external data' do - expect(trace_chunks.first).to be_a(described_class) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) - expect(trace_chunks.count).to eq(1) - - expect { build.trace_chunks.fast_destroy_all }.not_to raise_error - - expect(trace_chunks.count).to eq(0) - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) - end - end - end - - describe '.use_fast_destroy' do - let(:build) { create(:ci_build, :running, :trace_live, pipeline: pipeline, project: project) } - let(:trace_chunks) { build.trace_chunks } - - it 'performs cascading delete with fast_destroy_all' do - expect(trace_chunks.first).to be_a(described_class) - expect(project).to be_a(described_class::Helpers) - - Gitlab::Redis::SharedState.with do |redis| - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(1) - expect(trace_chunks.count).to eq(1) - - project.destroy - - expect(trace_chunks.count).to eq(0) - expect(redis.scan_each(match: "gitlab:ci:trace:*:chunks:*").to_a.size).to eq(0) - end - end - end -end -- cgit v1.2.1 From bc7ea2d4386f5624676e7a2ffb196de19d52910b Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 4 May 2018 12:42:58 +0000 Subject: Add ci_cd_settings delete_all dependency on project --- spec/models/project_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 08e42b61910..ce9783db062 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -101,6 +101,14 @@ describe Project do end end + context 'updating cd_cd_settings' do + it 'does not raise an error' do + project = create(:project) + + expect{ project.update(ci_cd_settings: nil) }.not_to raise_exception + end + end + describe '#members & #requesters' do let(:project) { create(:project, :public, :access_requestable) } let(:requester) { create(:user) } -- cgit v1.2.1 From 4f04aeec80bbfcb025e321693e6ca99b01244bb4 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 4 May 2018 17:22:06 +0200 Subject: fix missing space --- spec/models/project_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ce9783db062..c88510020c8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -105,7 +105,7 @@ describe Project do it 'does not raise an error' do project = create(:project) - expect{ project.update(ci_cd_settings: nil) }.not_to raise_exception + expect { project.update(ci_cd_settings: nil) }.not_to raise_exception end end -- cgit v1.2.1 From bddbcaefc2389e4c61763472cecbea150f10cd75 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Wed, 2 May 2018 14:35:04 +0100 Subject: Backports every CE related change from ee-44542 to CE --- spec/models/project_import_state_spec.rb | 13 +++++++++++++ spec/models/project_spec.rb | 6 ++++-- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 spec/models/project_import_state_spec.rb (limited to 'spec/models') diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb new file mode 100644 index 00000000000..f7033b28c76 --- /dev/null +++ b/spec/models/project_import_state_spec.rb @@ -0,0 +1,13 @@ +require 'rails_helper' + +describe ProjectImportState, type: :model do + subject { create(:import_state) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 08e42b61910..e41212b1e03 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1695,7 +1695,8 @@ describe Project do it 'resets project import_error' do error_message = 'Some error' - mirror = create(:project_empty_repo, :import_started, import_error: error_message) + mirror = create(:project_empty_repo, :import_started) + mirror.import_state.update_attributes(last_error: error_message) expect { mirror.import_finish }.to change { mirror.import_error }.from(error_message).to(nil) end @@ -3339,7 +3340,8 @@ describe Project do context 'with an import JID' do it 'unsets the import JID' do - project = create(:project, import_jid: '123') + project = create(:project) + create(:import_state, project: project, jid: '123') expect(Gitlab::SidekiqStatus) .to receive(:unset) -- cgit v1.2.1 From 5c2078838bb9de710f9025513c4b6ec664bba313 Mon Sep 17 00:00:00 2001 From: Mario de la Ossa Date: Thu, 26 Apr 2018 13:53:13 -0600 Subject: Backport of 4084-epics-username-autocomplete --- spec/models/group_spec.rb | 89 +++++++++++++++++++++++++++++++++++++++++++ spec/models/namespace_spec.rb | 15 ++++++++ 2 files changed, 104 insertions(+) (limited to 'spec/models') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d620943693c..0907d28d33b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -424,6 +424,95 @@ describe Group do end end + describe '#direct_and_indirect_members', :nested_groups do + let!(:group) { create(:group, :nested) } + let!(:sub_group) { create(:group, parent: group) } + let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) } + let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + let!(:other_developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + + it 'returns parents members' do + expect(group.direct_and_indirect_members).to include(developer) + expect(group.direct_and_indirect_members).to include(master) + end + + it 'returns descendant members' do + expect(group.direct_and_indirect_members).to include(other_developer) + end + end + + describe '#users_with_descendants', :nested_groups do + let(:user_a) { create(:user) } + let(:user_b) { create(:user) } + + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + + it 'returns member users on every nest level without duplication' do + group.add_developer(user_a) + nested_group.add_developer(user_b) + deep_nested_group.add_developer(user_a) + + expect(group.users_with_descendants).to contain_exactly(user_a, user_b) + expect(nested_group.users_with_descendants).to contain_exactly(user_a, user_b) + expect(deep_nested_group.users_with_descendants).to contain_exactly(user_a) + end + end + + describe '#direct_and_indirect_users', :nested_groups do + let(:user_a) { create(:user) } + let(:user_b) { create(:user) } + let(:user_c) { create(:user) } + let(:user_d) { create(:user) } + + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + let(:project) { create(:project, namespace: group) } + + before do + group.add_developer(user_a) + group.add_developer(user_c) + nested_group.add_developer(user_b) + deep_nested_group.add_developer(user_a) + project.add_developer(user_d) + end + + it 'returns member users on every nest level without duplication' do + expect(group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c, user_d) + expect(nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c) + expect(deep_nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c) + end + + it 'does not return members of projects belonging to ancestor groups' do + expect(nested_group.direct_and_indirect_users).not_to include(user_d) + end + end + + describe '#project_users_with_descendants', :nested_groups do + let(:user_a) { create(:user) } + let(:user_b) { create(:user) } + let(:user_c) { create(:user) } + + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:deep_nested_group) { create(:group, parent: nested_group) } + let(:project_a) { create(:project, namespace: group) } + let(:project_b) { create(:project, namespace: nested_group) } + let(:project_c) { create(:project, namespace: deep_nested_group) } + + it 'returns members of all projects in group and subgroups' do + project_a.add_developer(user_a) + project_b.add_developer(user_b) + project_c.add_developer(user_c) + + expect(group.project_users_with_descendants).to contain_exactly(user_a, user_b, user_c) + expect(nested_group.project_users_with_descendants).to contain_exactly(user_b, user_c) + expect(deep_nested_group.project_users_with_descendants).to contain_exactly(user_c) + end + end + describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do master = create(:user) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 506057dce87..6f702d8d95e 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -399,6 +399,21 @@ describe Namespace do end end + describe '#self_and_hierarchy', :nested_groups do + let!(:group) { create(:group, path: 'git_lab') } + let!(:nested_group) { create(:group, parent: group) } + let!(:deep_nested_group) { create(:group, parent: nested_group) } + let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) } + let!(:another_group) { create(:group, path: 'gitllab') } + let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) } + + it 'returns the correct tree' do + expect(group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) + expect(nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) + expect(very_deep_nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group) + end + end + describe '#ancestors', :nested_groups do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } -- cgit v1.2.1 From 9a13059332a0c81b3a953f57bb9e40346eba951d Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Thu, 3 May 2018 13:55:14 +0100 Subject: Backports every CE related change from ee-5484 to CE --- spec/models/project_spec.rb | 79 +++++++++++ spec/models/remote_mirror_spec.rb | 275 ++++++++++++++++++++++++++++++++++++++ spec/models/repository_spec.rb | 32 +++++ 3 files changed, 386 insertions(+) create mode 100644 spec/models/remote_mirror_spec.rb (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f3cf21cf279..8015c0926b6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1852,6 +1852,85 @@ describe Project do it { expect(project.gitea_import?).to be true } end + describe '#has_remote_mirror?' do + let(:project) { create(:project, :remote_mirror, :import_started) } + subject { project.has_remote_mirror? } + + before do + allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) + end + + it 'returns true when a remote mirror is enabled' do + is_expected.to be_truthy + end + + it 'returns false when remote mirror is disabled' do + project.remote_mirrors.first.update_attributes(enabled: false) + + is_expected.to be_falsy + end + end + + describe '#update_remote_mirrors' do + let(:project) { create(:project, :remote_mirror, :import_started) } + delegate :update_remote_mirrors, to: :project + + before do + allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) + end + + it 'syncs enabled remote mirror' do + expect_any_instance_of(RemoteMirror).to receive(:sync) + + update_remote_mirrors + end + + # TODO: study if remote_mirror_available_overridden is still a necessary attribute considering that + # it is no longer under any license + it 'does nothing when remote mirror is disabled globally and not overridden' do + stub_application_setting(mirror_available: false) + project.remote_mirror_available_overridden = false + + expect_any_instance_of(RemoteMirror).not_to receive(:sync) + + update_remote_mirrors + end + + it 'does not sync disabled remote mirrors' do + project.remote_mirrors.first.update_attributes(enabled: false) + + expect_any_instance_of(RemoteMirror).not_to receive(:sync) + + update_remote_mirrors + end + end + + describe '#remote_mirror_available?' do + let(:project) { create(:project) } + + context 'when remote mirror global setting is enabled' do + it 'returns true' do + expect(project.remote_mirror_available?).to be(true) + end + end + + context 'when remote mirror global setting is disabled' do + before do + stub_application_setting(mirror_available: false) + end + + it 'returns true when overridden' do + project.remote_mirror_available_overridden = true + + expect(project.remote_mirror_available?).to be(true) + end + + it 'returns false when not overridden' do + expect(project.remote_mirror_available?).to be(false) + end + end + end + describe '#ancestors_upto', :nested_groups do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb new file mode 100644 index 00000000000..d6181ca0783 --- /dev/null +++ b/spec/models/remote_mirror_spec.rb @@ -0,0 +1,275 @@ +require 'rails_helper' + +describe RemoteMirror do + describe 'URL validation' do + context 'with a valid URL' do + it 'should be valid' do + remote_mirror = build(:remote_mirror) + expect(remote_mirror).to be_valid + end + end + + context 'with an invalid URL' do + it 'should not be valid' do + remote_mirror = build(:remote_mirror, url: 'ftp://invalid.invalid') + expect(remote_mirror).not_to be_valid + expect(remote_mirror.errors[:url].size).to eq(2) + end + end + end + + describe 'encrypting credentials' do + context 'when setting URL for a first time' do + it 'stores the URL without credentials' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + expect(mirror.read_attribute(:url)).to eq('http://test.com') + end + + it 'stores the credentials on a separate field' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' }) + end + + it 'handles credentials with large content' do + mirror = create_mirror(url: 'http://bxnhm8dote33ct932r3xavslj81wxmr7o8yux8do10oozckkif:9ne7fuvjn40qjt35dgt8v86q9m9g9essryxj76sumg2ccl2fg26c0krtz2gzfpyq4hf22h328uhq6npuiq6h53tpagtsj7vsrz75@test.com') + + expect(mirror.credentials).to eq({ + user: 'bxnhm8dote33ct932r3xavslj81wxmr7o8yux8do10oozckkif', + password: '9ne7fuvjn40qjt35dgt8v86q9m9g9essryxj76sumg2ccl2fg26c0krtz2gzfpyq4hf22h328uhq6npuiq6h53tpagtsj7vsrz75' + }) + end + end + + context 'when updating the URL' do + it 'allows a new URL without credentials' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + mirror.update_attribute(:url, 'http://test.com') + + expect(mirror.url).to eq('http://test.com') + expect(mirror.credentials).to eq({ user: nil, password: nil }) + end + + it 'allows a new URL with credentials' do + mirror = create_mirror(url: 'http://test.com') + + mirror.update_attribute(:url, 'http://foo:bar@test.com') + + expect(mirror.url).to eq('http://foo:bar@test.com') + expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' }) + end + + it 'updates the remote config if credentials changed' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + repo = mirror.project.repository + + mirror.update_attribute(:url, 'http://foo:baz@test.com') + + config = repo.raw_repository.rugged.config + expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com') + end + + it 'removes previous remote' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original + + mirror.update_attributes(url: 'http://test.com') + end + end + end + + describe '#remote_name' do + context 'when remote name is persisted in the database' do + it 'returns remote name with random value' do + allow(SecureRandom).to receive(:hex).and_return('secret') + + remote_mirror = create(:remote_mirror) + + expect(remote_mirror.remote_name).to eq("remote_mirror_secret") + end + end + + context 'when remote name is not persisted in the database' do + it 'returns remote name with remote mirror id' do + remote_mirror = create(:remote_mirror) + remote_mirror.remote_name = nil + + expect(remote_mirror.remote_name).to eq("remote_mirror_#{remote_mirror.id}") + end + end + + context 'when remote is not persisted in the database' do + it 'returns nil' do + remote_mirror = build(:remote_mirror, remote_name: nil) + + expect(remote_mirror.remote_name).to be_nil + end + end + end + + describe '#safe_url' do + context 'when URL contains credentials' do + it 'masks the credentials' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + expect(mirror.safe_url).to eq('http://*****:*****@test.com') + end + end + + context 'when URL does not contain credentials' do + it 'shows the full URL' do + mirror = create_mirror(url: 'http://test.com') + + expect(mirror.safe_url).to eq('http://test.com') + end + end + end + + context 'when remote mirror gets destroyed' do + it 'removes remote' do + mirror = create_mirror(url: 'http://foo:bar@test.com') + + expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original + + mirror.destroy! + end + end + + context 'stuck mirrors' do + it 'includes mirrors stuck in started with no last_update_at set' do + mirror = create_mirror(url: 'http://cantbeblank', + update_status: 'started', + last_update_at: nil, + updated_at: 25.hours.ago) + + expect(described_class.stuck.last).to eq(mirror) + end + end + + context '#sync' do + let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with remote mirroring disabled' do + it 'returns nil' do + remote_mirror.update_attributes(enabled: false) + + expect(remote_mirror.sync).to be_nil + end + end + + context 'as a Geo secondary' do + it 'returns nil' do + allow(Gitlab::Geo).to receive(:secondary?).and_return(true) + + expect(remote_mirror.sync).to be_nil + end + end + + context 'with remote mirroring enabled' do + context 'with only protected branches enabled' do + context 'when it did not update in the last minute' do + it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now) + + remote_mirror.sync + end + end + + context 'when it did update in the last minute' do + it 'schedules a RepositoryUpdateRemoteMirrorWorker to run in the next minute' do + remote_mirror.last_update_started_at = Time.now - 30.seconds + + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::PROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.now) + + remote_mirror.sync + end + end + end + + context 'with only protected branches disabled' do + before do + remote_mirror.only_protected_branches = false + end + + context 'when it did not update in the last 5 minutes' do + it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now) + + remote_mirror.sync + end + end + + context 'when it did update within the last 5 minutes' do + it 'schedules a RepositoryUpdateRemoteMirrorWorker to run in the next 5 minutes' do + remote_mirror.last_update_started_at = Time.now - 30.seconds + + expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_in).with(RemoteMirror::UNPROTECTED_BACKOFF_DELAY, remote_mirror.id, Time.now) + + remote_mirror.sync + end + end + end + end + end + + context '#updated_since?' do + let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + let(:timestamp) { Time.now - 5.minutes } + + around do |example| + Timecop.freeze { example.run } + end + + before do + remote_mirror.update_attributes(last_update_started_at: Time.now) + end + + context 'when remote mirror does not have status failed' do + it 'returns true when last update started after the timestamp' do + expect(remote_mirror.updated_since?(timestamp)).to be true + end + + it 'returns false when last update started before the timestamp' do + expect(remote_mirror.updated_since?(Time.now + 5.minutes)).to be false + end + end + + context 'when remote mirror has status failed' do + it 'returns false when last update started after the timestamp' do + remote_mirror.update_attributes(update_status: 'failed') + + expect(remote_mirror.updated_since?(timestamp)).to be false + end + end + end + + context 'no project' do + it 'includes mirror with a project in pending_delete' do + mirror = create_mirror(url: 'http://cantbeblank', + update_status: 'finished', + enabled: true, + last_update_at: nil, + updated_at: 25.hours.ago) + project = mirror.project + project.pending_delete = true + project.save + mirror.reload + + expect(mirror.sync).to be_nil + expect(mirror.valid?).to be_truthy + expect(mirror.update_status).to eq('finished') + end + end + + def create_mirror(params) + project = FactoryBot.create(:project, :repository) + project.remote_mirrors.create!(params) + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 630b9e0519f..44d9ffd258d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -758,6 +758,38 @@ describe Repository do end end + describe '#async_remove_remote' do + before do + masterrev = repository.find_branch('master').dereferenced_target + create_remote_branch('joe', 'remote_branch', masterrev) + end + + context 'when worker is scheduled successfully' do + before do + masterrev = repository.find_branch('master').dereferenced_target + create_remote_branch('remote_name', 'remote_branch', masterrev) + + allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return('1234') + end + + it 'returns job_id' do + expect(repository.async_remove_remote('joe')).to eq('1234') + end + end + + context 'when worker does not schedule successfully' do + before do + allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return(nil) + end + + it 'returns nil' do + expect(Rails.logger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") + + expect(repository.async_remove_remote('joe')).to be_nil + end + end + end + describe '#fetch_ref' do let(:broken_repository) { create(:project, :broken_storage).repository } -- cgit v1.2.1 From 961255b107370a1350f91d0835cf0e849d237f7d Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Thu, 3 May 2018 15:19:21 +0100 Subject: Adds remote mirror table migration --- spec/models/project_spec.rb | 2 +- spec/models/remote_mirror_spec.rb | 8 -------- spec/models/repository_spec.rb | 5 +++++ 3 files changed, 6 insertions(+), 9 deletions(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 8015c0926b6..41622fbbb6f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1852,7 +1852,7 @@ describe Project do it { expect(project.gitea_import?).to be true } end - describe '#has_remote_mirror?' do + describe '#has_remote_mirror?' do let(:project) { create(:project, :remote_mirror, :import_started) } subject { project.has_remote_mirror? } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index d6181ca0783..a80800c6c92 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -164,14 +164,6 @@ describe RemoteMirror do end end - context 'as a Geo secondary' do - it 'returns nil' do - allow(Gitlab::Geo).to receive(:secondary?).and_return(true) - - expect(remote_mirror.sync).to be_nil - end - end - context 'with remote mirroring enabled' do context 'with only protected branches enabled' do context 'when it did not update in the last minute' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 44d9ffd258d..4b736b02b7d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2370,6 +2370,11 @@ describe Repository do end end + def create_remote_branch(remote_name, branch_name, target) + rugged = repository.rugged + rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) + end + describe '#ancestor?' do let(:commit) { repository.commit } let(:ancestor) { commit.parents.first } -- cgit v1.2.1 From 92482e01b45c337c0b9d761efd0297fc97ffcac0 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 4 May 2018 11:45:16 +0200 Subject: Bump Gitaly to 0.98.0 Hooks were run for wikis, but given the internal API wasn't responding failures happended, as seen on: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18693 --- spec/models/blob_viewer/readme_spec.rb | 2 +- spec/models/project_services/microsoft_teams_service_spec.rb | 2 +- spec/models/project_wiki_spec.rb | 6 +++++- spec/models/wiki_page_spec.rb | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/blob_viewer/readme_spec.rb b/spec/models/blob_viewer/readme_spec.rb index b9946c0315a..8d11d58cfca 100644 --- a/spec/models/blob_viewer/readme_spec.rb +++ b/spec/models/blob_viewer/readme_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BlobViewer::Readme do include FakeBlobHelpers - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, :wiki_repo) } let(:blob) { fake_blob(path: 'README.md') } subject { described_class.new(blob) } diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index 733086e258f..8d9ee96227f 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -30,7 +30,7 @@ describe MicrosoftTeamsService do describe "#execute" do let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + set(:project) { create(:project, :repository, :wiki_repo) } before do allow(chat_service).to receive_messages( diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index cbe7d111fcd..d6c4031329d 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe ProjectWiki do - let(:project) { create(:project) } + let(:project) { create(:project, :wiki_repo) } let(:repository) { project.repository } let(:user) { project.owner } let(:gitlab_shell) { Gitlab::Shell.new } @@ -328,6 +328,8 @@ describe ProjectWiki do end describe '#create_repo!' do + let(:project) { create(:project) } + it 'creates a repository' do expect(raw_repository.exists?).to eq(false) expect(subject.repository).to receive(:after_create) @@ -339,6 +341,8 @@ describe ProjectWiki do end describe '#ensure_repository' do + let(:project) { create(:project) } + it 'creates the repository if it not exist' do expect(raw_repository.exists?).to eq(false) diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 90b7e7715a8..1c765ceac2f 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe WikiPage do - let(:project) { create(:project) } + let(:project) { create(:project, :wiki_repo) } let(:user) { project.owner } let(:wiki) { ProjectWiki.new(project, user) } -- cgit v1.2.1 From 698a0c479858b5d76f0ce18934a23c4ea5a1e332 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 1 May 2018 17:09:33 +0200 Subject: Add a CI_COMMIT_MESSAGE predefined variable --- spec/models/ci/build_spec.rb | 5 ++++- spec/models/ci/pipeline_spec.rb | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 3158e006720..dc810489011 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1518,7 +1518,10 @@ describe Ci::Build do { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, - { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true } + { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }, + { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true }, + { key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true }, + { key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true } ] end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index dd94515b0a4..ddd66a6be87 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -173,7 +173,7 @@ describe Ci::Pipeline, :mailer do it 'includes all predefined variables in a valid order' do keys = subject.map { |variable| variable[:key] } - expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE] + expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION] end end -- cgit v1.2.1 From 3063225ca289e87c3f590e1722451a1c674e2ac9 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Thu, 3 May 2018 17:28:00 -0300 Subject: Display merge commit SHA in merge widget after merge --- spec/models/merge_request_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5a9aa7c7d1b..04379e7d2c3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1069,6 +1069,22 @@ describe MergeRequest do end end + describe '#short_merge_commit_sha' do + let(:merge_request) { build_stubbed(:merge_request) } + + it 'returns short id when there is a merge_commit_sha' do + merge_request.merge_commit_sha = 'f7ce827c314c9340b075657fd61c789fb01cf74d' + + expect(merge_request.short_merge_commit_sha).to eq('f7ce827c') + end + + it 'returns nil when there is no merge_commit_sha' do + merge_request.merge_commit_sha = nil + + expect(merge_request.short_merge_commit_sha).to be_nil + end + end + describe '#can_be_reverted?' do context 'when there is no merge_commit for the MR' do before do -- cgit v1.2.1 From 18094a17458aab8c9d629b86946b61e0e7bab251 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 8 May 2018 00:14:00 +0000 Subject: =?UTF-8?q?Backport:=20Keep=20ShaAttribute=20from=20halting=20star?= =?UTF-8?q?tup=20when=20we=20can=E2=80=99t=20connect=20to=20a=20database?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/models/concerns/sha_attribute_spec.rb | 67 ++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 13 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/sha_attribute_spec.rb b/spec/models/concerns/sha_attribute_spec.rb index 21893e0cbaa..592feddf1dc 100644 --- a/spec/models/concerns/sha_attribute_spec.rb +++ b/spec/models/concerns/sha_attribute_spec.rb @@ -13,33 +13,74 @@ describe ShaAttribute do end describe '#sha_attribute' do - context 'when the table exists' do + context 'when in non-production' do before do - allow(model).to receive(:table_exists?).and_return(true) + allow(Rails.env).to receive(:production?).and_return(false) end - it 'defines a SHA attribute for a binary column' do - expect(model).to receive(:attribute) - .with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) + context 'when the table exists' do + before do + allow(model).to receive(:table_exists?).and_return(true) + end - model.sha_attribute(:sha1) + it 'defines a SHA attribute for a binary column' do + expect(model).to receive(:attribute) + .with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) + + model.sha_attribute(:sha1) + end + + it 'raises ArgumentError when the column type is not :binary' do + expect { model.sha_attribute(:name) }.to raise_error(ArgumentError) + end + end + + context 'when the table does not exist' do + it 'allows the attribute to be added' do + allow(model).to receive(:table_exists?).and_return(false) + + expect(model).not_to receive(:columns) + expect(model).to receive(:attribute) + + model.sha_attribute(:name) + end end - it 'raises ArgumentError when the column type is not :binary' do - expect { model.sha_attribute(:name) }.to raise_error(ArgumentError) + context 'when the column does not exist' do + it 'raises ArgumentError' do + allow(model).to receive(:table_exists?).and_return(true) + + expect(model).to receive(:columns) + expect(model).not_to receive(:attribute) + + expect { model.sha_attribute(:no_name) }.to raise_error(ArgumentError) + end + end + + context 'when other execeptions are raised' do + it 'logs and re-rasises the error' do + allow(model).to receive(:table_exists?).and_raise(ActiveRecord::NoDatabaseError.new('does not exist')) + + expect(model).not_to receive(:columns) + expect(model).not_to receive(:attribute) + expect(Gitlab::AppLogger).to receive(:error) + + expect { model.sha_attribute(:name) }.to raise_error(ActiveRecord::NoDatabaseError) + end end end - context 'when the table does not exist' do + context 'when in production' do before do - allow(model).to receive(:table_exists?).and_return(false) + allow(Rails.env).to receive(:production?).and_return(true) end - it 'does nothing' do + it 'defines a SHA attribute' do + expect(model).not_to receive(:table_exists?) expect(model).not_to receive(:columns) - expect(model).not_to receive(:attribute) + expect(model).to receive(:attribute).with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) - model.sha_attribute(:name) + model.sha_attribute(:sha1) end end end -- cgit v1.2.1