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

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) }