Improve artifact locking specs

Improves several specs that touch artifact locking by adding explicit
contexts for the parent pipeline being locked and unlocked

spec/controllers/projects/jobs_controller_spec.rb

@@ -201,33 +201,61 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       end
 
       context 'when job has artifacts' do
-        before do
-          get_show_json
-        end
-
         context 'with not expiry date' do
           let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
 
           it 'exposes needed information' do
+            get_show_json
+
             expect(response).to have_gitlab_http_status(:ok)
             expect(response).to match_response_schema('job/job_details')
             expect(json_response['artifact']['download_path']).to match(%r{artifacts/download})
             expect(json_response['artifact']['browse_path']).to match(%r{artifacts/browse})
+            expect(json_response['artifact']).not_to have_key('keep_path')
             expect(json_response['artifact']).not_to have_key('expired')
             expect(json_response['artifact']).not_to have_key('expired_at')
           end
         end
 
-        context 'with expiry date' do
+        context 'with expired artifacts' do
           let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) }
 
+          context 'when artifacts are unlocked' do
+            before do
+              job.pipeline.unlocked!
+            end
+
             it 'exposes needed information' do
+              get_show_json
+
               expect(response).to have_gitlab_http_status(:ok)
               expect(response).to match_response_schema('job/job_details')
               expect(json_response['artifact']).not_to have_key('download_path')
               expect(json_response['artifact']).not_to have_key('browse_path')
+              expect(json_response['artifact']).not_to have_key('keep_path')
               expect(json_response['artifact']['expired']).to eq(true)
               expect(json_response['artifact']['expire_at']).not_to be_empty
+              expect(json_response['artifact']['locked']).to eq(false)
+            end
+          end
+
+          context 'when artifacts are locked' do
+            before do
+              job.pipeline.artifacts_locked!
+            end
+
+            it 'exposes needed information' do
+              get_show_json
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to match_response_schema('job/job_details')
+              expect(json_response['artifact']).to have_key('download_path')
+              expect(json_response['artifact']).to have_key('browse_path')
+              expect(json_response['artifact']).to have_key('keep_path')
+              expect(json_response['artifact']['expired']).to eq(true)
+              expect(json_response['artifact']['expire_at']).not_to be_empty
+              expect(json_response['artifact']['locked']).to eq(true)
+            end
           end
         end
       end

spec/features/projects/jobs_spec.rb

@@ -373,6 +373,11 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
         let(:expire_at) { Time.now + 7.days }
 
         context 'when user has ability to update job' do
+          context 'when artifacts are unlocked' do
+            before do
+              job.pipeline.unlocked!
+            end
+
             it 'keeps artifacts when keep button is clicked' do
               expect(page).to have_content 'The artifacts will be removed in'
 
@@ -383,6 +388,17 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
             end
           end
 
+          context 'when artifacts are locked' do
+            before do
+              job.pipeline.artifacts_locked!
+            end
+
+            it 'shows the keep button' do
+              expect(page).to have_link 'Keep'
+            end
+          end
+        end
+
         context 'when user does not have ability to update job' do
           let(:user_access_level) { :guest }
 
@@ -395,11 +411,28 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
       context 'when artifacts expired' do
         let(:expire_at) { Time.now - 7.days }
 
+        context 'when artifacts are unlocked' do
+          before do
+            job.pipeline.unlocked!
+          end
+
           it 'does not have the Keep button' do
             expect(page).to have_content 'The artifacts were removed'
             expect(page).not_to have_link 'Keep'
           end
         end
+
+        context 'when artifacts are locked' do
+          before do
+            job.pipeline.artifacts_locked!
+          end
+
+          it 'has the Keep button' do
+            expect(page).not_to have_content 'The artifacts were removed'
+            expect(page).to have_link 'Keep'
+          end
+        end
+      end
     end
 
     context "when visiting old URL" do

spec/models/ci/build_spec.rb

@@ -616,6 +616,10 @@ RSpec.describe Ci::Build do
     subject(:locked_artifacts) { build.locked_artifacts? }
 
     context 'when pipeline is artifacts_locked' do
+      before do
+        build.pipeline.artifacts_locked!
+      end
+
       context 'artifacts archive does not exist' do
         let(:build) { create(:ci_build) }
 

spec/serializers/build_details_entity_spec.rb

@@ -188,25 +188,31 @@ RSpec.describe BuildDetailsEntity do
     context 'when the build has expired artifacts' do
       let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: 7.days.ago) }
 
-      it 'does not expose any artifact actions path' do
-        expect(subject[:artifact].keys).not_to include(:download_path, :browse_path, :keep_path)
+      context 'when pipeline is unlocked' do
+        before do
+          build.pipeline.unlocked!
         end
 
         it 'artifact locked is false' do
           expect(subject.dig(:artifact, :locked)).to eq(false)
         end
 
+        it 'does not expose any artifact actions path' do
+          expect(subject[:artifact].keys).not_to include(:download_path, :browse_path, :keep_path)
+        end
+      end
+
       context 'when the pipeline is artifacts_locked' do
         before do
-          build.pipeline.update!(locked: :artifacts_locked)
+          build.pipeline.artifacts_locked!
         end
 
         it 'artifact locked is true' do
           expect(subject.dig(:artifact, :locked)).to eq(true)
         end
 
-        it 'exposes download and browse artifact actions path' do
-          expect(subject[:artifact].keys).to include(:download_path, :browse_path)
+        it 'exposes download, browse and keep artifact actions path' do
+          expect(subject[:artifact].keys).to include(:download_path, :browse_path, :keep_path)
         end
       end
     end

spec/services/ci/destroy_expired_job_artifacts_service_spec.rb

@@ -11,6 +11,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
     let(:service) { described_class.new }
     let!(:artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
 
+    before do
+      artifact.job.pipeline.unlocked!
+    end
+
     context 'when artifact is expired' do
       context 'when artifact is not locked' do
         before do
@@ -88,6 +92,8 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
       before do
         stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_LIMIT', 1)
         stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1)
+
+        second_artifact.job.pipeline.unlocked!
       end
 
       let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
@@ -102,7 +108,9 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
     end
 
     context 'when there are no artifacts' do
-      let!(:artifact) { }
+      before do
+        artifact.destroy!
+      end
 
       it 'does not raise error' do
         expect { subject }.not_to raise_error
@@ -112,6 +120,8 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
     context 'when there are artifacts more than batch sizes' do
       before do
         stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1)
+
+        second_artifact.job.pipeline.unlocked!
       end
 
       let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
@@ -126,6 +136,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
         let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
         let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
 
+        before do
+          [pipeline_artifact_1, pipeline_artifact_2].each { |pipeline_artifact| pipeline_artifact.pipeline.unlocked! }
+        end
+
         it 'destroys pipeline artifacts' do
           expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2)
         end
@@ -135,10 +149,26 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
         let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 2.days) }
         let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 2.days) }
 
-        it 'do not destroy pipeline artifacts' do
+        before do
+          [pipeline_artifact_1, pipeline_artifact_2].each { |pipeline_artifact| pipeline_artifact.pipeline.unlocked! }
+        end
+
+        it 'does not destroy pipeline artifacts' do
           expect { subject }.not_to change { Ci::PipelineArtifact.count }
         end
       end
     end
+
+    context 'when some artifacts are locked' do
+      before do
+        pipeline = create(:ci_pipeline, locked: :artifacts_locked)
+        job = create(:ci_build, pipeline: pipeline)
+        create(:ci_job_artifact, expire_at: 1.day.ago, job: job)
+      end
+
+      it 'destroys only unlocked artifacts' do
+        expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
+      end
+    end
   end
 end