Automatic merge of gitlab-org/gitlab-ce master

.gitlab/ci/frontend.gitlab-ci.yml

@@ -138,9 +138,8 @@ karma:
       - chrome_debug.log
       - coverage-javascript/
       - tmp/tests/frontend/
-# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756
-#    reports:
-#      junit: junit_karma.xml
+    reports:
+      junit: junit_karma.xml
 
 jest:
   extends: .dedicated-no-docs-and-no-qa-pull-cache-job
@@ -163,9 +162,8 @@ jest:
       - coverage-frontend/
       - junit_jest.xml
       - tmp/tests/frontend/
-# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756
-#    reports:
-#      junit: junit_jest.xml
+    reports:
+      junit: junit_jest.xml
   cache:
     key: jest
     paths:

.gitlab/ci/rails.gitlab-ci.yml

@@ -80,9 +80,8 @@
       - rspec_profiling/
       - tmp/capybara/
       - tmp/memory_test/
-# see https://gitlab.com/gitlab-org/gitlab-ce/issues/64756
-#    reports:
-#      junit: junit_rspec.xml
+    reports:
+      junit: junit_rspec.xml
 
 .rspec-metadata-pg: &rspec-metadata-pg
   <<: *rspec-metadata

app/mailers/notify.rb

@@ -78,17 +78,7 @@ class Notify < BaseMailer
   #
   # Returns a String containing the User's email address.
   def recipient(recipient_id, notification_group = nil)
-    @current_user = User.find(recipient_id)
-    group_notification_email = nil
-
-    if notification_group
-      notification_settings = notification_group.notification_settings_for(@current_user, hierarchy_order: :asc)
-      group_notification_email = notification_settings.find { |n| n.notification_email.present? }&.notification_email
-    end
-
-    # Return group-specific email address if present, otherwise return global
-    # email address
-    group_notification_email || @current_user.notification_email
+    User.find(recipient_id).notification_email_for(notification_group)
   end
 
   # Formats arguments into a String suitable for use as an email subject

app/models/ci/build.rb

@@ -531,6 +531,14 @@ module Ci
       trace.exist?
     end
 
+    def has_live_trace?
+      trace.live_trace_exist?
+    end
+
+    def has_archived_trace?
+      trace.archived_trace_exist?
+    end
+
     def artifacts_file
       job_artifacts_archive&.file
     end

app/models/ci/job_artifact.rb

@@ -176,6 +176,10 @@ module Ci
       end
     end
 
+    def self.archived_trace_exists_for?(job_id)
+      where(job_id: job_id).trace.take&.file&.file&.exists?
+    end
+
     private
 
     def file_format_adapter_class

app/models/ci/runner.rb

@@ -264,7 +264,7 @@ module Ci
     private
 
     def cleanup_runner_queue
-      Gitlab::Redis::Queues.with do |redis|
+      Gitlab::Redis::SharedState.with do |redis|
         redis.del(runner_queue_key)
       end
     end

app/models/group.rb

@@ -144,6 +144,12 @@ class Group < Namespace
     notification_settings(hierarchy_order: hierarchy_order).where(user: user)
   end
 
+  def notification_email_for(user)
+    # Finds the closest notification_setting with a `notification_email`
+    notification_settings = notification_settings_for(user, hierarchy_order: :asc)
+    notification_settings.find { |n| n.notification_email.present? }&.notification_email
+  end
+
   def to_reference(_from = nil, full: nil)
     "#{self.class.reference_prefix}#{full_path}"
   end

app/models/user.rb

@@ -1259,6 +1259,11 @@ class User < ApplicationRecord
     end
   end
 
+  def notification_email_for(notification_group)
+    # Return group-specific email address if present, otherwise return global notification email address
+    notification_group&.notification_email_for(self) || notification_email
+  end
+
   def notification_settings_for(source)
     if notification_settings.loaded?
       notification_settings.find do |notification|

app/presenters/blob_presenter.rb

@@ -4,7 +4,7 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
   presents :blob
 
   def highlight(plain: nil)
-    blob.load_all_data! if blob.respond_to?(:load_all_data!)
+    load_all_blob_data
 
     Gitlab::Highlight.highlight(
       blob.path,
@@ -17,4 +17,10 @@ class BlobPresenter < Gitlab::View::Presenter::Delegated
   def web_url
     Gitlab::Routing.url_helpers.project_blob_url(blob.repository.project, File.join(blob.commit_id, blob.path))
   end
+
+  private
+
+  def load_all_blob_data
+    blob.load_all_data! if blob.respond_to?(:load_all_data!)
+  end
 end

app/presenters/blobs/unfold_presenter.rb

@@ -16,8 +16,12 @@ module Blobs
     attribute :indent, Integer, default: 0
 
     def initialize(blob, params)
+      # Load all blob data first as we need to ensure they're all loaded first
+      # so we can accurately show the rest of the diff when unfolding.
+      load_all_blob_data
+
       @subject = blob
-      @all_lines = highlight.lines
+      @all_lines = blob.data.lines
       super(params)
 
       if full?
@@ -25,10 +29,12 @@ module Blobs
       end
     end
 
-    # Converts a String array to Gitlab::Diff::Line array, with match line added
+    # Returns an array of Gitlab::Diff::Line with match line added
     def diff_lines
-      diff_lines = lines.map do |line|
-        Gitlab::Diff::Line.new(line, nil, nil, nil, nil, rich_text: line)
+      diff_lines = lines.map.with_index do |line, index|
+        full_line = limited_blob_lines[index].delete("\n")
+
+        Gitlab::Diff::Line.new(full_line, nil, nil, nil, nil, rich_text: line)
       end
 
       add_match_line(diff_lines)
@@ -37,11 +43,7 @@ module Blobs
     end
 
     def lines
-      strong_memoize(:lines) do
-        lines = @all_lines
-        lines = lines[since - 1..to - 1] unless full?
-        lines.map(&:html_safe)
-      end
+      @lines ||= limit(highlight.lines).map(&:html_safe)
     end
 
     def match_line_text
@@ -71,5 +73,15 @@ module Blobs
 
       bottom? ? diff_lines.push(match_line) : diff_lines.unshift(match_line)
     end
+
+    def limited_blob_lines
+      @limited_blob_lines ||= limit(@all_lines)
+    end
+
+    def limit(lines)
+      return lines if full?
+
+      lines[since - 1..to - 1]
+    end
   end
 end

app/services/ci/archive_trace_service.rb

@@ -2,8 +2,25 @@
 
 module Ci
   class ArchiveTraceService
-    def execute(job)
+    def execute(job, worker_name:)
+      # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+      # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+      unless job.has_live_trace?
+        Sidekiq.logger.warn(class: worker_name,
+                            message: 'The job does not have live trace but going to be archived.',
+                            job_id: job.id)
+        return
+      end
+
       job.trace.archive!
+
+      # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+      # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+      unless job.has_archived_trace?
+        Sidekiq.logger.warn(class: worker_name,
+                            message: 'The job does not have archived trace after archiving.',
+                            job_id: job.id)
+      end
     rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
       # It's already archived, thus we can safely ignore this exception.
     rescue => e
@@ -11,7 +28,7 @@ module Ci
       # If `archive!` keeps failing for over a week, that could incur data loss.
       # (See more https://docs.gitlab.com/ee/administration/job_traces.html#new-live-trace-architecture)
       # In order to avoid interrupting the system, we do not raise an exception here.
-      archive_error(e, job)
+      archive_error(e, job, worker_name)
     end
 
     private
@@ -22,9 +39,12 @@ module Ci
                                 "Counter of failed attempts of trace archiving")
     end
 
-    def archive_error(error, job)
+    def archive_error(error, job, worker_name)
       failed_archive_counter.increment
-      Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}" # rubocop:disable Gitlab/RailsLogger
+
+      Sidekiq.logger.warn(class: worker_name,
+        message: "Failed to archive trace. message: #{error.message}.",
+        job_id: job.id)
 
       Gitlab::Sentry
         .track_exception(error,

app/services/notification_service.rb

@@ -418,7 +418,9 @@ class NotificationService
       [pipeline.user], :watch,
       custom_action: :"#{pipeline.status}_pipeline",
       target: pipeline
-    ).map(&:notification_email)
+    ).map do |user|
+      user.notification_email_for(pipeline.project.group)
+    end
 
     if recipients.any?
       mailer.public_send(email_template, pipeline, recipients).deliver_later

app/workers/archive_trace_worker.rb

@@ -7,7 +7,7 @@ class ArchiveTraceWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(job_id)
     Ci::Build.without_archived_trace.find_by(id: job_id).try do |job|
-      Ci::ArchiveTraceService.new.execute(job)
+      Ci::ArchiveTraceService.new.execute(job, worker_name: self.class.name)
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/workers/ci/archive_traces_cron_worker.rb

@@ -11,7 +11,7 @@ module Ci
       # This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
       # More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
       Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
-        Ci::ArchiveTraceService.new.execute(build)
+        Ci::ArchiveTraceService.new.execute(build, worker_name: self.class.name)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord

changelogs/unreleased/57953-fix-unfolded-diff-suggestions.yml

+---
+title: Fix suggestion on lines that are not part of an MR
+merge_request: 30606
+author:
+type: fixed

changelogs/unreleased/63485-fix-pipeline-emails-to-use-group-setting.yml

+---
+title: Fix pipeline emails not respecting group notification email setting
+merge_request: 30907
+author:
+type: fixed

changelogs/unreleased/safe-archiving-for-traces.yml

+---
+title: Extra logging for new live trace architecture
+merge_request: 30892
+author:
+type: fixed

changelogs/unreleased/sh-use-shared-state-cluster-pubsub.yml

+---
+title: Use persistent Redis cluster for Workhorse pub/sub notifications
+merge_request: 30990
+author:
+type: fixed

lib/gitlab/ci/trace.rb

@@ -63,7 +63,15 @@ module Gitlab
       end
 
       def exist?
-        trace_artifact&.exists? || job.trace_chunks.any? || current_path.present? || old_trace.present?
+        archived_trace_exist? || live_trace_exist?
+      end
+
+      def archived_trace_exist?
+        trace_artifact&.exists?
+      end
+
+      def live_trace_exist?
+        job.trace_chunks.any? || current_path.present? || old_trace.present?
       end
 
       def read
@@ -167,7 +175,7 @@ module Gitlab
 
       def clone_file!(src_stream, temp_dir)
         FileUtils.mkdir_p(temp_dir)
-        Dir.mktmpdir('tmp-trace', temp_dir) do |dir_path|
+        Dir.mktmpdir("tmp-trace-#{job.id}", temp_dir) do |dir_path|
           temp_path = File.join(dir_path, "job.log")
           FileUtils.touch(temp_path)
           size = IO.copy_stream(src_stream, temp_path)

lib/gitlab/ci/trace/chunked_io.rb

@@ -166,6 +166,13 @@ module Gitlab
         end
 
         def destroy!
+          # TODO: Remove this logging once we confirmed new live trace architecture is functional.
+          # See https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/4667.
+          unless build.has_archived_trace?
+            Sidekiq.logger.warn(message: 'The job does not have archived trace but going to be destroyed.',
+                                job_id: build.id)
+          end
+
           trace_chunks.fast_destroy_all
           @tell = @size = 0
         ensure

lib/gitlab/workhorse.rb

@@ -221,7 +221,7 @@ module Gitlab
       end
 
       def set_key_and_notify(key, value, expire: nil, overwrite: true)
-        Gitlab::Redis::Queues.with do |redis|
+        Gitlab::Redis::SharedState.with do |redis|
           result = redis.set(key, value, ex: expire, nx: !overwrite)
           if result
             redis.publish(NOTIFICATION_CHANNEL, "#{key}=#{value}")

spec/lib/gitlab/ci/trace/chunked_io_spec.rb

@@ -442,5 +442,15 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
 
       expect(Ci::BuildTraceChunk.where(build: build).count).to eq(0)
     end
+
+    context 'when the job does not have archived trace' do
+      it 'leaves a message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          message: 'The job does not have archived trace but going to be destroyed.',
+          job_id: build.id).and_call_original
+
+        subject
+      end
+    end
   end
 end

spec/lib/gitlab/workhorse_spec.rb

@@ -404,6 +404,7 @@ describe Gitlab::Workhorse do
       end
 
       it 'set and notify' do
+        expect(Gitlab::Redis::SharedState).to receive(:with).and_call_original
         expect_any_instance_of(::Redis).to receive(:publish)
           .with(described_class::NOTIFICATION_CHANNEL, "test-key=test-value")
 

spec/models/ci/build_spec.rb

@@ -692,6 +692,34 @@ describe Ci::Build do
     end
   end
 
+  describe '#has_live_trace?' do
+    subject { build.has_live_trace? }
+
+    let(:build) { create(:ci_build, :trace_live) }
+
+    it { is_expected.to be_truthy }
+
+    context 'when build does not have live trace' do
+      let(:build) { create(:ci_build) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#has_archived_trace?' do
+    subject { build.has_archived_trace? }
+
+    let(:build) { create(:ci_build, :trace_artifact) }
+
+    it { is_expected.to be_truthy }
+
+    context 'when build does not have archived trace' do
+      let(:build) { create(:ci_build) }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe '#has_job_artifacts?' do
     subject { build.has_job_artifacts? }
 

spec/models/ci/job_artifact_spec.rb

@@ -70,6 +70,31 @@ describe Ci::JobArtifact do
     end
   end
 
+  describe '.archived_trace_exists_for?' do
+    subject { described_class.archived_trace_exists_for?(job_id) }
+
+    let!(:artifact) { create(:ci_job_artifact, :trace, job: job) }
+    let(:job) { create(:ci_build) }
+
+    context 'when the specified job_id exists' do
+      let(:job_id) { job.id }
+
+      it { is_expected.to be_truthy }
+
+      context 'when the job does have archived trace' do
+        let!(:artifact) { }
+
+        it { is_expected.to be_falsy }
+      end
+    end
+
+    context 'when the specified job_id does not exist' do
+      let(:job_id) { 10000 }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
   describe 'callbacks' do
     subject { create(:ci_job_artifact, :archive) }
 

spec/models/ci/runner_spec.rb

@@ -554,7 +554,7 @@ describe Ci::Runner do
     end
 
     def expect_value_in_queues
-      Gitlab::Redis::Queues.with do |redis|
+      Gitlab::Redis::SharedState.with do |redis|
         runner_queue_key = runner.send(:runner_queue_key)
         expect(redis.get(runner_queue_key))
       end
@@ -627,7 +627,7 @@ describe Ci::Runner do
       end
 
       it 'cleans up the queue' do
-        Gitlab::Redis::Queues.with do |redis|
+        Gitlab::Redis::SharedState.with do |redis|
           expect(redis.get(queue_key)).to be_nil
         end
       end

spec/models/group_spec.rb

@@ -95,6 +95,43 @@ describe Group do
       end
     end
 
+    describe '#notification_email_for' do
+      let(:user) { create(:user) }
+      let(:group) { create(:group) }
+      let(:subgroup) { create(:group, parent: group) }
+
+      let(:group_notification_email) { 'user+group@example.com' }
+      let(:subgroup_notification_email) { 'user+subgroup@example.com' }
+
+      subject { subgroup.notification_email_for(user) }
+
+      context 'when both group notification emails are set' do
+        it 'returns subgroup notification email' do
+          create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
+          create(:notification_setting, user: user, source: subgroup, notification_email: subgroup_notification_email)
+
+          is_expected.to eq(subgroup_notification_email)
+        end
+      end
+
+      context 'when subgroup notification email is blank' do
+        it 'returns parent group notification email' do
+          create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
+          create(:notification_setting, user: user, source: subgroup, notification_email: '')
+
+          is_expected.to eq(group_notification_email)
+        end
+      end
+
+      context 'when only the parent group notification email is set' do
+        it 'returns parent group notification email' do
+          create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
+
+          is_expected.to eq(group_notification_email)
+        end
+      end
+    end
+
     describe '#visibility_level_allowed_by_parent' do
       let(:parent) { create(:group, :internal) }
       let(:sub_group) { build(:group, parent_id: parent.id) }

spec/models/user_spec.rb

@@ -3504,4 +3504,37 @@ describe User do
       expect(described_class.reorder_by_name).to eq([user1, user2])
     end
   end
+
+  describe '#notification_email_for' do
+    let(:user) { create(:user) }
+    let(:group) { create(:group) }
+
+    subject { user.notification_email_for(group) }
+
+    context 'when group is nil' do
+      let(:group) { nil }
+
+      it 'returns global notification email' do
+        is_expected.to eq(user.notification_email)
+      end
+    end
+
+    context 'when group has no notification email set' do
+      it 'returns global notification email' do
+        create(:notification_setting, user: user, source: group, notification_email: '')
+
+        is_expected.to eq(user.notification_email)
+      end
+    end
+
+    context 'when group has notification email set' do
+      it 'returns group notification email' do
+        group_notification_email = 'user+group@example.com'
+
+        create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
+
+        is_expected.to eq(group_notification_email)
+      end
+    end
+  end
 end

spec/presenters/blobs/unfold_presenter_spec.rb

@@ -54,8 +54,10 @@ describe Blobs::UnfoldPresenter do
         expect(lines.size).to eq(total_lines)
 
         lines.each.with_index do |line, index|
-          expect(line.text).to include("LC#{index + 1}")
-          expect(line.text).to eq(line.rich_text)
+          line_number = index + 1
+
+          expect(line.text).to eq(line_number.to_s)
+          expect(line.rich_text).to include("LC#{line_number}")
           expect(line.type).to be_nil
         end
       end

spec/services/ci/archive_trace_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe Ci::ArchiveTraceService, '#execute' do
-  subject { described_class.new.execute(job) }
+  subject { described_class.new.execute(job, worker_name: ArchiveTraceWorker.name) }
 
   context 'when job is finished' do
     let(:job) { create(:ci_build, :success, :trace_live) }
@@ -25,6 +25,34 @@ describe Ci::ArchiveTraceService, '#execute' do
         expect { subject }.not_to change { Ci::JobArtifact.trace.count }
       end
     end
+
+    context 'when job does not have trace' do
+      let(:job) { create(:ci_build, :success) }
+
+      it 'leaves a warning message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: ArchiveTraceWorker.name,
+          message: 'The job does not have live trace but going to be archived.',
+          job_id: job.id)
+
+        subject
+      end
+    end
+
+    context 'when job failed to archive trace but did not raise an exception' do
+      before do
+        allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!) {}
+      end
+
+      it 'leaves a warning message in sidekiq log' do
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: ArchiveTraceWorker.name,
+          message: 'The job does not have archived trace after archiving.',
+          job_id: job.id)
+
+        subject
+      end
+    end
   end
 
   context 'when job is running' do
@@ -37,10 +65,10 @@ describe Ci::ArchiveTraceService, '#execute' do
               issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
               extra: { job_id: job.id } ).once
 
-      expect(Rails.logger)
-        .to receive(:error)
-        .with("Failed to archive trace. id: #{job.id} message: Job is not finished yet")
-        .and_call_original
+      expect(Sidekiq.logger).to receive(:warn).with(
+        class: ArchiveTraceWorker.name,
+        message: "Failed to archive trace. message: Job is not finished yet.",
+        job_id: job.id).and_call_original
 
       expect(Gitlab::Metrics)
         .to receive(:counter)

spec/services/notification_service_spec.rb

@@ -2063,27 +2063,59 @@ describe NotificationService, :mailer do
         end
 
         context 'when the creator has custom notifications enabled' do
-          before do
-            pipeline = create_pipeline(u_custom_notification_enabled, :success)
-            notification.pipeline_finished(pipeline)
-          end
+          let(:pipeline) { create_pipeline(u_custom_notification_enabled, :success) }
 
           it 'emails only the creator' do
+            notification.pipeline_finished(pipeline)
+
             should_only_email(u_custom_notification_enabled, kind: :bcc)
           end
+
+          context 'when the creator has group notification email set' do
+            let(:group_notification_email) { 'user+group@example.com' }
+
+            before do
+              group = create(:group)
+
+              project.update(group: group)
+              create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email)
+            end
+
+            it 'sends to group notification email' do
+              notification.pipeline_finished(pipeline)
+
+              expect(email_recipients(kind: :bcc).first).to eq(group_notification_email)
+            end
+          end
         end
       end
 
       context 'with a failed pipeline' do
         context 'when the creator has no custom notification set' do
-          before do
-            pipeline = create_pipeline(u_member, :failed)
-            notification.pipeline_finished(pipeline)
-          end
+          let(:pipeline) { create_pipeline(u_member, :failed) }
 
           it 'emails only the creator' do
+            notification.pipeline_finished(pipeline)
+
             should_only_email(u_member, kind: :bcc)
           end
+
+          context 'when the creator has group notification email set' do
+            let(:group_notification_email) { 'user+group@example.com' }
+
+            before do
+              group = create(:group)
+
+              project.update(group: group)
+              create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
+            end
+
+            it 'sends to group notification email' do
+              notification.pipeline_finished(pipeline)
+
+              expect(email_recipients(kind: :bcc).first).to eq(group_notification_email)
+            end
+          end
         end
 
         context 'when the creator has watch set' do

spec/support/shared_examples/ci_trace_shared_examples.rb

@@ -720,6 +720,58 @@ shared_examples_for 'trace with enabled live trace feature' do
     end
   end
 
+  describe '#archived_trace_exist?' do
+    subject { trace.archived_trace_exist? }
+
+    context 'when trace does not exist' do
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when archived trace exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when live trace exists' do
+      before do
+        Gitlab::Ci::Trace::ChunkedIO.new(build) do |stream|
+          stream.write('abc')
+        end
+      end
+
+      it { is_expected.to be_falsy }
+    end
+  end
+
+  describe '#live_trace_exist?' do
+    subject { trace.live_trace_exist? }
+
+    context 'when trace does not exist' do
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when archived trace exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when live trace exists' do
+      before do
+        Gitlab::Ci::Trace::ChunkedIO.new(build) do |stream|
+          stream.write('abc')
+        end
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe '#archive!' do
     subject { trace.archive! }
 

spec/support/shared_examples/notify_shared_examples.rb

@@ -42,42 +42,17 @@ shared_examples 'an email sent from GitLab' do
 end
 
 shared_examples 'an email sent to a user' do
-  let(:group_notification_email) { 'user+group@example.com' }
-
   it 'is sent to user\'s global notification email address' do
     expect(subject).to deliver_to(recipient.notification_email)
   end
 
-  context 'that is part of a project\'s group' do
-    it 'is sent to user\'s group notification email address when set' do
-      create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email)
-      expect(subject).to deliver_to(group_notification_email)
-    end
-
-    it 'is sent to user\'s global notification email address when no group email set' do
-      create(:notification_setting, user: recipient, source: project.group, notification_email: '')
-      expect(subject).to deliver_to(recipient.notification_email)
-    end
-  end
-
-  context 'when project is in a sub-group', :nested_groups do
-    before do
-      project.update!(group: subgroup)
-    end
-
-    it 'is sent to user\'s subgroup notification email address when set' do
-      # Set top-level group notification email address to make sure it doesn't get selected
-      create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
-
-      subgroup_notification_email = 'user+subgroup@example.com'
-      create(:notification_setting, user: recipient, source: subgroup, notification_email: subgroup_notification_email)
+  context 'with group notification email' do
+    it 'is sent to user\'s group notification email' do
+      group_notification_email = 'user+group@example.com'
 
-      expect(subject).to deliver_to(subgroup_notification_email)
-    end
+      create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email)
 
-    it 'is sent to user\'s group notification email address when set and subgroup email address not set' do
-      create(:notification_setting, user: recipient, source: subgroup, notification_email: '')
-      expect(subject).to deliver_to(recipient.notification_email)
+      expect(subject).to deliver_to(group_notification_email)
     end
   end
 end

spec/workers/archive_trace_worker_spec.rb

@@ -11,7 +11,7 @@ describe ArchiveTraceWorker do
 
       it 'executes service' do
         expect_any_instance_of(Ci::ArchiveTraceService)
-          .to receive(:execute).with(job)
+          .to receive(:execute).with(job, anything)
 
         subject
       end

spec/workers/ci/archive_traces_cron_worker_spec.rb

@@ -34,7 +34,7 @@ describe Ci::ArchiveTracesCronWorker do
 
     it 'executes service' do
       expect_any_instance_of(Ci::ArchiveTraceService)
-        .to receive(:execute).with(build)
+        .to receive(:execute).with(build, anything)
 
       subject
     end
@@ -60,7 +60,10 @@ describe Ci::ArchiveTracesCronWorker do
       end
 
       it 'puts a log' do
-        expect(Rails.logger).to receive(:error).with("Failed to archive trace. id: #{build.id} message: Unexpected error")
+        expect(Sidekiq.logger).to receive(:warn).with(
+          class: described_class.name,
+          message: "Failed to archive trace. message: Unexpected error.",
+          job_id: build.id)
 
         subject
       end

spec/workers/stuck_ci_jobs_worker_spec.rb

@@ -7,8 +7,6 @@ describe StuckCiJobsWorker do
 
   let!(:runner) { create :ci_runner }
   let!(:job) { create :ci_build, runner: runner }
-  let(:trace_lease_key) { "trace:write:lock:#{job.id}" }
-  let(:trace_lease_uuid) { SecureRandom.uuid }
   let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY }
   let(:worker_lease_uuid) { SecureRandom.uuid }
 
@@ -16,7 +14,6 @@ describe StuckCiJobsWorker do
 
   before do
     stub_exclusive_lease(worker_lease_key, worker_lease_uuid)
-    stub_exclusive_lease(trace_lease_key, trace_lease_uuid)
     job.update!(status: status, updated_at: updated_at)
   end
 
@@ -195,7 +192,6 @@ describe StuckCiJobsWorker do
     end
 
     it 'cancels exclusive leases after worker perform' do
-      expect_to_cancel_exclusive_lease(trace_lease_key, trace_lease_uuid)
       expect_to_cancel_exclusive_lease(worker_lease_key, worker_lease_uuid)
 
       worker.perform