Merge branch '205597-refactor-analytics-specs' into 'master'

Refactor analytics metric refresh specs

See merge request gitlab-org/gitlab!26585

ee/spec/lib/analytics/refresh_approvals_data_spec.rb

@@ -5,56 +5,11 @@ require 'spec_helper'
 describe Analytics::RefreshApprovalsData do
   subject { described_class.new(merge_request) }
 
-  let!(:approval) { create(:approval) }
-  let(:merge_request) { approval.merge_request }
+  let(:merge_request) { create :merge_request }
 
   describe '#execute' do
-    it 'updates mr first_approved_at metric' do
-      expect do
-        subject.execute
-        merge_request.metrics.reload
-      end.to change { merge_request.metrics.first_approved_at }.from(nil).to(be_like_time(approval.created_at))
-    end
-
-    context 'when first_approved_at is already present' do
-      before do
-        merge_request.metrics.update(first_approved_at: 3.days.ago.beginning_of_day)
-      end
-
-      it 'does not change mr first_approved_at metric' do
-        expect do
-          subject.execute
-          merge_request.metrics.reload
-        end.not_to change { merge_request.metrics.first_approved_at }
-      end
-
-      it 'updates mr first_approved_at metric if forced' do
-        expect do
-          subject.execute(force: true)
-          merge_request.metrics.reload
-        end.to change { merge_request.metrics.first_approved_at }.to(be_like_time(approval.created_at))
-      end
-    end
+    let(:calculated_value) { 2.days.ago.beginning_of_day }
 
-    context 'when no merge request metric is present' do
-      before do
-        merge_request.metrics.destroy
-        merge_request.reload
-      end
-
-      it 'creates one' do
-        expect { subject.execute }
-          .to change { merge_request.metrics&.first_approved_at }.from(nil).to(be_like_time(approval.created_at))
-      end
-    end
-  end
-
-  describe '#execute_async' do
-    it 'schedules async execution' do
-      expect(Analytics::CodeReviewMetricsWorker)
-        .to receive(:perform_async).with(described_class.name, merge_request.id, force: true)
-
-      subject.execute_async(force: true)
-    end
+    include_examples 'common merge request metric refresh for', :first_approved_at
   end
 end

ee/spec/lib/analytics/refresh_comments_data_spec.rb

@@ -3,63 +3,28 @@
 require 'spec_helper'
 
 describe Analytics::RefreshCommentsData do
+  describe '.for_note' do
     subject { described_class.for_note(note) }
 
-  describe '.for_note' do
     context 'for non-commit, non-mr note' do
       let(:note) { create(:note, :on_issue) }
 
       it { is_expected.to be_nil }
     end
-  end
-
-  describe '#execute' do
-    context 'when note is for a merge request' do
-      let(:noteable) { create(:merge_request) }
-      let(:note) { create(:note, :on_merge_request, noteable: noteable, project: noteable.project, author: create(:user)) }
-
-      it 'updates mr first_comment_at metric' do
-        expect do
-          subject.execute
-          noteable.metrics.reload
-        end.to change { noteable.metrics.first_comment_at }.from(nil).to(be_like_time(note.created_at))
-      end
 
-      context 'when no merge request metric is present' do
-        before do
-          noteable.metrics.destroy
-          noteable.reload
-        end
-
-        it 'creates a new metric and updates the first_comment_at' do
-          subject.execute
-
-          expect(noteable.metrics.reload.first_comment_at).to(be_like_time(note.created_at))
-        end
-      end
+    context 'for MR note' do
+      let(:note) { create(:note, :on_merge_request) }
 
-      context 'and first_comment_at is already filled' do
-        before do
-          noteable.metrics.update(first_comment_at: 3.days.ago.beginning_of_day)
-        end
+      it 'returns refresh comments instance for note MR' do
+        expected_mock = instance_double(described_class)
 
-        it 'does not change mr first_comment_at metric' do
-          expect do
-            subject.execute
-            noteable.metrics.reload
-          end.not_to change { noteable.metrics.first_comment_at }
-        end
+        allow(described_class).to receive(:new).with(note.noteable).and_return(expected_mock)
 
-        it 'updates mr first_comment_at metric if forced' do
-          expect do
-            subject.execute(force: true)
-            noteable.metrics.reload
-          end.to change { noteable.metrics.first_comment_at }.to(be_like_time(note.created_at))
-        end
+        expect(subject).to eq expected_mock
       end
     end
 
-    context 'when noteable is a commit' do
+    context 'for commit note' do
       let(:note) { create(:diff_note_on_commit, author: create(:user)) }
       let!(:merge_request) { create :merge_request, source_project: note.project }
 
@@ -67,33 +32,22 @@ describe Analytics::RefreshCommentsData do
         allow(note.noteable).to receive(:merge_requests).and_return(note.project.merge_requests)
       end
 
-      it 'updates mr first_comment_at metric' do
-        expect do
-          subject.execute
-          merge_request.metrics.reload
-        end.to change { merge_request.metrics.first_comment_at }.from(nil).to(be_like_time(note.created_at))
-      end
-
-      context 'and first_comment_at is already filled' do
-        let!(:merge_request) do
-          create :merge_request, :with_productivity_metrics,
-                 source_project: note.project, metrics_data: { first_comment_at: 3.days.ago.beginning_of_day }
-        end
+      it 'returns refresh comments instance for commit MR' do
+        expected_mock = instance_double(described_class)
 
-        it 'does not change mr first_comment_at metric' do
-          expect do
-            subject.execute
-            merge_request.metrics.reload
-          end.not_to change { merge_request.metrics.first_comment_at }
-        end
+        allow(described_class).to receive(:new).with(merge_request).and_return(expected_mock)
 
-        it 'updates mr first_comment_at metric if forced' do
-          expect do
-            subject.execute(force: true)
-            merge_request.metrics.reload
-          end.to change { merge_request.metrics.first_comment_at }.to(be_like_time(note.created_at))
+        expect(subject).to eq expected_mock
       end
     end
   end
+
+  describe '#execute' do
+    subject { described_class.new(merge_request) }
+
+    let(:merge_request) { create :merge_request }
+    let(:calculated_value) { 2.days.ago.beginning_of_day }
+
+    include_examples 'common merge request metric refresh for', :first_comment_at
   end
 end

ee/spec/support/shared_examples/lib/analytics/common_merge_request_metrics_refresh_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'common merge request metric refresh for' do |metric_name|
+  before do
+    allow_next_instance_of(Analytics::MergeRequestMetricsCalculator, merge_request) do |instance|
+      allow(instance).to receive(metric_name).and_return(calculated_value)
+    end
+  end
+
+  it "updates #{metric_name}" do
+    expect do
+      subject.execute
+      merge_request.metrics.reload
+    end.to change { merge_request.metrics[metric_name] }.from(nil).to(calculated_value)
+  end
+
+  context "when #{metric_name} is already present" do
+    before do
+      merge_request.metrics.update(metric_name => calculated_value - 10)
+    end
+
+    it "does not change #{metric_name}" do
+      expect do
+        subject.execute
+        merge_request.metrics.reload
+      end.not_to change { merge_request.metrics[metric_name] }
+    end
+
+    it "updates #{metric_name} if forced" do
+      expect do
+        subject.execute(force: true)
+        merge_request.metrics.reload
+      end.to change { merge_request.metrics[metric_name] }.to(calculated_value)
+    end
+  end
+
+  context 'when no merge request metric is present' do
+    before do
+      merge_request.metrics.destroy
+      merge_request.reload
+    end
+
+    it 'creates one' do
+      expect { subject.execute }
+        .to change { merge_request.metrics && merge_request.metrics[metric_name] }.from(nil).to(calculated_value)
+    end
+  end
+end