diff --git a/app/workers/merge_request_mergeability_check_worker.rb b/app/workers/merge_request_mergeability_check_worker.rb index db1bd0aba2abfe4377099d70140933f5060207ae..d477101a768fb24f10e0e427698ec0db4c76191b 100644 --- a/app/workers/merge_request_mergeability_check_worker.rb +++ b/app/workers/merge_request_mergeability_check_worker.rb @@ -14,7 +14,7 @@ def perform(merge_request_id) merge_request = MergeRequest.find_by_id(merge_request_id) unless merge_request - logger.error("Failed to find merge request with ID: #{merge_request_id}") + Sidekiq.logger.error(worker: self.class.name, message: "Failed to find merge request", merge_request_id: merge_request_id) return end @@ -23,6 +23,6 @@ def perform(merge_request_id) .new(merge_request) .execute(recheck: false, retry_lease: false) - logger.error("Failed to check mergeability of merge request (#{merge_request_id}): #{result.message}") if result.error? + Sidekiq.logger.error(worker: self.class.name, message: "Failed to check mergeability of merge request: #{result.message}", merge_request_id: merge_request_id) if result.error? end end diff --git a/spec/workers/merge_request_mergeability_check_worker_spec.rb b/spec/workers/merge_request_mergeability_check_worker_spec.rb index 0349de5cbb3cf6dd8373b656576c736420e4cf7a..32debcf96519eae049966ca0530a473451b70e71 100644 --- a/spec/workers/merge_request_mergeability_check_worker_spec.rb +++ b/spec/workers/merge_request_mergeability_check_worker_spec.rb @@ -10,6 +10,12 @@ it 'does not execute MergeabilityCheckService' do expect(MergeRequests::MergeabilityCheckService).not_to receive(:new) + expect(Sidekiq.logger).to receive(:error).once + .with( + merge_request_id: 1, + worker: "MergeRequestMergeabilityCheckWorker", + message: 'Failed to find merge request') + subject.perform(1) end end @@ -24,6 +30,20 @@ subject.perform(merge_request.id) end + + it 'structurally logs a failed mergeability check' do + expect_next_instance_of(MergeRequests::MergeabilityCheckService, merge_request) do |service| + expect(service).to receive(:execute).and_return(double(error?: true, message: "solar flares")) + end + + expect(Sidekiq.logger).to receive(:error).once + .with( + merge_request_id: merge_request.id, + worker: "MergeRequestMergeabilityCheckWorker", + message: 'Failed to check mergeability of merge request: solar flares') + + subject.perform(merge_request.id) + end end it_behaves_like 'an idempotent worker' do