diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-06-01 15:43:43 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2018-06-01 15:43:43 +0300 |
commit | afb2d667cae42b77e090b4b240848c4d4ddcaf2d (patch) | |
tree | 6cb66332f5ec2889bbc4e3dbd870a6b65fbe8aa1 /spec/models | |
parent | 709e8b263863c5a92959700b67462c2ebe4f1831 (diff) | |
parent | 5c6c184f70719c464690455abc02e777b3ba4b7b (diff) |
Merge branch '46010-add-more-validations-for-runners-and-runner-type' into 'master'
Improve validations for Ci::Runner#runner_type
See merge request gitlab-org/gitlab-ce!18901
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/ci/build_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/ci/runner_spec.rb | 199 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 40 |
5 files changed, 126 insertions, 139 deletions
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 77179028ede..b5eac913639 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -148,10 +148,9 @@ describe Ci::Build do end context 'when there are runners' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project, projects: [build.project]) } before do - build.project.runners << runner runner.update_attributes(contacted_at: 1.second.ago) end @@ -1388,12 +1387,7 @@ describe Ci::Build do it { is_expected.to be_truthy } context "and there are specific runner" do - let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) } - - before do - build.project.runners << runner - runner.save - end + let!(:runner) { create(:ci_runner, :project, projects: [build.project], contacted_at: 1.second.ago) } it { is_expected.to be_falsey } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e4f4c62bd22..f5295bec65b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1589,7 +1589,7 @@ describe Ci::Pipeline, :mailer do context 'when pipeline is not stuck' do before do - create(:ci_runner, :shared, :online) + create(:ci_runner, :instance, :online) end it 'is not stuck' do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0fbc934f669..0f072aa1719 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -21,60 +21,58 @@ describe Ci::Runner do end end - context 'either_projects_or_group' do + context '#exactly_one_group' do let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } - it 'disallows assigning to a group if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - + it 'disallows assigning group if already assigned to a group' do 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'] + expect(runner.errors.full_messages).to include('Runner needs to be assigned to exactly one group') end + end - it 'disallows assigning to a group if already assigned to a project' do - project = create(:project) - runner = create(:ci_runner, projects: [project]) + context 'runner_type validations' do + set(:group) { create(:group) } + set(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let(:instance_runner) { create(:ci_runner, :instance) } - runner.groups << build(:group) + it 'disallows assigning group to project_type runner' do + 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'] + expect(project_runner).not_to be_valid + expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned') end - it 'disallows assigning to a project if already assigned to a group' do - runner = create(:ci_runner, groups: [group]) - - runner.projects << build(:project) + it 'disallows assigning group to instance_type runner' do + instance_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'] + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned') 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) + it 'disallows assigning project to group_type runner' do + group_runner.projects << build(:project) - expect(runner).to be_valid + expect(group_runner).not_to be_valid + expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned') 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) + it 'disallows assigning project to instance_type runner' do + instance_runner.projects << build(:project) - expect(runner).to be_valid + expect(instance_runner).not_to be_valid + expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned') 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) + it 'should fail to save a group assigned to a project runner even if the runner is already saved' do + group_runner - expect(runner).to be_valid + expect { create(:group, runners: [project_runner]) } + .to raise_error(ActiveRecord::RecordInvalid) end end end @@ -110,17 +108,12 @@ describe Ci::Runner do describe '.shared' do let(:group) { create(:group) } let(:project) { create(:project) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:shared_runner) { create(:ci_runner, :instance) } - it 'returns the shared group runner' do - runner = create(:ci_runner, :shared, groups: [group]) - - expect(described_class.shared).to eq [runner] - end - - it 'returns the shared project runner' do - runner = create(:ci_runner, :shared, projects: [project]) - - expect(described_class.shared).to eq [runner] + it 'returns only shared runners' do + expect(described_class.shared).to contain_exactly(shared_runner) end end @@ -128,11 +121,11 @@ describe Ci::Runner do it 'returns the specific project runner' do # own specific_project = create(:project) - specific_runner = create(:ci_runner, :specific, projects: [specific_project]) + specific_runner = create(:ci_runner, :project, projects: [specific_project]) # other other_project = create(:project) - create(:ci_runner, :specific, projects: [other_project]) + create(:ci_runner, :project, projects: [other_project]) expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner] end @@ -141,17 +134,17 @@ describe Ci::Runner do describe '.belonging_to_parent_group_of_project' do let(:project) { create(:project, group: group) } let(:group) { create(:group) } - let(:runner) { create(:ci_runner, :specific, groups: [group]) } + let(:runner) { create(:ci_runner, :group, 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]) } + let!(:unrelated_runner) { create(:ci_runner, :group, groups: [unrelated_group]) } it 'returns the specific group runner' do expect(described_class.belonging_to_parent_group_of_project(project.id)).to contain_exactly(runner) end context 'with a parent group with a runner', :nested_groups do - let(:runner) { create(:ci_runner, :specific, groups: [parent_group]) } + let(:runner) { create(:ci_runner, :group, groups: [parent_group]) } let(:project) { create(:project, group: group) } let(:group) { create(:group, parent: parent_group) } let(:parent_group) { create(:group) } @@ -167,13 +160,13 @@ describe Ci::Runner do # group specific group = create(:group) project = create(:project, group: group) - group_runner = create(:ci_runner, :specific, groups: [group]) + group_runner = create(:ci_runner, :group, groups: [group]) # project specific - project_runner = create(:ci_runner, :specific, projects: [project]) + project_runner = create(:ci_runner, :project, projects: [project]) # globally shared - shared_runner = create(:ci_runner, :shared) + shared_runner = create(:ci_runner, :instance) expect(described_class.owned_or_shared(project.id)).to contain_exactly( group_runner, project_runner, shared_runner @@ -183,31 +176,32 @@ describe Ci::Runner do 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') + runner = build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') expect(runner.display_name).to eq 'Linux/Ruby-1.9.3-p448' end it 'returns the token if it does not have a description' do - runner = FactoryBot.create(:ci_runner) + runner = create(:ci_runner) expect(runner.display_name).to eq runner.description end it 'returns the token if the description is an empty string' do - runner = FactoryBot.build(:ci_runner, description: '', token: 'token') + runner = build(:ci_runner, description: '', token: 'token') expect(runner.display_name).to eq runner.token end end describe '#assign_to' do - let!(:project) { FactoryBot.create(:project) } + let(:project) { create(:project) } subject { runner.assign_to(project) } context 'with shared_runner' do - let!(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } it 'transitions shared runner to project runner and assigns project' do - subject + expect(subject).to be_truthy + expect(runner).to be_specific expect(runner).to be_project_type expect(runner.projects).to eq([project]) @@ -216,7 +210,8 @@ describe Ci::Runner do end context 'with group runner' do - let!(:runner) { FactoryBot.create(:ci_runner, runner_type: :group_type) } + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } it 'raises an error' do expect { subject } @@ -229,15 +224,15 @@ describe Ci::Runner do subject { described_class.online } before do - @runner1 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.year.ago) - @runner2 = FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) + @runner1 = create(:ci_runner, :instance, contacted_at: 1.year.ago) + @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago) end it { is_expected.to eq([@runner2])} end describe '#online?' do - let(:runner) { FactoryBot.create(:ci_runner, :shared) } + let(:runner) { create(:ci_runner, :instance) } subject { runner.online? } @@ -307,21 +302,20 @@ describe Ci::Runner do end describe '#can_pick?' do - let(:pipeline) { create(:ci_pipeline) } + set(:pipeline) { create(:ci_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:runner) { create(:ci_runner, tag_list: tag_list, run_untagged: run_untagged) } + let(:runner_project) { build.project } + let(:runner) { create(:ci_runner, :project, projects: [runner_project], tag_list: tag_list, run_untagged: run_untagged) } let(:tag_list) { [] } let(:run_untagged) { true } subject { runner.can_pick?(build) } - before do - build.project.runners << runner - end - context 'a different runner' do + let(:other_project) { create(:project) } + let(:other_runner) { create(:ci_runner, :project, projects: [other_project], tag_list: tag_list, run_untagged: run_untagged) } + it 'cannot handle builds' do - other_runner = create(:ci_runner) expect(other_runner.can_pick?(build)).to be_falsey end end @@ -375,18 +369,14 @@ describe Ci::Runner do end context 'when runner is shared' do - let(:runner) { create(:ci_runner, :shared) } - - before do - build.project.runners = [] - end + let(:runner) { create(:ci_runner, :instance) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy end context 'when runner is locked' do - let(:runner) { create(:ci_runner, :shared, locked: true) } + let(:runner) { create(:ci_runner, :instance, locked: true) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -401,10 +391,8 @@ describe Ci::Runner do end end - context 'when runner is not assigned to a project' do - before do - build.project.runners = [] - end + context 'when runner is assigned to another project' do + let(:runner_project) { create(:project) } it 'cannot handle builds' do expect(runner.can_pick?(build)).to be_falsey @@ -412,10 +400,8 @@ describe Ci::Runner do end context 'when runner is assigned to a group' do - before do - build.project.runners = [] - runner.groups << create(:group, projects: [build.project]) - end + let(:group) { create(:group, projects: [build.project]) } + let(:runner) { create(:ci_runner, :group, tag_list: tag_list, run_untagged: run_untagged, groups: [group]) } it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy @@ -469,7 +455,7 @@ describe Ci::Runner do end describe '#status' do - let(:runner) { FactoryBot.create(:ci_runner, :shared, contacted_at: 1.second.ago) } + let(:runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } subject { runner.status } @@ -626,12 +612,13 @@ describe Ci::Runner do end describe '.assignable_for' do - let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) } - let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) } - let!(:group_runner) { create(:ci_runner, runner_type: :group_type) } - let!(:instance_runner) { create(:ci_runner, :shared) } let(:project) { create(:project) } + let(:group) { create(:group) } let(:another_project) { create(:project) } + let!(:unlocked_project_runner) { create(:ci_runner, :project, projects: [project]) } + let!(:locked_project_runner) { create(:ci_runner, :project, locked: true, projects: [project]) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } + let!(:instance_runner) { create(:ci_runner, :instance) } context 'with already assigned project' do subject { described_class.assignable_for(project) } @@ -651,19 +638,16 @@ describe Ci::Runner do describe "belongs_to_one_project?" do it "returns false if there are two projects runner assigned to" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project1 = FactoryBot.create(:project) - project.runners << runner - project1.runners << runner + project1 = create(:project) + project2 = create(:project) + runner = create(:ci_runner, :project, projects: [project1, project2]) expect(runner.belongs_to_one_project?).to be_falsey end it "returns true" do - runner = FactoryBot.create(:ci_runner) - project = FactoryBot.create(:project) - project.runners << runner + project = create(:project) + runner = create(:ci_runner, :project, projects: [project]) expect(runner.belongs_to_one_project?).to be_truthy end @@ -713,21 +697,21 @@ describe Ci::Runner do subject { runner.assigned_to_group? } context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Project runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when group runner' do let(:group) { create(:group) } - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } it { is_expected.to be_truthy } end @@ -737,18 +721,18 @@ describe Ci::Runner do subject { runner.assigned_to_project? } context 'when group runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let(:runner) { create(:ci_runner, :group, description: 'Group runner', groups: [group]) } let(:group) { create(:group) } it { is_expected.to be_falsey } end context 'when shared runner' do - let(:runner) { create(:ci_runner, :shared, description: 'Shared runner') } + let(:runner) { create(:ci_runner, :instance, description: 'Shared runner') } it { is_expected.to be_falsey } end context 'when project runner' do - let(:runner) { create(:ci_runner, description: 'Group runner', projects: [project]) } + let(:runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } let(:project) { create(:project) } it { is_expected.to be_truthy } @@ -780,4 +764,17 @@ describe Ci::Runner do end end end + + describe 'project runner without projects is destroyable' do + subject { create(:ci_runner, :project, :without_projects) } + + it 'does not have projects' do + expect(subject.runner_projects).to be_empty + end + + it 'can be destroyed' do + subject + expect { subject.destroy }.to change { described_class.count }.by(-1) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index af2240f4f89..9a76452a808 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1177,8 +1177,8 @@ describe Project 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) } - let(:shared_runner) { create(:ci_runner, :shared) } + let(:specific_runner) { create(:ci_runner, :project, projects: [project]) } + let(:shared_runner) { create(:ci_runner, :instance) } context 'for shared runners disabled' do let(:shared_runners_enabled) { false } @@ -1188,7 +1188,7 @@ describe Project do end it 'has a specific runner' do - project.runners << specific_runner + specific_runner expect(project.any_runners?).to be_truthy end @@ -1200,13 +1200,13 @@ describe Project do end it 'checks the presence of specific runner' do - project.runners << specific_runner + 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 + specific_runner expect(project.any_runners? { false }).to be_falsey end @@ -1238,7 +1238,7 @@ describe Project do 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]) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } context 'for group runners disabled' do let(:group_runners_enabled) { false } @@ -1279,7 +1279,7 @@ describe Project do end describe '#shared_runners' do - let!(:runner) { create(:ci_runner, :shared) } + let!(:runner) { create(:ci_runner, :instance) } subject { project.shared_runners } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 16b409844fa..09dfeae6377 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1858,13 +1858,10 @@ describe User do describe '#ci_owned_runners' do let(:user) { create(:user) } - let(:runner_1) { create(:ci_runner) } - let(:runner_2) { create(:ci_runner) } + let!(:project) { create(:project) } + let(:runner) { create(:ci_runner, :project, projects: [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_owned_runners).to be_empty end @@ -1872,38 +1869,40 @@ 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_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end context 'with personal group runner' do - let!(:project) { create(:project, runners: [runner_1]) } + let!(:project) { create(:project) } + let(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_2) + expect(user.ci_owned_runners).to contain_exactly(group_runner) 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!(:project) { create(:project, namespace: namespace) } + let!(:group_runner) { create(:ci_runner, :group, groups: [group]) } let!(:group) do - create(:group, runners: [runner_2]).tap do |group| + create(:group).tap do |group| group.add_owner(user) end end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2) + expect(user.ci_owned_runners).to contain_exactly(runner, group_runner) end end @@ -1914,7 +1913,7 @@ describe User do end it 'loads' do - expect(user.ci_owned_runners).to contain_exactly(runner_1) + expect(user.ci_owned_runners).to contain_exactly(runner) end end @@ -1931,7 +1930,7 @@ describe User do context 'with groups projects runners' do let(:group) { create(:group) } - let!(:project) { create(:project, group: group, runners: [runner_1]) } + let!(:project) { create(:project, group: group) } def add_user(access) group.add_user(user, access) @@ -1941,11 +1940,8 @@ describe User do end context 'with groups runners' do - let!(:group) do - create(:group, runners: [runner_1]).tap do |group| - group.add_owner(user) - end - end + let!(:runner) { create(:ci_runner, :group, groups: [group]) } + let!(:group) { create(:group) } def add_user(access) group.add_user(user, access) @@ -1955,7 +1951,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) @@ -1968,7 +1964,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) |