Fix specs failures, and use factory with `:ci_job_artifact, :archive`

9 files changed, 94 insertions, 26 deletions

diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
index d54411e198f..15dfb5a5763 100644
--- a/app/uploaders/job_artifact_uploader.rb
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -1,12 +1,12 @@
 class JobArtifactUploader < GitlabUploader
   storage :file
 
-  def self.local_artifacts_store
+  def self.local_store_path
     Gitlab.config.artifacts.path
   end
 
   def self.artifacts_upload_path
-    File.join(self.local_artifacts_store, 'tmp/uploads/')
+    File.join(self.local_store_path, 'tmp/uploads/')
   end
 
   def size
@@ -15,8 +15,24 @@ class JobArtifactUploader < GitlabUploader
     model.size
   end
 
+  def store_dir
+    default_local_path
+  end
+
+  def cache_dir
+    File.join(self.class.local_store_path, 'tmp/cache')
+  end
+
+  def work_dir
+    File.join(self.class.local_store_path, 'tmp/work')
+  end
+
   private
 
+  def default_local_path
+    File.join(self.class.local_store_path, default_path)
+  end
+
   def default_path
     creation_date = model.created_at.utc.strftime('%Y_%m_%d')
 
diff --git a/app/uploaders/legacy_artifact_uploader.rb b/app/uploaders/legacy_artifact_uploader.rb
index 0c23e05b680..4f7f8a63108 100644
--- a/app/uploaders/legacy_artifact_uploader.rb
+++ b/app/uploaders/legacy_artifact_uploader.rb
@@ -6,7 +6,7 @@ class LegacyArtifactUploader < GitlabUploader
   end
 
   def self.artifacts_upload_path
-    File.join(self.local_artifacts_store, 'tmp/uploads/')
+    File.join(self.local_store_path, 'tmp/uploads/')
   end
 
   def store_dir
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 441f740e1e5..c868525cbc0 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -167,8 +167,8 @@ FactoryGirl.define do
 
     trait :artifacts do
       after(:create) do |build|
-        create(:ci_job_artifact, job: build)
-        create(:ci_job_metadata, job: build)
+        create(:ci_job_artifact, :archive, job: build)
+        create(:ci_job_artifact, :metadata, job: build)
         build.reload
       end
     end
diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb
index 0abebd14286..47c9842e698 100644
--- a/spec/factories/ci/job_artifacts.rb
+++ b/spec/factories/ci/job_artifacts.rb
@@ -9,22 +9,24 @@ FactoryGirl.define do
       artifact.project ||= artifact.job.project
     end
 
-    after :create do |artifact|
-      if artifact.archive?
-        artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
-                                            'application/zip')
-        artifact.save
+    trait :archive do
+      after(:create) do |artifact, _|
+        artifact.update!(
+          file_type: :archive,
+          file: fixture_file_upload(
+            Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+        )
       end
     end
-  end
-
-  factory :ci_job_metadata, parent: :ci_job_artifact do
-    file_type :metadata
 
-    after :create do |artifact|
-      artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
-                                            'application/x-gzip')
-      artifact.save
+    trait :metadata do
+      after(:create) do |artifact, _|
+        artifact.update!(
+          file_type: :metadata,
+          file: fixture_file_upload(
+            Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
+        )
+      end
     end
   end
 end
diff --git a/spec/migrations/migrate_old_artifacts_spec.rb b/spec/migrations/migrate_old_artifacts_spec.rb
index b09243e5c95..92eb1d9ce86 100644
--- a/spec/migrations/migrate_old_artifacts_spec.rb
+++ b/spec/migrations/migrate_old_artifacts_spec.rb
@@ -40,7 +40,7 @@ describe MigrateOldArtifacts do
     end
 
     it "legacy artifacts are set" do
-      expect(build_with_legacy_artifacts.artifacts_file_identifier).not_to be_nil
+      expect(build_with_legacy_artifacts.legacy_artifacts_file_identifier).not_to be_nil
     end
 
     describe '#min_id' do
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
index 5202a8183af..9dd5cba150d 100644
--- a/spec/models/ci/job_artifact_spec.rb
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Ci::JobArtifact do
-  set(:artifact) { create(:ci_job_artifact) }
+  set(:artifact) { create(:ci_job_artifact, :archive) }
 
   describe "Associations" do
     it { is_expected.to belong_to(:project) }
diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb
index 95e8d519bdd..4496f91fc5e 100644
--- a/spec/models/project_statistics_spec.rb
+++ b/spec/models/project_statistics_spec.rb
@@ -137,7 +137,7 @@ describe ProjectStatistics do
 
     before do
       create(:ci_build, pipeline: pipeline, artifacts_size: 56.megabytes)
-      create(:ci_job_artifact, project: pipeline.project, job: ci_build)
+      create(:ci_job_artifact, :archive, project: pipeline.project, job: ci_build)
 
       statistics.update_build_artifacts_size
     end
diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb
index d045acf9089..bb2cc52381d 100644
--- a/spec/uploaders/job_artifact_uploader_spec.rb
+++ b/spec/uploaders/job_artifact_uploader_spec.rb
@@ -2,14 +2,46 @@ require 'spec_helper'
 
 describe JobArtifactUploader do
   set(:job_artifact) { create(:ci_job_artifact) }
-  let(:job) { job_artifact.job }
   let(:uploader) { described_class.new(job_artifact, :file) }
+  let(:path) { Gitlab.config.artifacts.path }
 
   describe '#store_dir' do
     subject { uploader.store_dir }
 
-    it { is_expected.to start_with(Gitlab.config.artifacts.path) }
-    it { is_expected.not_to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") }
+    it { is_expected.to start_with(path) }
+    it { is_expected.not_to end_with("#{job_artifact.project_id}/#{job_artifact.created_at.utc.strftime('%Y_%m')}/#{job_artifact.id}") }
     it { is_expected.to match(/\h{2}\/\h{2}\/\h{64}\/\d{4}_\d{1,2}_\d{1,2}\/\d+\/\d+\z/) }
   end
+
+  describe '#cache_dir' do
+    subject { uploader.cache_dir }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('/tmp/cache') }
+  end
+
+  describe '#work_dir' do
+    subject { uploader.work_dir }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('/tmp/work') }
+  end
+
+  context 'file is stored in valid path' do
+    let(:file) do
+      fixture_file_upload(Rails.root.join(
+        'spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+    end
+
+    before do
+      uploader.store!(file)
+    end
+
+    subject { uploader.file.path }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to include("/#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/") }
+    it { is_expected.to include("/#{job_artifact.project_id.to_s}/") }
+    it { is_expected.to end_with("ci_build_artifacts.zip") }
+  end
 end
diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb
index 1d9adaccd8d..203630de91c 100644
--- a/spec/uploaders/legacy_artifact_uploader_spec.rb
+++ b/spec/uploaders/legacy_artifact_uploader_spec.rb
@@ -5,8 +5,8 @@ describe LegacyArtifactUploader do
   let(:uploader) { described_class.new(job, :artifacts_file) }
   let(:path) { Gitlab.config.artifacts.path }
 
-  describe '.local_artifacts_store' do
-    subject { described_class.local_artifacts_store }
+  describe '.local_store_path' do
+    subject { described_class.local_store_path }
 
     it "delegate to artifacts path" do
       expect(Gitlab.config.artifacts).to receive(:path)
@@ -58,4 +58,22 @@ describe LegacyArtifactUploader do
       it { is_expected.not_to be_nil }
     end
   end
+
+  context 'file is stored in valid path' do
+    let(:file) do
+      fixture_file_upload(Rails.root.join(
+        'spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+    end
+
+    before do
+      uploader.store!(file)
+    end
+
+    subject { uploader.file.path }
+
+    it { is_expected.to start_with(path) }
+    it { is_expected.to include("/#{job.created_at.utc.strftime('%Y_%m')}/") }
+    it { is_expected.to include("/#{job.project_id.to_s}/") }
+    it { is_expected.to end_with("ci_build_artifacts.zip") }
+  end
 end