Fix spec failures

8 files changed, 27 insertions, 128 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 11028f233c0..221cba8047a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -11,6 +11,8 @@ module Ci
     include IgnorableColumn
     include Gitlab::Utils::StrongMemoize
 
+    BuildArchivedError = Class.new(StandardError)
+
     ignore_column :commands
 
     belongs_to :project, inverse_of: :builds
@@ -34,7 +36,7 @@ module Ci
       has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     end
 
-    has_one :metadata, class_name: 'Ci::BuildMetadata'
+    has_one :metadata, class_name: 'Ci::BuildMetadata', autosave: true
     has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
 
     accepts_nested_attributes_for :runner_session
@@ -154,8 +156,7 @@ module Ci
       end
 
       before_transition any => [:pending, :running] do |build|
-        # forbid transition if job is archived
-        !build.archived?
+        raise BuildArchivedError if build.archived? && Feature.enabled?(:ci_use_build_metadata_for_config)
       end
 
       after_transition any => [:pending] do |build|
@@ -206,7 +207,7 @@ module Ci
     end
 
     def ensure_metadata
-      metadata || build_metadata(project: project)
+      metadata || build_metadata
     end
 
     def archived?
@@ -559,7 +560,7 @@ module Ci
     end
 
     def yaml_variables=(value)
-      if Feature.enabled?(:ci_use_build_metadata_for_config)
+      if Feature.enabled?(:ci_use_build_metadata_for_config) && false
         # save and remove from this model
         ensure_metadata.yaml_variables = value
         write_attribute(:yaml_variables, nil)
diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb
index 631006edcbb..b0ccf11cade 100644
--- a/app/models/ci/build_metadata.rb
+++ b/app/models/ci/build_metadata.rb
@@ -13,6 +13,8 @@ module Ci
     belongs_to :build, class_name: 'Ci::Build'
     belongs_to :project
 
+    before_validation :set_build_project
+
     validates :build, presence: true
     validates :project, presence: true
 
@@ -36,5 +38,11 @@ module Ci
 
       update(timeout: timeout, timeout_source: timeout_source)
     end
+
+    private
+
+    def set_build_project
+      self.project_id ||= self.build&.project_id
+    end
   end
 end
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 18e4a51214a..3ff812c7a92 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -25,7 +25,7 @@ module Ci
       end
 
       if build.archived?
-        raise Gitlab::Access::AccessDeniedError, "job archived"
+        raise Ci::Build::BuildArchivedError
       end
 
       attributes = CLONE_ACCESSORS.map do |attribute|
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index 4e207e4e9e9..de6ce080580 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -195,16 +195,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/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index b37e11ca967..c4efb48ee3e 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -16,7 +16,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/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb
index 6dba132184c..733287471a0 100644
--- a/spec/models/ci/build_metadata_spec.rb
+++ b/spec/models/ci/build_metadata_spec.rb
@@ -13,10 +13,10 @@ describe Ci::BuildMetadata do
   end
 
   let(:build) { create(:ci_build, pipeline: pipeline) }
-  let(:build_metadata) { build.metadata }
+  let(:metadata) { build.metadata }
 
   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 42b627b6823..9f6e5528e95 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1342,9 +1342,8 @@ describe Ci::Build do
     let(:options) do
       {
         image: "ruby:2.1",
-        services: [
-          "postgres"
-        ]
+        services: ["postgres"],
+        script: ["ls -a"]
       }
     end
 
@@ -1625,55 +1624,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(:pipeline) { create(:ci_pipeline) }
-
-        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 }
     let(:predefined_variables) do
@@ -2002,61 +1952,6 @@ describe Ci::Build do
       it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) }
     end
 
-    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
-
     context 'when container registry is enabled' do
       let(:container_registry_enabled) { true }
       let(:ci_registry) do
@@ -2548,7 +2443,7 @@ describe Ci::Build do
     end
 
     context 'when build is configured to be retried' do
-      subject { create(:ci_build, :running, options: { retry: 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)
diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb
index a6565709641..dfe9d17a773 100644
--- a/spec/services/ci/register_job_service_spec.rb
+++ b/spec/services/ci/register_job_service_spec.rb
@@ -458,7 +458,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) }