Add latest changes from gitlab-org/gitlab@master

3 files changed, 62 insertions, 55 deletions

diff --git a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
index b3db8b56b7c..1edcef2977b 100644
--- a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
+++ b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
   describe '.execute' do
     subject { service.execute }
 
-    let_it_be(:artifact, reload: true) do
+    let_it_be(:artifact, refind: true) do
       create(:ci_job_artifact, expire_at: 1.day.ago)
     end
 
@@ -164,13 +164,21 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
     end
 
     context 'when timeout happens' do
+      let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
+
       before do
-        stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 1.second)
-        allow_any_instance_of(described_class).to receive(:destroy_pipeline_artifacts_batch) { true }
+        stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 0.seconds)
+        stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1)
+
+        second_artifact.job.pipeline.unlocked!
       end
 
-      it 'returns false and does not continue destroying' do
-        is_expected.to be_falsy
+      it 'destroys one artifact' do
+        expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
+      end
+
+      it 'reports the number of destroyed artifacts' do
+        is_expected.to eq(1)
       end
     end
 
@@ -187,6 +195,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
       it 'destroys one artifact' do
         expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
       end
+
+      it 'reports the number of destroyed artifacts' do
+        is_expected.to eq(1)
+      end
     end
 
     context 'when there are no artifacts' do
@@ -197,6 +209,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
       it 'does not raise error' do
         expect { subject }.not_to raise_error
       end
+
+      it 'reports the number of destroyed artifacts' do
+        is_expected.to eq(0)
+      end
     end
 
     context 'when there are artifacts more than batch sizes' do
@@ -211,45 +227,9 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
       it 'destroys all expired artifacts' do
         expect { subject }.to change { Ci::JobArtifact.count }.by(-2)
       end
-    end
-
-    context 'when artifact is a pipeline artifact' do
-      context 'when artifacts are expired' do
-        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! }
-
-          stub_feature_flags(ci_split_pipeline_artifacts_removal: false)
-        end
-
-        it 'destroys pipeline artifacts' do
-          expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2)
-        end
-
-        context 'with ci_split_pipeline_artifacts_removal enabled' do
-          before do
-            stub_feature_flags(ci_split_pipeline_artifacts_removal: true)
-          end
-
-          it 'does not destroy pipeline artifacts' do
-            expect { subject }.not_to change { Ci::PipelineArtifact.count }
-          end
-        end
-      end
-
-      context 'when artifacts are not expired' do
-        let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 2.days.from_now) }
-        let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 2.days.from_now) }
-
-        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
+      it 'reports the number of destroyed artifacts' do
+        is_expected.to eq(2)
       end
     end
 
@@ -265,16 +245,4 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
       end
     end
   end
-
-  describe '.destroy_job_artifacts_batch' do
-    it 'returns a falsy value without artifacts' do
-      expect(service.send(:destroy_job_artifacts_batch)).to be_falsy
-    end
-  end
-
-  describe '.destroy_pipeline_artifacts_batch' do
-    it 'returns a falsy value without artifacts' do
-      expect(service.send(:destroy_pipeline_artifacts_batch)).to be_falsy
-    end
-  end
 end
diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb
index f0291067777..9e084dbed1c 100644
--- a/spec/services/draft_notes/create_service_spec.rb
+++ b/spec/services/draft_notes/create_service_spec.rb
@@ -20,6 +20,23 @@ RSpec.describe DraftNotes::CreateService do
     expect(draft.discussion_id).to be_nil
   end
 
+  it 'tracks the start event when the draft is persisted' do
+    expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+      .to receive(:track_create_review_note_action)
+      .with(user: user)
+
+    draft = create_draft(note: 'This is a test')
+    expect(draft).to be_persisted
+  end
+
+  it 'does not track the start event when the draft is not persisted' do
+    expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+      .not_to receive(:track_create_review_note_action)
+
+    draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
+    expect(draft).not_to be_persisted
+  end
+
   it 'cannot resolve when there is nothing to resolve' do
     draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
 
diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
index ae0c8113904..f83e91b683f 100644
--- a/spec/services/draft_notes/publish_service_spec.rb
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -43,6 +43,13 @@ RSpec.describe DraftNotes::PublishService do
       expect(result[:status]).to eq(:success)
     end
 
+    it 'does not track the publish event' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .not_to receive(:track_publish_review_action)
+
+      publish(draft: drafts.first)
+    end
+
     context 'commit_id is set' do
       let(:commit_id) { commit.id }
 
@@ -74,6 +81,13 @@ RSpec.describe DraftNotes::PublishService do
         expect { publish }.not_to change { DraftNote.count }
       end
 
+      it 'does not track the publish event' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .not_to receive(:track_publish_review_action)
+
+        publish
+      end
+
       it 'returns an error' do
         result = publish
 
@@ -105,6 +119,14 @@ RSpec.describe DraftNotes::PublishService do
       publish
     end
 
+    it 'tracks the publish event' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_publish_review_action)
+        .with(user: user)
+
+      publish
+    end
+
     context 'commit_id is set' do
       let(:commit_id) { commit.id }