From 268f7713a9c076204a8b6badb847d2e23bafdd71 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 8 Jun 2016 15:19:49 +0800
Subject: [PATCH] Remove Build#can_be_served? and rename Runner#can_serve? to
can_pick?
This also moves tests from build_spec.rb to runner_spec.rb
---
app/models/ci/build.rb | 6 +-
app/models/ci/runner.rb | 2 +-
app/services/ci/register_build_service.rb | 2 +-
spec/models/build_spec.rb | 118 +--------------------
spec/models/ci/runner_spec.rb | 119 ++++++++++++++++++++++
5 files changed, 123 insertions(+), 124 deletions(-)
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 860ac16eefd..a3a30fe17c2 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -290,16 +290,12 @@ module Ci
project.valid_runners_token? token
end
- def can_be_served?(runner)
- runner.can_serve?(self)
- end
-
def has_tags?
tag_list.any?
end
def any_runners_online?
- project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) }
+ project.any_runners? { |runner| runner.active? && runner.online? && runner.can_pick?(self) }
end
def stuck?
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index bb5340a0f03..44f820d100b 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -98,7 +98,7 @@ module Ci
!shared?
end
- def can_serve?(build)
+ def can_pick?(build)
not_locked_or_locked_to?(build.project) &&
run_untagged_or_has_tags?(build) &&
accepting_tags?(build.tag_list)
diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb
index 4ff268a6f06..511505bc9a9 100644
--- a/app/services/ci/register_build_service.rb
+++ b/app/services/ci/register_build_service.rb
@@ -17,7 +17,7 @@ module Ci
builds = builds.order('created_at ASC')
build = builds.find do |build|
- build.can_be_served?(current_runner)
+ current_runner.can_pick?(build)
end
if build
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 1d4b8bc4c98..e703a07013b 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -275,122 +275,6 @@ describe Ci::Build, models: true do
end
end
- describe '#can_be_served?' do
- let(:runner) { create(:ci_runner) }
-
- before do
- build.project.runners << runner
- end
-
- context 'when runner does not have tags' do
- it 'can handle builds without tags' do
- expect(build.can_be_served?(runner)).to be_truthy
- end
-
- it 'cannot handle build with tags' do
- build.tag_list = ['aa']
- expect(build.can_be_served?(runner)).to be_falsey
- end
- end
-
- context 'when runner has tags' do
- before do
- runner.tag_list = ['bb', 'cc']
- end
-
- shared_examples 'tagged build picker' do
- it 'can handle build with matching tags' do
- build.tag_list = ['bb']
- expect(build.can_be_served?(runner)).to be_truthy
- end
-
- it 'cannot handle build without matching tags' do
- build.tag_list = ['aa']
- expect(build.can_be_served?(runner)).to be_falsey
- end
- end
-
- context 'when runner can pick untagged jobs' do
- it 'can handle builds without tags' do
- expect(build.can_be_served?(runner)).to be_truthy
- end
-
- it_behaves_like 'tagged build picker'
- end
-
- context 'when runner cannot pick untagged jobs' do
- before do
- runner.run_untagged = false
- end
-
- it 'cannot handle builds without tags' do
- expect(build.can_be_served?(runner)).to be_falsey
- end
-
- it_behaves_like 'tagged build picker'
- end
- end
-
- context 'when runner is locked' do
- before do
- runner.locked = true
- end
-
- shared_examples 'locked build picker' do |serve_matching_tags|
- context 'when runner cannot pick untagged jobs' do
- before do
- runner.run_untagged = false
- end
-
- it 'cannot handle builds without tags' do
- expect(build.can_be_served?(runner)).to be_falsey
- end
- end
-
- context 'when having runner tags' do
- before do
- runner.tag_list = ['bb', 'cc']
- end
-
- it "#{serve_matching_tags} handle it for matching tags" do
- build.tag_list = ['bb']
- expected = if serve_matching_tags
- be_truthy
- else
- be_falsey
- end
- expect(build.can_be_served?(runner)).to expected
- end
-
- it 'cannot handle it for builds without matching tags' do
- build.tag_list = ['aa']
- expect(build.can_be_served?(runner)).to be_falsey
- end
- end
- end
-
- context 'when serving the same project' do
- it 'can handle it' do
- expect(build.can_be_served?(runner)).to be_truthy
- end
-
- it_behaves_like 'locked build picker', true
- end
-
- context 'serving a different project' do
- before do
- runner.runner_projects.destroy_all
- end
-
- it 'cannot handle it' do
- expect(build.can_be_served?(runner)).to be_falsey
- end
-
- it_behaves_like 'locked build picker', false
- end
- end
- end
-
describe '#has_tags?' do
context 'when build has tags' do
subject { create(:ci_build, tag_list: ['tag']) }
@@ -431,7 +315,7 @@ describe Ci::Build, models: true do
end
it 'that cannot handle build' do
- expect_any_instance_of(Ci::Build).to receive(:can_be_served?).and_return(false)
+ expect_any_instance_of(Ci::Runner).to receive(:can_pick?).and_return(false)
is_expected.to be_falsey
end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index c3b715dcb92..477a32a1f9e 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -90,6 +90,125 @@ describe Ci::Runner, models: true do
end
end
+ describe '#can_pick?' do
+ let(:project) { create(:project) }
+ let(:commit) { create(:ci_commit, project: project) }
+ let(:build) { create(:ci_build, commit: commit) }
+ let(:runner) { create(:ci_runner) }
+
+ before do
+ build.project.runners << runner
+ end
+
+ context 'when runner does not have tags' do
+ it 'can handle builds without tags' do
+ expect(runner.can_pick?(build)).to be_truthy
+ end
+
+ it 'cannot handle build with tags' do
+ build.tag_list = ['aa']
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+ end
+
+ context 'when runner has tags' do
+ before do
+ runner.tag_list = ['bb', 'cc']
+ end
+
+ shared_examples 'tagged build picker' do
+ it 'can handle build with matching tags' do
+ build.tag_list = ['bb']
+ expect(runner.can_pick?(build)).to be_truthy
+ end
+
+ it 'cannot handle build without matching tags' do
+ build.tag_list = ['aa']
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+ end
+
+ context 'when runner can pick untagged jobs' do
+ it 'can handle builds without tags' do
+ expect(runner.can_pick?(build)).to be_truthy
+ end
+
+ it_behaves_like 'tagged build picker'
+ end
+
+ context 'when runner cannot pick untagged jobs' do
+ before do
+ runner.run_untagged = false
+ end
+
+ it 'cannot handle builds without tags' do
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+
+ it_behaves_like 'tagged build picker'
+ end
+ end
+
+ context 'when runner is locked' do
+ before do
+ runner.locked = true
+ end
+
+ shared_examples 'locked build picker' do |serve_matching_tags|
+ context 'when runner cannot pick untagged jobs' do
+ before do
+ runner.run_untagged = false
+ end
+
+ it 'cannot handle builds without tags' do
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+ end
+
+ context 'when having runner tags' do
+ before do
+ runner.tag_list = ['bb', 'cc']
+ end
+
+ it "#{serve_matching_tags} handle it for matching tags" do
+ build.tag_list = ['bb']
+ expected = if serve_matching_tags
+ be_truthy
+ else
+ be_falsey
+ end
+ expect(runner.can_pick?(build)).to expected
+ end
+
+ it 'cannot handle it for builds without matching tags' do
+ build.tag_list = ['aa']
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+ end
+ end
+
+ context 'when serving the same project' do
+ it 'can handle it' do
+ expect(runner.can_pick?(build)).to be_truthy
+ end
+
+ it_behaves_like 'locked build picker', true
+ end
+
+ context 'serving a different project' do
+ before do
+ runner.runner_projects.destroy_all
+ end
+
+ it 'cannot handle it' do
+ expect(runner.can_pick?(build)).to be_falsey
+ end
+
+ it_behaves_like 'locked build picker', false
+ end
+ end
+ end
+
describe '#status' do
let(:runner) { FactoryGirl.create(:ci_runner, :shared, contacted_at: 1.second.ago) }
--
GitLab