From 07f7a01b4da02533c417fa636f004fedf3c4c661 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Thu, 31 Aug 2017 18:28:29 +0900
Subject: [PATCH] Improve spec. Add validation for accel_level on runner.

---
 app/models/ci/runner.rb                       |  1 +
 spec/factories/ci/runners.rb                  |  4 +-
 spec/models/ci/build_spec.rb                  | 22 ++++++--
 spec/models/ci/runner_spec.rb                 | 54 ++++++++++---------
 spec/services/ci/register_job_service_spec.rb |  6 ++-
 5 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index f202f08b151..b1798084787 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -35,6 +35,7 @@ module Ci
     end
 
     validate :tag_constraints
+    validates :access_level, presence: true
 
     acts_as_taggable
 
diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb
index 249d8bde5a1..efd9b34fb3f 100644
--- a/spec/factories/ci/runners.rb
+++ b/spec/factories/ci/runners.rb
@@ -23,11 +23,11 @@ FactoryGirl.define do
     end
 
     trait :ref_protected do
-      access_level 'ref_protected'
+      access_level :ref_protected
     end
 
     trait :not_protected do
-      access_level 'not_protected'
+      access_level :not_protected
     end
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index ef6c91d11f1..a94fdbf8434 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -44,13 +44,25 @@ describe Ci::Build do
   end
 
   describe '.ref_protected' do
-    let!(:protected_job) { create(:ci_build, :protected) }
-    let!(:unprotected_job) { create(:ci_build, :unprotected) }
-
     subject { described_class.ref_protected }
 
-    it { is_expected.to include(protected_job) }
-    it { is_expected.not_to include(unprotected_job) }
+    context 'when protected is true' do
+      let!(:job) { create(:ci_build, :protected) }
+
+      it { is_expected.to include(job) }
+    end
+
+    context 'when protected is false' do
+      let!(:job) { create(:ci_build, :unprotected) }
+
+      it { is_expected.not_to include(job) }
+    end
+
+    context 'when protected is false' do
+      let!(:job) { create(:ci_build, protected: nil) }
+
+      it { is_expected.not_to include(job) }
+    end
   end
 
   describe '#actionize' do
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 8c157d4369b..ca48372c5e2 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -2,6 +2,8 @@ require 'spec_helper'
 
 describe Ci::Runner do
   describe 'validation' do
+    it { is_expected.to validate_presence_of(:access_level) }
+
     context 'when runner is not allowed to pick untagged jobs' do
       context 'when runner does not have tags' do
         it 'is not valid' do
@@ -19,6 +21,34 @@ describe Ci::Runner do
     end
   end
 
+  describe '#access_level' do
+    context 'when creating new runner and access_level is nil' do
+      let(:runner) do
+        build(:ci_runner, access_level: nil)
+      end
+
+      it "object is invalid" do
+        expect(runner).not_to be_valid
+      end
+    end
+
+    context 'when creating new runner and access_level is defined in enum' do
+      let(:runner) do
+        build(:ci_runner, access_level: :not_protected)
+      end
+
+      it "object is valid" do
+        expect(runner).to be_valid
+      end
+    end
+
+    context 'when creating new runner and access_level is not defined in enum' do
+      it "raises an error" do
+        expect { build(:ci_runner, access_level: :this_is_not_defined) }.to raise_error(ArgumentError)
+      end
+    end
+  end
+
   describe '#display_name' do
     it 'returns the description if it has a value' do
       runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
@@ -480,28 +510,4 @@ describe Ci::Runner do
       expect(described_class.search(runner.description.upcase)).to eq([runner])
     end
   end
-
-  describe '.access_level' do
-    context 'when access_level of a runner is ref_protected' do
-      before do
-        create(:ci_runner, :ref_protected)
-      end
-
-      it 'a protected runner exists' do
-        expect(described_class.count).to eq(1)
-        expect(described_class.last.ref_protected?).to eq(true)
-      end
-    end
-
-    context 'when access_level of a runner is not_protected' do
-      before do
-        create(:ci_runner, :not_protected)
-      end
-
-      it 'an not_protected runner exists' do
-        expect(described_class.count).to eq(1)
-        expect(described_class.last.not_protected?).to eq(true)
-      end
-    end
-  end
 end
diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb
index e9bcb41c5f5..6f8ebf5d3ba 100644
--- a/spec/services/ci/register_job_service_spec.rb
+++ b/spec/services/ci/register_job_service_spec.rb
@@ -215,7 +215,9 @@ module Ci
         end
       end
 
-      context 'when a runner is not_protected' do
+      context 'when access_level of runner is not_protected' do
+        let!(:specific_runner) { create(:ci_runner, :not_protected, :specific) }
+
         context 'when a job is protected' do
           let!(:pending_build) { create(:ci_build, :protected, pipeline: pipeline) }
 
@@ -233,7 +235,7 @@ module Ci
         end
       end
 
-      context 'when a runner is ref_protected' do
+      context 'when access_level of runner is ref_protected' do
         let!(:specific_runner) { create(:ci_runner, :ref_protected, :specific) }
 
         context 'when a job is protected' do
-- 
GitLab