Commit d3991289 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Handle after script CI config in new classes

This also makes Script to return an array of commands instead of
concatented command, which is our current direction.
parent fc00c545
...@@ -14,7 +14,7 @@ module Ci ...@@ -14,7 +14,7 @@ module Ci
ALLOWED_CACHE_KEYS = [:key, :untracked, :paths] ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in] ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
attr_reader :after_script, :path, :cache attr_reader :path, :cache
def initialize(config, path = nil) def initialize(config, path = nil)
@ci_config = Gitlab::Ci::Config.new(config) @ci_config = Gitlab::Ci::Config.new(config)
...@@ -65,10 +65,9 @@ module Ci ...@@ -65,10 +65,9 @@ module Ci
def initial_parsing def initial_parsing
@before_script = @ci_config.before_script @before_script = @ci_config.before_script
@image = @ci_config.image @image = @ci_config.image
@after_script = @ci_config.after_script
@after_script = @config[:after_script]
@image = @config[:image]
@services = @ci_config.services @services = @ci_config.services
@stages = @config[:stages] || @config[:types] @stages = @config[:stages] || @config[:types]
@variables = @config[:variables] || {} @variables = @config[:variables] || {}
@cache = @config[:cache] @cache = @config[:cache]
...@@ -93,7 +92,7 @@ module Ci ...@@ -93,7 +92,7 @@ module Ci
{ {
stage_idx: stages.index(job[:stage]), stage_idx: stages.index(job[:stage]),
stage: job[:stage], stage: job[:stage],
commands: [job[:before_script] || [@before_script], job[:script]].flatten.compact.join("\n"), commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
tag_list: job[:tags] || [], tag_list: job[:tags] || [],
name: name, name: name,
only: job[:only], only: job[:only],
...@@ -123,10 +122,6 @@ module Ci ...@@ -123,10 +122,6 @@ module Ci
end end
def validate_global! def validate_global!
unless @after_script.nil? || validate_array_of_strings(@after_script)
raise ValidationError, "after_script should be an array of strings"
end
unless @stages.nil? || validate_array_of_strings(@stages) unless @stages.nil? || validate_array_of_strings(@stages)
raise ValidationError, "stages should be an array of strings" raise ValidationError, "stages should be an array of strings"
end end
......
...@@ -7,7 +7,7 @@ module Gitlab ...@@ -7,7 +7,7 @@ module Gitlab
## ##
# Temporary delegations that should be removed after refactoring # Temporary delegations that should be removed after refactoring
# #
delegate :before_script, :image, :services, to: :@global delegate :before_script, :image, :services, :after_script, to: :@global
def initialize(config) def initialize(config)
@config = Loader.new(config).load! @config = Loader.new(config).load!
......
...@@ -17,6 +17,9 @@ module Gitlab ...@@ -17,6 +17,9 @@ module Gitlab
allow_node :services, Services, allow_node :services, Services,
description: 'Docker images that will be linked to the container.' description: 'Docker images that will be linked to the container.'
allow_node :after_script, Script,
description: 'Script that will be executed after each job.'
end end
end end
end end
......
...@@ -5,11 +5,6 @@ module Gitlab ...@@ -5,11 +5,6 @@ module Gitlab
## ##
# Entry that represents a script. # Entry that represents a script.
# #
# Each element in the value array is a command that will be executed
# by GitLab Runner. Currently we concatenate these commands with
# new line character as a separator, what is compatible with
# implementation in Runner.
#
class Script < Entry class Script < Entry
include Validatable include Validatable
...@@ -18,7 +13,7 @@ module Gitlab ...@@ -18,7 +13,7 @@ module Gitlab
end end
def value def value
@config.join("\n") @config
end end
end end
end end
......
...@@ -965,7 +965,7 @@ EOT ...@@ -965,7 +965,7 @@ EOT
config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } }) config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
expect do expect do
GitlabCiYamlProcessor.new(config, path) GitlabCiYamlProcessor.new(config, path)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings") end.to raise_error(GitlabCiYamlProcessor::ValidationError, "After script config should be an array of strings")
end end
it "returns errors if job after_script parameter is not an array of strings" do it "returns errors if job after_script parameter is not an array of strings" do
......
...@@ -11,7 +11,7 @@ describe Gitlab::Ci::Config::Node::Factory do ...@@ -11,7 +11,7 @@ describe Gitlab::Ci::Config::Node::Factory do
.with(value: ['ls', 'pwd']) .with(value: ['ls', 'pwd'])
.create! .create!
expect(entry.value).to eq "ls\npwd" expect(entry.value).to eq ['ls', 'pwd']
end end
context 'when setting description' do context 'when setting description' do
...@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Factory do ...@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Factory do
.with(description: 'test description') .with(description: 'test description')
.create! .create!
expect(entry.value).to eq "ls\npwd" expect(entry.value).to eq ['ls', 'pwd']
expect(entry.description).to eq 'test description' expect(entry.description).to eq 'test description'
end end
end end
......
...@@ -23,7 +23,8 @@ describe Gitlab::Ci::Config::Node::Global do ...@@ -23,7 +23,8 @@ describe Gitlab::Ci::Config::Node::Global do
let(:hash) do let(:hash) do
{ before_script: ['ls', 'pwd'], { before_script: ['ls', 'pwd'],
image: 'ruby:2.2', image: 'ruby:2.2',
services: ['postgres:9.1', 'mysql:5.5'] } services: ['postgres:9.1', 'mysql:5.5'],
after_script: ['make clean'] }
end end
describe '#process!' do describe '#process!' do
...@@ -34,7 +35,7 @@ describe Gitlab::Ci::Config::Node::Global do ...@@ -34,7 +35,7 @@ describe Gitlab::Ci::Config::Node::Global do
end end
it 'creates node object for each entry' do it 'creates node object for each entry' do
expect(global.nodes.count).to eq 3 expect(global.nodes.count).to eq 4
end end
it 'creates node object using valid class' do it 'creates node object using valid class' do
...@@ -71,7 +72,7 @@ describe Gitlab::Ci::Config::Node::Global do ...@@ -71,7 +72,7 @@ describe Gitlab::Ci::Config::Node::Global do
describe '#before_script' do describe '#before_script' do
it 'returns correct script' do it 'returns correct script' do
expect(global.before_script).to eq "ls\npwd" expect(global.before_script).to eq ['ls', 'pwd']
end end
end end
...@@ -86,6 +87,12 @@ describe Gitlab::Ci::Config::Node::Global do ...@@ -86,6 +87,12 @@ describe Gitlab::Ci::Config::Node::Global do
expect(global.services).to eq ['postgres:9.1', 'mysql:5.5'] expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
end end
end end
describe '#after_script' do
it 'returns after script' do
expect(global.after_script).to eq ['make clean']
end
end
end end
end end
......
...@@ -10,8 +10,8 @@ describe Gitlab::Ci::Config::Node::Script do ...@@ -10,8 +10,8 @@ describe Gitlab::Ci::Config::Node::Script do
let(:config) { ['ls', 'pwd'] } let(:config) { ['ls', 'pwd'] }
describe '#value' do describe '#value' do
it 'returns concatenated command' do it 'returns array of strings' do
expect(entry.value).to eq "ls\npwd" expect(entry.value).to eq config
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment