Fix most test failures

app/models/ci/build.rb

@@ -15,10 +15,9 @@ module Ci
     has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment'
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'
 
-    # TODO: what to do with dependent destroy
-    has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :ci_job_id, dependent: :destroy
-    has_one :job_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :ci_job_id
-    has_one :job_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :ci_job_id
+    has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+    has_one :job_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id
+    has_one :job_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id
 
     # The "environment" field for builds is a String, and is the unexpanded name
     def persisted_environment
@@ -39,7 +38,7 @@ module Ci
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
     scope :with_artifacts, ->() do
-      where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.ci_job_id'))
+      where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id'))
     end
     scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }

app/models/ci/job_artifact.rb

@@ -3,7 +3,7 @@ module Ci
     extend Gitlab::Ci::Model
 
     belongs_to :project
-    belongs_to :job, class_name: "Ci::Build", foreign_key: :ci_job_id
+    belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
 
     before_save :set_size, if: :file_changed?
     after_commit :remove_file!, on: :destroy

app/models/project_statistics.rb

@@ -37,8 +37,7 @@ class ProjectStatistics < ActiveRecord::Base
   def update_build_artifacts_size
     self.build_artifacts_size =
       project.builds.sum(:artifacts_size) +
-      Ci::JobArtifact.artifacts_size_for(self) +
-      Ci::JobArtifactMetadata.artifacts_size_for(self)
+      Ci::JobArtifact.artifacts_size_for(self)
   end
 
   def update_storage_size

app/uploaders/job_artifact_uploader.rb

@@ -4,7 +4,7 @@ class JobArtifactUploader < ArtifactUploader
   end
 
   def size
-    return super unless @artifact.size
+    return super if @artifact.size.nil?
 
     @artifact.size
   end
@@ -12,17 +12,13 @@ class JobArtifactUploader < ArtifactUploader
   private
 
   def disk_hash
-    @disk_hash ||= Digest::SHA2.hexdigest(job.project_id.to_s)
+    @disk_hash ||= Digest::SHA2.hexdigest(@artifact.project_id.to_s)
   end
 
   def default_path
-    creation_date = job.created_at.utc.strftime('%Y_%m_%d')
+    creation_date = @artifact.created_at.utc.strftime('%Y_%m_%d')
 
     File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
-              creation_date, job.id.to_s, @artifact.id.to_s)
-  end
-
-  def job
-    @artifact.job
+              creation_date, @artifact.ci_job_id.to_s, @artifact.id.to_s)
   end
 end

db/migrate/20170918072948_create_job_artifacts.rb

@@ -6,7 +6,7 @@ class CreateJobArtifacts < ActiveRecord::Migration
   def change
     create_table :ci_job_artifacts do |t|
       t.belongs_to :project, null: false, index: true, foreign_key: { on_delete: :cascade }
-      t.integer :ci_job_id, null: false, index: true
+      t.integer :job_id, null: false, index: true
       t.integer :size, limit: 8
       t.integer :file_type, null: false, index: true
 
@@ -15,5 +15,7 @@ class CreateJobArtifacts < ActiveRecord::Migration
 
       t.string :file
     end
+
+    add_foreign_key :ci_job_artifacts, :ci_builds, column: :job_id, on_delete: :cascade
   end
 end

db/schema.rb

@@ -321,7 +321,7 @@ ActiveRecord::Schema.define(version: 20171124150326) do
 
   create_table "ci_job_artifacts", force: :cascade do |t|
     t.integer "project_id", null: false
-    t.integer "ci_job_id", null: false
+    t.integer "job_id", null: false
     t.integer "size", limit: 8
     t.integer "file_type", null: false
     t.datetime_with_timezone "created_at", null: false
@@ -329,8 +329,8 @@ ActiveRecord::Schema.define(version: 20171124150326) do
     t.string "file"
   end
 
-  add_index "ci_job_artifacts", ["ci_job_id"], name: "index_ci_job_artifacts_on_ci_job_id", using: :btree
   add_index "ci_job_artifacts", ["file_type"], name: "index_ci_job_artifacts_on_file_type", using: :btree
+  add_index "ci_job_artifacts", ["job_id"], name: "index_ci_job_artifacts_on_job_id", using: :btree
   add_index "ci_job_artifacts", ["project_id"], name: "index_ci_job_artifacts_on_project_id", using: :btree
 
   create_table "ci_pipeline_schedule_variables", force: :cascade do |t|
@@ -1923,6 +1923,7 @@ ActiveRecord::Schema.define(version: 20171124150326) do
   add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade
   add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade
   add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade
+  add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade
   add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade
   add_foreign_key "ci_pipeline_schedule_variables", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_41c35fda51", on_delete: :cascade
   add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade

spec/factories/ci/builds.rb

@@ -158,6 +158,7 @@ FactoryGirl.define do
       after(:create) do |build|
         create(:ci_job_artifact, job: build)
         create(:ci_job_metadata, job: build)
+        build.reload
       end
     end
 

spec/features/projects/pipelines/pipelines_spec.rb

@@ -304,7 +304,7 @@ describe 'Pipelines', :js do
 
         context 'with artifacts expired' do
           let!(:with_artifacts_expired) do
-            create(:ci_build, :artifacts_expired, :success,
+            create(:ci_build, :expired, :success,
               pipeline: pipeline,
               name: 'rspec',
               stage: 'test')

spec/serializers/pipeline_serializer_spec.rb

@@ -117,7 +117,7 @@ describe PipelineSerializer do
       shared_examples 'no N+1 queries' do
         it 'verifies number of queries', :request_store do
           recorded = ActiveRecord::QueryRecorder.new { subject }
-          expect(recorded.count).to be_within(1).of(57)
+          expect(recorded.count).to be_within(1).of(36)
           expect(recorded.cached_count).to eq(0)
         end
       end

spec/services/ci/retry_build_service_spec.rb

@@ -17,7 +17,7 @@ describe Ci::RetryBuildService do
     %i[id status user token coverage trace runner artifacts_expire_at
        artifacts_file artifacts_metadata artifacts_size created_at
        updated_at started_at finished_at queued_at erased_by
-       erased_at auto_canceled_by].freeze
+       erased_at auto_canceled_by job_artifacts job_archive job_metadata].freeze
 
   IGNORE_ACCESSORS =
     %i[type lock_version target_url base_tags trace_sections
@@ -34,7 +34,7 @@ describe Ci::RetryBuildService do
     end
 
     let(:build) do
-      create(:ci_build, :failed, :artifacts_expired, :erased,
+      create(:ci_build, :failed, :artifacts, :expired, :erased,
              :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
              :triggered, :trace, :teardown_environment,
              description: 'my-job', stage: 'test',  pipeline: pipeline,

spec/tasks/gitlab/backup_rake_spec.rb

@@ -24,6 +24,7 @@ describe 'gitlab:app namespace rake task' do
 
     # We need this directory to run `gitlab:backup:create` task
     FileUtils.mkdir_p('public/uploads')
+    FileUtils.mkdir_p(Rails.root.join('tmp/tests/artifacts'))
   end
 
   before do