Change and rename behavior of save_timeout_state!

4 files changed, 33 insertions, 10 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 355d1c0523f..073d73f0426 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -157,7 +157,7 @@ module Ci
       end
 
       before_transition pending: :running do |build|
-        build.ensure_metadata.save_timeout_state!
+        build.ensure_metadata.update_timeout_state
       end
     end
 
diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb
index 6d8a895d509..de5b4170201 100644
--- a/app/models/ci/build_metadata.rb
+++ b/app/models/ci/build_metadata.rb
@@ -21,14 +21,14 @@ module Ci
         runner_timeout_source: 3
     }
 
-    def save_timeout_state!
+    def update_timeout_state
       return unless build.runner.present?
 
       project_timeout = project&.build_timeout
       timeout = [project_timeout, build.runner.maximum_timeout].compact.min
       timeout_source = timeout < project_timeout ? :runner_timeout_source : :project_timeout_source
 
-      update!(timeout: timeout, timeout_source: timeout_source)
+      update(timeout: timeout, timeout_source: timeout_source)
     end
   end
 end
diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb
index d21e9600e42..268561ee941 100644
--- a/spec/models/ci/build_metadata_spec.rb
+++ b/spec/models/ci/build_metadata_spec.rb
@@ -15,16 +15,16 @@ describe Ci::BuildMetadata do
   let(:build) { create(:ci_build, pipeline: pipeline) }
   let(:build_metadata) { create(:ci_build_metadata, build: build) }
 
-  describe '#save_timeout_state!' do
+  describe '#update_timeout_state' do
     subject { build_metadata }
 
     context 'when runner is not assigned to the job' do
       it "doesn't change timeout value" do
-        expect { subject.save_timeout_state! }.not_to change { subject.reload.timeout }
+        expect { subject.update_timeout_state }.not_to change { subject.reload.timeout }
       end
 
       it "doesn't change timeout_source value" do
-        expect { subject.save_timeout_state! }.not_to change { subject.reload.timeout_source }
+        expect { subject.update_timeout_state }.not_to change { subject.reload.timeout_source }
       end
     end
 
@@ -37,11 +37,11 @@ describe Ci::BuildMetadata do
         let(:runner) { create(:ci_runner, maximum_timeout: 1900) }
 
         it 'sets runner timeout' do
-          expect { subject.save_timeout_state! }.to change { subject.reload.timeout }.to(1900)
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(1900)
         end
 
         it 'sets runner_timeout_source' do
-          expect { subject.save_timeout_state! }.to change { subject.reload.timeout_source }.to('runner_timeout_source')
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('runner_timeout_source')
         end
       end
 
@@ -49,11 +49,11 @@ describe Ci::BuildMetadata do
         let(:runner) { create(:ci_runner, maximum_timeout: 2100) }
 
         it 'sets project timeout' do
-          expect { subject.save_timeout_state! }.to change { subject.reload.timeout }.to(2000)
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout }.to(2000)
         end
 
         it 'sets project_timeout_source' do
-          expect { subject.save_timeout_state! }.to change { subject.reload.timeout_source }.to('project_timeout_source')
+          expect { subject.update_timeout_state }.to change { subject.reload.timeout_source }.to('project_timeout_source')
         end
       end
     end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 14dec2d4fc9..920b2284eb1 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -2050,6 +2050,11 @@ describe Ci::Build do
       job.project.update_attribute(:build_timeout, 1800)
     end
 
+    def run_job_without_exception
+      job.run!
+    rescue StateMachines::InvalidTransition
+    end
+
     shared_examples 'saves data on transition' do
       it 'saves timeout' do
         expect { job.run! }.to change { job.reload.ensure_metadata.timeout }.from(nil).to(expected_timeout)
@@ -2058,6 +2063,24 @@ describe Ci::Build do
       it 'saves timeout_source' do
         expect { job.run! }.to change { job.reload.ensure_metadata.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source)
       end
+
+      context 'when Ci::BuildMetadata#update_timeout_state fails update' do
+        before do
+          allow_any_instance_of(Ci::BuildMetadata).to receive(:update_timeout_state).and_return(false)
+        end
+
+        it "doesn't save timeout" do
+          expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout_source }
+        end
+
+        it "doesn't save timeout_source" do
+          expect { run_job_without_exception }.not_to change { job.reload.ensure_metadata.timeout_source }
+        end
+
+        it 'raises an exception' do
+          expect { job.run! }.to raise_error(StateMachines::InvalidTransition)
+        end
+      end
     end
 
     context 'when runner timeout overrides project timeout' do