diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index acc2dbb59fd..9642c94a231 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -363,7 +363,7 @@ module Ci return [] unless config_processor strong_memoize(:stage_seeds) do - seeds = config_processor.stages.map do |attributes| + seeds = config_processor.stages_attributes.map do |attributes| Gitlab::Ci::Pipeline::Seed::Stage.new(self, attributes) end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 6074829e152..bc2a6f98dae 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -27,7 +27,7 @@ module Gitlab end def build_attributes(name) - job = @jobs[name.to_sym] || {} + job = @jobs.fetch(name.to_sym, {}) { stage_idx: @stages.index(job[:stage]), stage: job[:stage], @@ -53,39 +53,23 @@ module Gitlab }.compact } end - # REFACTORING, this needs improvement, and specs - # - def stage_attributes(stage) - selected = @jobs.values.select do |job| - job[:stage] == stage - end - - selected.map do |job| - build_attributes(job[:name]) - end + def stage_builds_attributes(stage) + @jobs.values + .select { |job| job[:stage] == stage } + .map { |job| build_attributes(job[:name]) } end - # REFACTORING, needs specs - # - def seed_attributes(stage) - seeds = stage_attributes(stage).map do |attributes| - job = @jobs.fetch(attributes[:name].to_sym) - - attributes - .merge(only: job.fetch(:only, {})) - .merge(except: job.fetch(:except, {})) - end - - { name: stage, - index: @stages.index(stage), - builds: seeds } - end - - # REFACTORING, needs specs - # - def stages + def stages_attributes @stages.uniq.map do |stage| - seed_attributes(stage) + seeds = stage_builds_attributes(stage).map do |attributes| + job = @jobs.fetch(attributes[:name].to_sym) + + attributes + .merge(only: job.fetch(:only, {})) + .merge(except: job.fetch(:except, {})) + end + + { name: stage, index: @stages.index(stage), builds: seeds } end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 94ead73e875..116573379e0 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -89,13 +89,13 @@ describe Gitlab::Ci::Pipeline::Seed::Build do context 'when branch policy matches' do context 'when using only' do - let(:attributes) { { name: 'rspec', only: { refs: ['deploy', 'master'] } } } + let(:attributes) { { name: 'rspec', only: { refs: %w[deploy master] } } } it { is_expected.to be_included } end context 'when using except' do - let(:attributes) { { name: 'rspec', except: { refs: ['deploy', 'master'] } } } + let(:attributes) { { name: 'rspec', except: { refs: %w[deploy master] } } } it { is_expected.not_to be_included } end @@ -130,12 +130,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do end context 'when keywords and pipeline source policy matches' do - possibilities = [['pushes', 'push'], - ['web', 'web'], - ['triggers', 'trigger'], - ['schedules', 'schedule'], - ['api', 'api'], - ['external', 'external']] + possibilities = [%w[pushes push], + %w[web web], + %w[triggers trigger], + %w[schedules schedule], + %w[api api], + %w[external external]] context 'when using only' do possibilities.each do |keyword, source| @@ -167,12 +167,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do end context 'when keywords and pipeline source does not match' do - possibilities = [['pushes', 'web'], - ['web', 'push'], - ['triggers', 'schedule'], - ['schedules', 'external'], - ['api', 'trigger'], - ['external', 'api']] + possibilities = [%w[pushes web], + %w[web push], + %w[triggers schedule], + %w[schedules external], + %w[api trigger], + %w[external api]] context 'when using only' do possibilities.each do |keyword, source| diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 19dd89f0b6d..ce941197ce1 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -133,6 +133,57 @@ module Gitlab end end + describe '#stages_attributes' do + let(:config) do + YAML.dump( + rspec: { script: 'rspec', stage: 'test', only: ['branches'] }, + prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } + ) + end + + let(:attributes) do + [{ name: "build", + index: 0, + builds: [] }, + { name: "test", + index: 1, + builds: + [{ stage_idx: 1, + stage: "test", + commands: "rspec", + tag_list: [], + name: "rspec", + allow_failure: false, + when: "on_success", + environment: nil, + coverage_regex: nil, + yaml_variables: [], + options: { script: ["rspec"] }, + only: { refs: ["branches"] }, + except: {} }] }, + { name: "deploy", + index: 2, + builds: + [{ stage_idx: 2, + stage: "deploy", + commands: "cap prod", + tag_list: [], + name: "prod", + allow_failure: false, + when: "on_success", + environment: nil, + coverage_regex: nil, + yaml_variables: [], + options: { script: ["cap prod"] }, + only: { refs: ["tags"] }, + except: {} }] }] + end + + it 'returns stages seed attributes' do + expect(subject.stages_attributes).to eq attributes + end + end + describe 'only / except policies validations' do context 'when `only` has an invalid value' do let(:config) { { rspec: { script: "rspec", type: "test", only: only } } } @@ -203,7 +254,7 @@ module Gitlab let(:config_data) { YAML.dump(config) } let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config_data) } - subject { config_processor.stage_attributes('test').first } + subject { config_processor.stage_builds_attributes('test').first } describe "before_script" do context "in global context" do @@ -286,8 +337,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first).to eq({ + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -321,8 +372,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first).to eq({ + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -354,8 +405,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first).to eq({ + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -383,8 +434,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first).to eq({ + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -529,7 +580,7 @@ module Gitlab }) config_processor = Gitlab::Ci::YamlProcessor.new(config) - builds = config_processor.stage_attributes("test") + builds = config_processor.stage_builds_attributes("test") expect(builds.size).to eq(1) expect(builds.first[:when]).to eq(when_state) @@ -561,8 +612,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, key: 'key', @@ -580,8 +631,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, key: 'key', @@ -600,8 +651,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first[:options][:cache]).to eq( paths: ["test/"], untracked: false, key: 'local', @@ -629,8 +680,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.stage_attributes("test").size).to eq(1) - expect(config_processor.stage_attributes("test").first).to eq({ + expect(config_processor.stage_builds_attributes("test").size).to eq(1) + expect(config_processor.stage_builds_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -666,7 +717,7 @@ module Gitlab }) config_processor = Gitlab::Ci::YamlProcessor.new(config) - builds = config_processor.stage_attributes("test") + builds = config_processor.stage_builds_attributes("test") expect(builds.size).to eq(1) expect(builds.first[:options][:artifacts][:when]).to eq(when_state) @@ -682,7 +733,7 @@ module Gitlab end let(:processor) { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) } - let(:builds) { processor.stage_attributes('deploy') } + let(:builds) { processor.stage_builds_attributes('deploy') } context 'when a production environment is specified' do let(:environment) { 'production' } @@ -839,7 +890,7 @@ module Gitlab describe "Hidden jobs" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } - subject { config_processor.stage_attributes("test") } + subject { config_processor.stage_builds_attributes("test") } shared_examples 'hidden_job_handling' do it "doesn't create jobs that start with dot" do @@ -887,7 +938,7 @@ module Gitlab describe "YAML Alias/Anchor" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } - subject { config_processor.stage_attributes("build") } + subject { config_processor.stage_builds_attributes("build") } shared_examples 'job_templates_handling' do it "is correctly supported for jobs" do diff --git a/spec/views/ci/lints/show.html.haml_spec.rb b/spec/views/ci/lints/show.html.haml_spec.rb index 7724d54c569..ded320793ea 100644 --- a/spec/views/ci/lints/show.html.haml_spec.rb +++ b/spec/views/ci/lints/show.html.haml_spec.rb @@ -5,6 +5,7 @@ describe 'ci/lints/show' do describe 'XSS protection' do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(YAML.dump(content)) } + before do assign(:status, true) assign(:builds, config_processor.builds)