diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-09-02 16:35:15 +0200 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-04 16:38:17 +0100 |
commit | 0103d5be960e620342c67436ddd64ca9e729d7a8 (patch) | |
tree | b4f2cdd4a5ef8f6c906d71c674cc5f13f791c889 /spec | |
parent | b647ad96f6e7cd1e6ca078635bb1ea49ee7d589f (diff) | |
download | gitlab-ce-0103d5be960e620342c67436ddd64ca9e729d7a8.tar.gz |
Add config_options|variables to BuildMetadatakamil-refactor-ci-builds-v5
These are data columns that store runtime configuration
of build needed to execute it on runner and within pipeline.
The definition of this data is that once used, and when no longer
needed (due to retry capability) they can be freely removed.
They use `jsonb` on PostgreSQL, and `text` on MySQL (due to lacking
support for json datatype on old enough version).
Diffstat (limited to 'spec')
21 files changed, 411 insertions, 212 deletions
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 07c1fc31152..bb3c0d6537d 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -7,7 +7,6 @@ FactoryBot.define do stage_idx 0 ref 'master' tag false - commands 'ls -a' protected false created_at 'Di 29. Okt 09:50:00 CET 2013' pending @@ -15,7 +14,8 @@ FactoryBot.define do options do { image: 'ruby:2.1', - services: ['postgres'] + services: ['postgres'], + script: ['ls -a'] } end @@ -28,7 +28,6 @@ FactoryBot.define do pipeline factory: :ci_pipeline trait :degenerated do - commands nil options nil yaml_variables nil end @@ -95,33 +94,53 @@ FactoryBot.define do trait :teardown_environment do environment 'staging' - options environment: { name: 'staging', - action: 'stop', - url: 'http://staging.example.com/$CI_JOB_NAME' } + options do + { + script: %w(ls), + environment: { name: 'staging', + action: 'stop', + url: 'http://staging.example.com/$CI_JOB_NAME' } + } + end end trait :deploy_to_production do environment 'production' - options environment: { name: 'production', - url: 'http://prd.example.com/$CI_JOB_NAME' } + options do + { + script: %w(ls), + environment: { name: 'production', + url: 'http://prd.example.com/$CI_JOB_NAME' } + } + end end trait :start_review_app do environment 'review/$CI_COMMIT_REF_NAME' - options environment: { name: 'review/$CI_COMMIT_REF_NAME', - url: 'http://staging.example.com/$CI_JOB_NAME', - on_stop: 'stop_review_app' } + options do + { + script: %w(ls), + environment: { name: 'review/$CI_COMMIT_REF_NAME', + url: 'http://staging.example.com/$CI_JOB_NAME', + on_stop: 'stop_review_app' } + } + end end trait :stop_review_app do name 'stop_review_app' environment 'review/$CI_COMMIT_REF_NAME' - options environment: { name: 'review/$CI_COMMIT_REF_NAME', - url: 'http://staging.example.com/$CI_JOB_NAME', - action: 'stop' } + options do + { + script: %w(ls), + environment: { name: 'review/$CI_COMMIT_REF_NAME', + url: 'http://staging.example.com/$CI_JOB_NAME', + action: 'stop' } + } + end end trait :allowed_to_fail do @@ -142,7 +161,13 @@ FactoryBot.define do trait :schedulable do self.when 'delayed' - options start_in: '1 minute' + + options do + { + script: ['ls -a'], + start_in: '1 minute' + } + end end trait :actionable do @@ -265,6 +290,7 @@ FactoryBot.define do { image: { name: 'ruby:2.1', entrypoint: '/bin/sh' }, services: ['postgres', { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }], + script: %w(echo), after_script: %w(ls date), artifacts: { name: 'artifacts_file', diff --git a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb index 0272d300e06..0959f1b12f3 100644 --- a/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb +++ b/spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb @@ -5,7 +5,7 @@ describe 'Merge request < User sees mini pipeline graph', :js do let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project, head_pipeline: pipeline) } let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running', sha: project.commit.id) } - let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') } + let(:build) { create(:ci_build, pipeline: pipeline, stage: 'test') } before do build.run diff --git a/spec/features/projects/environments/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb index 89954d35f91..0c517d5f490 100644 --- a/spec/features/projects/environments/environments_spec.rb +++ b/spec/features/projects/environments/environments_spec.rb @@ -272,8 +272,7 @@ describe 'Environments page', :js do create(:ci_build, :scheduled, pipeline: pipeline, name: 'delayed job', - stage: 'test', - commands: 'test') + stage: 'test') end let!(:deployment) do @@ -304,8 +303,7 @@ describe 'Environments page', :js do create(:ci_build, :expired_scheduled, pipeline: pipeline, name: 'delayed job', - stage: 'test', - commands: 'test') + stage: 'test') end it "shows 00:00:00 as the remaining time" do diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index a37ad9c3f43..4706c28bb3d 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -18,7 +18,7 @@ describe 'Pipeline', :js do let!(:build_failed) do create(:ci_build, :failed, - pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') + pipeline: pipeline, stage: 'test', name: 'test') end let!(:build_running) do diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 17772a35779..b75dee66592 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -109,8 +109,7 @@ describe 'Pipelines', :js do context 'when pipeline is cancelable' do let!(:build) do create(:ci_build, pipeline: pipeline, - stage: 'test', - commands: 'test') + stage: 'test') end before do @@ -140,8 +139,7 @@ describe 'Pipelines', :js do context 'when pipeline is retryable' do let!(:build) do create(:ci_build, pipeline: pipeline, - stage: 'test', - commands: 'test') + stage: 'test') end before do @@ -202,8 +200,7 @@ describe 'Pipelines', :js do create(:ci_build, :manual, pipeline: pipeline, name: 'manual build', - stage: 'test', - commands: 'test') + stage: 'test') end before do @@ -237,8 +234,7 @@ describe 'Pipelines', :js do create(:ci_build, :scheduled, pipeline: pipeline, name: 'delayed job', - stage: 'test', - commands: 'test') + stage: 'test') end before do @@ -262,8 +258,7 @@ describe 'Pipelines', :js do create(:ci_build, :expired_scheduled, pipeline: pipeline, name: 'delayed job', - stage: 'test', - commands: 'test') + stage: 'test') end it "shows 00:00:00 as the remaining time" do diff --git a/spec/javascripts/fixtures/jobs.rb b/spec/javascripts/fixtures/jobs.rb index d6b5349594d..433bb690a1c 100644 --- a/spec/javascripts/fixtures/jobs.rb +++ b/spec/javascripts/fixtures/jobs.rb @@ -14,8 +14,7 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do create(:ci_build, :scheduled, pipeline: pipeline, name: 'delayed job', - stage: 'test', - commands: 'test') + stage: 'test') end render_views diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index c7b272cd6ca..6ab126ad39a 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -5,6 +5,11 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do let(:migration) { described_class.new } + before do + # This migration was created before we introduced metadata configs + stub_feature_flags(ci_build_metadata_config: false) + end + let!(:internal_pipeline) { create(:ci_pipeline, source: :web) } let(:pipelines) { [internal_pipeline, unknown_pipeline].map(&:id) } diff --git a/spec/lib/gitlab/ci/build/step_spec.rb b/spec/lib/gitlab/ci/build/step_spec.rb index cce4efaa069..e3136fc925e 100644 --- a/spec/lib/gitlab/ci/build/step_spec.rb +++ b/spec/lib/gitlab/ci/build/step_spec.rb @@ -18,13 +18,6 @@ describe Gitlab::Ci::Build::Step do end end - context 'when commands are specified' do - it_behaves_like 'has correct script' do - let(:job) { create(:ci_build, :no_options, commands: "ls -la\ndate") } - let(:script) { ['ls -la', 'date'] } - end - end - context 'when script option is specified' do it_behaves_like 'has correct script' do let(:job) { create(:ci_build, :no_options, options: { script: ["ls -la\necho aaa", "date"] }) } @@ -62,7 +55,7 @@ describe Gitlab::Ci::Build::Step do end context 'when after_script is not empty' do - let(:job) { create(:ci_build, options: { after_script: ['ls -la', 'date'] }) } + let(:job) { create(:ci_build, options: { script: ['bash'], after_script: ['ls -la', 'date'] }) } it 'fabricates an object' do expect(subject.name).to eq(:after_script) diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index 61d78f86b51..941ef33c8a4 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -153,7 +153,6 @@ describe Gitlab::Ci::Config::Entry::Global do rspec: { name: :rspec, script: %w[rspec ls], before_script: %w(ls pwd), - commands: "ls\npwd\nrspec\nls", image: { name: 'ruby:2.2' }, services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], stage: 'test', @@ -166,7 +165,6 @@ describe Gitlab::Ci::Config::Entry::Global do spinach: { name: :spinach, before_script: [], script: %w[spinach], - commands: 'spinach', image: { name: 'ruby:2.2' }, services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], stage: 'test', diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 8e32cede3b5..3d0b98eb238 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -255,7 +255,6 @@ describe Gitlab::Ci::Config::Entry::Job do .to eq(name: :rspec, before_script: %w[ls pwd], script: %w[rspec], - commands: "ls\npwd\nrspec", stage: 'test', ignore: false, after_script: %w[cleanup], @@ -264,16 +263,6 @@ describe Gitlab::Ci::Config::Entry::Job do end end end - - describe '#commands' do - let(:config) do - { before_script: %w[ls pwd], script: 'rspec' } - end - - it 'returns a string of commands concatenated with new line character' do - expect(entry.commands).to eq "ls\npwd\nrspec" - end - end end describe '#manual_action?' do diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index 1a2c30d3571..d97be76f0e0 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -65,14 +65,12 @@ describe Gitlab::Ci::Config::Entry::Jobs do expect(entry.value).to eq( rspec: { name: :rspec, script: %w[rspec], - commands: 'rspec', ignore: false, stage: 'test', only: { refs: %w[branches tags] }, except: {} }, spinach: { name: :spinach, script: %w[spinach], - commands: 'spinach', ignore: false, stage: 'test', only: { refs: %w[branches tags] }, diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 2cf812b26dc..a700cfd4546 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -6,8 +6,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do let(:attributes) do { name: 'rspec', - ref: 'master', - commands: 'rspec' } + ref: 'master' } end subject do @@ -18,7 +17,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do it 'returns hash attributes of a build' do expect(subject.attributes).to be_a Hash expect(subject.attributes) - .to include(:name, :project, :ref, :commands) + .to include(:name, :project, :ref) end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 441e8214181..63e1f167ce2 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -21,7 +21,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -155,7 +154,6 @@ module Gitlab builds: [{ stage_idx: 1, stage: "test", - commands: "rspec", tag_list: [], name: "rspec", allow_failure: false, @@ -171,7 +169,6 @@ module Gitlab builds: [{ stage_idx: 2, stage: "deploy", - commands: "cap prod", tag_list: [], name: "prod", allow_failure: false, @@ -271,7 +268,7 @@ module Gitlab end it "return commands with scripts concencaced" do - expect(subject[:commands]).to eq("global script\nscript") + expect(subject[:options][:before_script]).to eq(["global script"]) end end @@ -284,7 +281,7 @@ module Gitlab end it "return commands with scripts concencaced" do - expect(subject[:commands]).to eq("local script\nscript") + expect(subject[:options][:before_script]).to eq(["local script"]) end end end @@ -297,7 +294,7 @@ module Gitlab end it "return commands with scripts concencaced" do - expect(subject[:commands]).to eq("script") + expect(subject[:options][:script]).to eq(["script"]) end end @@ -347,7 +344,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -382,7 +378,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -415,7 +410,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -444,7 +438,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -596,7 +589,7 @@ module Gitlab it 'correctly extends rspec job' do expect(config_processor.builds).to be_one - expect(subject.dig(:commands)).to eq 'test' + expect(subject.dig(:options, :script)).to eq %w(test) expect(subject.dig(:options, :image, :name)).to eq 'ruby:alpine' end end @@ -622,7 +615,8 @@ module Gitlab it 'correctly extends rspec job' do expect(config_processor.builds).to be_one - expect(subject.dig(:commands)).to eq "bundle install\nrspec" + expect(subject.dig(:options, :before_script)).to eq ["bundle install"] + expect(subject.dig(:options, :script)).to eq %w(rspec) expect(subject.dig(:options, :image, :name)).to eq 'image:test' expect(subject.dig(:when)).to eq 'always' end @@ -769,7 +763,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "rspec", - commands: "pwd\nrspec", coverage_regex: nil, tag_list: [], options: { @@ -983,7 +976,6 @@ module Gitlab stage: "test", stage_idx: 1, name: "normal_job", - commands: "test", coverage_regex: nil, tag_list: [], options: { @@ -1031,7 +1023,6 @@ module Gitlab stage: "build", stage_idx: 0, name: "job1", - commands: "execute-script-for-job", coverage_regex: nil, tag_list: [], options: { @@ -1046,7 +1037,6 @@ module Gitlab stage: "build", stage_idx: 0, name: "job2", - commands: "execute-script-for-job", coverage_regex: nil, tag_list: [], options: { diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index f5a4b7e2ebf..8f5029b3565 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -197,4 +197,20 @@ describe Gitlab::Utils do end end end + + describe '.deep_indifferent_access' do + let(:hash) do + { "variables" => [{ "key" => "VAR1", "value" => "VALUE2" }] } + end + + subject { described_class.deep_indifferent_access(hash) } + + it 'allows to access hash keys with symbols' do + expect(subject[:variables]).to be_a(Array) + end + + it 'allows to access array keys with symbols' do + expect(subject[:variables].first[:key]).to eq('VAR1') + end + end end diff --git a/spec/lib/serializers/json_spec.rb b/spec/lib/serializers/json_spec.rb new file mode 100644 index 00000000000..5d59d66e8b8 --- /dev/null +++ b/spec/lib/serializers/json_spec.rb @@ -0,0 +1,102 @@ +require 'fast_spec_helper' + +describe Serializers::JSON do + describe '.dump' do + let(:obj) { { key: "value" } } + + subject { described_class.dump(obj) } + + context 'when MySQL is used' do + before do + allow(Gitlab::Database).to receive(:adapter_name) { 'mysql2' } + end + + it 'encodes as string' do + is_expected.to eq('{"key":"value"}') + end + end + + context 'when PostgreSQL is used' do + before do + allow(Gitlab::Database).to receive(:adapter_name) { 'postgresql' } + end + + it 'returns a hash' do + is_expected.to eq(obj) + end + end + end + + describe '.load' do + let(:data_string) { '{"key":"value","variables":[{"key":"VAR1","value":"VALUE1"}]}' } + let(:data_hash) { JSON.parse(data_string) } + + shared_examples 'having consistent accessor' do + it 'allows to access with symbols' do + expect(subject[:key]).to eq('value') + expect(subject[:variables].first[:key]).to eq('VAR1') + end + + it 'allows to access with strings' do + expect(subject["key"]).to eq('value') + expect(subject["variables"].first["key"]).to eq('VAR1') + end + end + + context 'when MySQL is used' do + before do + allow(Gitlab::Database).to receive(:adapter_name) { 'mysql2' } + end + + context 'when loading a string' do + subject { described_class.load(data_string) } + + it 'decodes a string' do + is_expected.to be_a(Hash) + end + + it_behaves_like 'having consistent accessor' + end + + context 'when loading a different type' do + subject { described_class.load({ key: 'hash' }) } + + it 'raises an exception' do + expect { subject }.to raise_error(TypeError) + end + end + + context 'when loading a nil' do + subject { described_class.load(nil) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + + context 'when PostgreSQL is used' do + before do + allow(Gitlab::Database).to receive(:adapter_name) { 'postgresql' } + end + + context 'when loading a hash' do + subject { described_class.load(data_hash) } + + it 'decodes a string' do + is_expected.to be_a(Hash) + end + + it_behaves_like 'having consistent accessor' + end + + context 'when loading a nil' do + subject { described_class.load(nil) } + + it 'returns nil' do + is_expected.to be_nil + end + end + end + end +end diff --git a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb index 8c55daf0d37..51291cb362a 100644 --- a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb +++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb @@ -90,6 +90,13 @@ describe DeleteInconsistentInternalIdRecords, :migration do context 'for ci_pipelines' do let(:scope) { :ci_pipeline } + + let(:create_models) do + create_list(:ci_empty_pipeline, 3, project: project1) + create_list(:ci_empty_pipeline, 3, project: project2) + create_list(:ci_empty_pipeline, 3, project: project3) + end + it_behaves_like 'deleting inconsistent internal_id records' end diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 519968b9e48..016a5899eef 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -13,12 +13,12 @@ describe Ci::BuildMetadata do end let(:build) { create(:ci_build, pipeline: pipeline) } - let(:build_metadata) { build.metadata } + let(:metadata) { build.metadata } it_behaves_like 'having unique enum values' describe '#update_timeout_state' do - subject { build_metadata } + subject { metadata } context 'when runner is not assigned to the job' do it "doesn't change timeout value" do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 7baf4d93804..7e9f62b7419 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1457,8 +1457,24 @@ describe Ci::Build do context 'with retries max config option' do subject { create(:ci_build, options: { retry: { max: 1 } }) } - it 'returns the number of configured max retries' do - expect(subject.retries_max).to eq 1 + context 'when build_metadata_config is set' do + before do + stub_feature_flags(ci_build_metadata_config: true) + end + + it 'returns the number of configured max retries' do + expect(subject.retries_max).to eq 1 + end + end + + context 'when build_metadata_config is not set' do + before do + stub_feature_flags(ci_build_metadata_config: false) + end + + it 'returns the number of configured max retries' do + expect(subject.retries_max).to eq 1 + end end end @@ -1679,14 +1695,49 @@ describe Ci::Build do let(:options) do { image: "ruby:2.1", - services: [ - "postgres" - ] + services: ["postgres"], + script: ["ls -a"] } end it 'contains options' do - expect(build.options).to eq(options) + expect(build.options).to eq(options.stringify_keys) + end + + it 'allows to access with keys' do + expect(build.options[:image]).to eq('ruby:2.1') + end + + it 'allows to access with strings' do + expect(build.options['image']).to eq('ruby:2.1') + end + + context 'when ci_build_metadata_config is set' do + before do + stub_feature_flags(ci_build_metadata_config: true) + end + + it 'persist data in build metadata' do + expect(build.metadata.read_attribute(:config_options)).to eq(options.stringify_keys) + end + + it 'does not persist data in build' do + expect(build.read_attribute(:options)).to be_nil + end + end + + context 'when ci_build_metadata_config is disabled' do + before do + stub_feature_flags(ci_build_metadata_config: false) + end + + it 'persist data in build' do + expect(build.read_attribute(:options)).to eq(options.symbolize_keys) + end + + it 'does not persist data in build metadata' do + expect(build.metadata.read_attribute(:config_options)).to be_nil + end end end @@ -2030,56 +2081,6 @@ describe Ci::Build do end end - describe '#when' do - subject { build.when } - - context 'when `when` is undefined' do - before do - build.when = nil - end - - context 'use from gitlab-ci.yml' do - let(:project) { create(:project, :repository) } - let(:pipeline) { create(:ci_pipeline, project: project) } - - before do - stub_ci_pipeline_yaml_file(config) - end - - context 'when config is not found' do - let(:config) { nil } - - it { is_expected.to eq('on_success') } - end - - context 'when config does not have a questioned job' do - let(:config) do - YAML.dump({ - test_other: { - script: 'Hello World' - } - }) - end - - it { is_expected.to eq('on_success') } - end - - context 'when config has `when`' do - let(:config) do - YAML.dump({ - test: { - script: 'Hello World', - when: 'always' - } - }) - end - - it { is_expected.to eq('always') } - end - end - end - end - describe '#variables' do let(:container_registry_enabled) { false } @@ -2148,62 +2149,6 @@ describe Ci::Build do it { is_expected.to include(*predefined_variables) } - context 'when yaml variables are undefined' do - let(:pipeline) do - create(:ci_pipeline, project: project, - sha: project.commit.id, - ref: project.default_branch) - end - - before do - build.yaml_variables = nil - end - - context 'use from gitlab-ci.yml' do - before do - stub_ci_pipeline_yaml_file(config) - end - - context 'when config is not found' do - let(:config) { nil } - - it { is_expected.to include(*predefined_variables) } - end - - context 'when config does not have a questioned job' do - let(:config) do - YAML.dump({ - test_other: { - script: 'Hello World' - } - }) - end - - it { is_expected.to include(*predefined_variables) } - end - - context 'when config has variables' do - let(:config) do - YAML.dump({ - test: { - script: 'Hello World', - variables: { - KEY: 'value' - } - } - }) - end - - let(:variables) do - [{ key: 'KEY', value: 'value', public: true }] - end - - it { is_expected.to include(*predefined_variables) } - it { is_expected.to include(*variables) } - end - end - end - describe 'variables ordering' do context 'when variables hierarchy is stubbed' do let(:build_pre_var) { { key: 'build', value: 'value', public: true } } @@ -2792,29 +2737,53 @@ describe Ci::Build do end describe '#yaml_variables' do - before do - build.update_attribute(:yaml_variables, variables) + let(:build) { create(:ci_build, pipeline: pipeline, yaml_variables: variables) } + + let(:variables) do + [ + { 'key' => :VARIABLE, 'value' => 'my value' }, + { 'key' => 'VARIABLE2', 'value' => 'my value 2' } + ] end - context 'when serialized valu is a symbolized hash' do - let(:variables) do - [{ key: :VARIABLE, value: 'my value 1' }] + shared_examples 'having consistent representation' do + it 'allows to access using symbols' do + expect(build.reload.yaml_variables.first[:key]).to eq('VARIABLE') + expect(build.reload.yaml_variables.first[:value]).to eq('my value') + expect(build.reload.yaml_variables.second[:key]).to eq('VARIABLE2') + expect(build.reload.yaml_variables.second[:value]).to eq('my value 2') end + end + + context 'when ci_build_metadata_config is set' do + before do + stub_feature_flags(ci_build_metadata_config: true) + end + + it_behaves_like 'having consistent representation' - it 'keeps symbolizes keys and stringifies variables names' do - expect(build.yaml_variables) - .to eq [{ key: 'VARIABLE', value: 'my value 1' }] + it 'persist data in build metadata' do + expect(build.metadata.read_attribute(:config_variables)).not_to be_nil + end + + it 'does not persist data in build' do + expect(build.read_attribute(:yaml_variables)).to be_nil end end - context 'when serialized value is a hash with string keys' do - let(:variables) do - [{ 'key' => :VARIABLE, 'value' => 'my value 2' }] + context 'when ci_build_metadata_config is disabled' do + before do + stub_feature_flags(ci_build_metadata_config: false) end - it 'symblizes variables hash' do - expect(build.yaml_variables) - .to eq [{ key: 'VARIABLE', value: 'my value 2' }] + it_behaves_like 'having consistent representation' + + it 'persist data in build' do + expect(build.read_attribute(:yaml_variables)).not_to be_nil + end + + it 'does not persist data in build metadata' do + expect(build.metadata.read_attribute(:config_variables)).to be_nil end end end @@ -2986,7 +2955,7 @@ describe Ci::Build do end context 'when build is configured to be retried' do - subject { create(:ci_build, :running, options: { retry: { max: 3 } }, project: project, user: user) } + subject { create(:ci_build, :running, options: { script: ["ls -al"], retry: 3 }, project: project, user: user) } it 'retries build and assigns the same user to it' do expect(described_class).to receive(:retry) @@ -3475,6 +3444,23 @@ describe Ci::Build do end end + describe 'degenerate!' do + let(:build) { create(:ci_build) } + + subject { build.degenerate! } + + before do + build.ensure_metadata + end + + it 'drops metadata' do + subject + + expect(build.reload).to be_degenerated + expect(build.metadata).to be_nil + end + end + describe '#archived?' do context 'when build is degenerated' do subject { create(:ci_build, :degenerated) } @@ -3502,4 +3488,97 @@ describe Ci::Build do end end end + + describe '#read_metadata_attribute' do + let(:build) { create(:ci_build, :degenerated) } + let(:build_options) { { "key" => "build" } } + let(:metadata_options) { { "key" => "metadata" } } + let(:default_options) { { "key" => "default" } } + + subject { build.send(:read_metadata_attribute, :options, :config_options, default_options) } + + context 'when build and metadata options is set' do + before do + build.write_attribute(:options, build_options) + build.ensure_metadata.write_attribute(:config_options, metadata_options) + end + + it 'prefers build options' do + is_expected.to eq(build_options) + end + end + + context 'when only metadata options is set' do + before do + build.write_attribute(:options, nil) + build.ensure_metadata.write_attribute(:config_options, metadata_options) + end + + it 'returns metadata options' do + is_expected.to eq(metadata_options) + end + end + + context 'when none is set' do + it 'returns default value' do + is_expected.to eq(default_options) + end + end + end + + describe '#write_metadata_attribute' do + let(:build) { create(:ci_build, :degenerated) } + let(:options) { { "key" => "new options" } } + let(:existing_options) { { "key" => "existing options" } } + + subject { build.send(:write_metadata_attribute, :options, :config_options, options) } + + context 'when ci_build_metadata_config is set' do + before do + stub_feature_flags(ci_build_metadata_config: true) + end + + context 'when data in build is already set' do + before do + build.write_attribute(:options, existing_options) + end + + it 'does set metadata options' do + subject + + expect(build.metadata.read_attribute(:config_options)).to eq(options) + end + + it 'does reset build options' do + subject + + expect(build.read_attribute(:options)).to be_nil + end + end + end + + context 'when ci_build_metadata_config is disabled' do + before do + stub_feature_flags(ci_build_metadata_config: false) + end + + context 'when data in build metadata is already set' do + before do + build.ensure_metadata.write_attribute(:config_options, existing_options) + end + + it 'does set metadata options' do + subject + + expect(build.read_attribute(:options)).to eq(options) + end + + it 'does reset build options' do + subject + + expect(build.metadata.read_attribute(:config_options)).to be_nil + end + end + end + end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 2f322cc7054..ec48bf60426 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -287,7 +287,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:job) do create(:ci_build, :artifacts, :extended_options, - pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate") + pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) end describe 'POST /api/v4/jobs/request' do @@ -422,7 +422,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let(:expected_steps) do [{ 'name' => 'script', - 'script' => %w(ls date), + 'script' => %w(echo), 'timeout' => job.metadata_timeout, 'when' => 'on_success', 'allow_failure' => false }, @@ -588,7 +588,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let!(:test_job) do create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy', stage: 'deploy', stage_idx: 1, - options: { dependencies: [job2.name] }) + options: { script: ['bash'], dependencies: [job2.name] }) end before do @@ -612,7 +612,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do let!(:empty_dependencies_job) do create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'empty_dependencies_job', stage: 'deploy', stage_idx: 1, - options: { dependencies: [] }) + options: { script: ['bash'], dependencies: [] }) end before do diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 538992b621e..7ce7d2d882a 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -671,9 +671,9 @@ describe Ci::ProcessPipelineService, '#execute' do context 'when builds with auto-retries are configured' do before do - create_build('build:1', stage_idx: 0, user: user, options: { retry: { max: 2 } }) + create_build('build:1', stage_idx: 0, user: user, options: { script: 'aa', retry: 2 }) create_build('test:1', stage_idx: 1, user: user, when: :on_failure) - create_build('test:2', stage_idx: 1, user: user, options: { retry: { max: 1 } }) + create_build('test:2', stage_idx: 1, user: user, options: { script: 'aa', retry: 1 }) end it 'automatically retries builds in a valid order' do @@ -770,7 +770,7 @@ describe Ci::ProcessPipelineService, '#execute' do end def delayed_options - { when: 'delayed', options: { start_in: '1 minute' } } + { when: 'delayed', options: { script: %w(echo), start_in: '1 minute' } } end def unschedule diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 9d65ac15213..20181387612 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -460,7 +460,12 @@ module Ci end let!(:pre_stage_job) { create(:ci_build, :success, pipeline: pipeline, name: 'test', stage_idx: 0) } - let!(:pending_job) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['test'] } ) } + + let!(:pending_job) do + create(:ci_build, :pending, + pipeline: pipeline, stage_idx: 1, + options: { script: ["bash"], dependencies: ['test'] }) + end subject { execute(specific_runner) } |