From a52e3edd1a1869c2656193c95f8f2e8fd3bc8fa2 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 4 Apr 2018 21:31:56 +0200 Subject: [PATCH] Specify default value for Project#build_timeout --- .../concerns/chronic_duration_attribute.rb | 6 ++-- app/models/project.rb | 2 +- .../chronic_duration_attribute_spec.rb | 34 +++++++++++++------ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/app/models/concerns/chronic_duration_attribute.rb b/app/models/concerns/chronic_duration_attribute.rb index fa1eafb1d7a..593a9b3d71d 100644 --- a/app/models/concerns/chronic_duration_attribute.rb +++ b/app/models/concerns/chronic_duration_attribute.rb @@ -8,14 +8,14 @@ module ChronicDurationAttribute end end - def chronic_duration_attr_writer(virtual_attribute, source_attribute) + def chronic_duration_attr_writer(virtual_attribute, source_attribute, parameters = {}) chronic_duration_attr_reader(virtual_attribute, source_attribute) define_method("#{virtual_attribute}=") do |value| - chronic_duration_attributes[virtual_attribute] = value.presence || '' + chronic_duration_attributes[virtual_attribute] = value.presence || parameters[:default].presence.to_s begin - new_value = ChronicDuration.parse(value).to_i if value.present? + new_value = value.present? ? ChronicDuration.parse(value).to_i : parameters[:default].presence assign_attributes(source_attribute => new_value) rescue ChronicDuration::DurationParseError # ignore error as it will be caught by validation diff --git a/app/models/project.rb b/app/models/project.rb index 8a325f49ecb..cfd00e8a73c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -326,7 +326,7 @@ class Project < ActiveRecord::Base enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } - chronic_duration_attr :build_timeout_human_readable, :build_timeout + chronic_duration_attr :build_timeout_human_readable, :build_timeout, default: 3600 validates :build_timeout, allow_nil: true, numericality: { greater_than_or_equal_to: 600, diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index 27c86e60e60..8847623f705 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -63,8 +63,8 @@ shared_examples 'ChronicDurationAttribute writer' do subject.send("#{virtual_field}=", '') end - it 'writes nil' do - expect(subject.send(source_field)).to be_nil + it 'writes default value' do + expect(subject.send(source_field)).to eq(default_value) end it 'passes validation' do @@ -77,8 +77,8 @@ shared_examples 'ChronicDurationAttribute writer' do subject.send("#{virtual_field}=", nil) end - it 'writes nil' do - expect(subject.send(source_field)).to be_nil + it 'writes default value' do + expect(subject.send(source_field)).to eq(default_value) end it 'passes validation' do @@ -92,20 +92,34 @@ shared_examples 'ChronicDurationAttribute writer' do end describe 'ChronicDurationAttribute' do - let(:source_field) {:maximum_timeout} - let(:virtual_field) {:maximum_timeout_human_readable} + context 'when default value is not set' do + let(:source_field) {:maximum_timeout} + let(:virtual_field) {:maximum_timeout_human_readable} + let(:default_value) { nil } - subject { Ci::Runner.new } + subject { create(:ci_runner) } - it_behaves_like 'ChronicDurationAttribute reader' - it_behaves_like 'ChronicDurationAttribute writer' + it_behaves_like 'ChronicDurationAttribute reader' + it_behaves_like 'ChronicDurationAttribute writer' + end + + context 'when default value is set' do + let(:source_field) {:build_timeout} + let(:virtual_field) {:build_timeout_human_readable} + let(:default_value) { 3600 } + + subject { create(:project) } + + it_behaves_like 'ChronicDurationAttribute reader' + it_behaves_like 'ChronicDurationAttribute writer' + end end describe 'ChronicDurationAttribute - reader' do let(:source_field) {:timeout} let(:virtual_field) {:timeout_human_readable} - subject {Ci::BuildMetadata.new} + subject { create(:ci_build).ensure_metadata } it "doesn't contain dynamically created writer method" do expect(subject.class).not_to be_public_method_defined("#{virtual_field}=")