From 114c26ccf0f10788271c6108774e72809a7f93e1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 4 Jun 2018 11:29:39 +0200
Subject: [PATCH] Raise error if pipeline / stage hits unknown status

---
 app/models/ci/pipeline.rb         |  6 ++++-
 app/models/ci/stage.rb            |  4 ++++
 app/models/concerns/has_status.rb |  2 ++
 spec/models/ci/pipeline_spec.rb   | 37 +++++++++++++++++++++++++++++++
 spec/models/ci/stage_spec.rb      | 13 +++++++++++
 5 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 94a548181d9..1274d2a5f16 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -517,7 +517,8 @@ module Ci
 
     def update_status
       retry_optimistic_lock(self) do
-        case latest_builds_status
+        case latest_builds_status.to_s
+        when 'created' then nil
         when 'pending' then enqueue
         when 'running' then run
         when 'success' then succeed
@@ -525,6 +526,9 @@ module Ci
         when 'canceled' then cancel
         when 'skipped' then skip
         when 'manual' then block
+        else
+          raise HasStatus::UnknownStatusError,
+                "Unknown status `#{latest_builds_status}`"
         end
       end
     end
diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb
index abf73c00539..ea07f37e6c1 100644
--- a/app/models/ci/stage.rb
+++ b/app/models/ci/stage.rb
@@ -68,6 +68,7 @@ module Ci
     def update_status
       retry_optimistic_lock(self) do
         case statuses.latest.status
+        when 'created' then nil
         when 'pending' then enqueue
         when 'running' then run
         when 'success' then succeed
@@ -75,6 +76,9 @@ module Ci
         when 'canceled' then cancel
         when 'manual' then block
         when 'skipped', nil then skip
+        else
+          raise HasStatus::UnknownStatusError,
+                "Unknown status `#{statuses.latest.status}`"
         end
       end
     end
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 7c3ed96bc28..72c236a0fc7 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -11,6 +11,8 @@ module HasStatus
   STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3,
                     failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
 
+  UnknownStatusError = Class.new(StandardError)
+
   class_methods do
     def status_sql
       scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index a29fa0c4cae..decfef6c8cc 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -1251,6 +1251,43 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#update_status' do
+    context 'when pipeline is empty' do
+      it 'updates does not change pipeline status' do
+        expect(pipeline.statuses.latest.status).to be_nil
+
+        expect { pipeline.update_status }
+          .to change { pipeline.reload.status }.to 'skipped'
+      end
+    end
+
+    context 'when updating status to pending' do
+      before do
+        allow(pipeline)
+          .to receive_message_chain(:statuses, :latest, :status)
+          .and_return(:running)
+      end
+
+      it 'updates pipeline status to running' do
+        expect { pipeline.update_status }
+          .to change { pipeline.reload.status }.to 'running'
+      end
+    end
+
+    context 'when statuses status was not recognized' do
+      before do
+        allow(pipeline)
+          .to receive(:latest_builds_status)
+          .and_return(:unknown)
+      end
+
+      it 'raises an exception' do
+        expect { pipeline.update_status }
+          .to raise_error(HasStatus::UnknownStatusError)
+      end
+    end
+  end
+
   describe '#detailed_status' do
     subject { pipeline.detailed_status(user) }
 
diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb
index b40496252b4..22a4556c10c 100644
--- a/spec/models/ci/stage_spec.rb
+++ b/spec/models/ci/stage_spec.rb
@@ -110,6 +110,19 @@ describe Ci::Stage, :models do
         expect(stage.reload).to be_failed
       end
     end
+
+    context 'when statuses status was not recognized' do
+      before do
+        allow(stage)
+          .to receive_message_chain(:statuses, :latest, :status)
+          .and_return(:unknown)
+      end
+
+      it 'raises an exception' do
+        expect { stage.update_status }
+          .to raise_error(HasStatus::UnknownStatusError)
+      end
+    end
   end
 
   describe '#detailed_status' do