From a3467ce9f0b4485a3f82c0ed9bf198575bf288d0 Mon Sep 17 00:00:00 2001
From: Kayla Reopelle <kreopelle@newrelic.com>
Date: Thu, 19 Sep 2024 14:55:24 -0700
Subject: [PATCH] test: Fix intermittent test failures

Some tests in BatchLogRecordProcessor had timing issues due to
the work method. Since we don't rely on that method for these tests,
stub the work operation to do nothing.

Co-authored-by: Tanna McClure <tannalynn@users.noreply.github.com>
---
 .../export/batch_log_record_processor_test.rb | 45 ++++++++++---------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb b/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb
index 64e81477a3..a7ed0c3620 100644
--- a/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb
+++ b/logs_sdk/test/opentelemetry/sdk/logs/export/batch_log_record_processor_test.rb
@@ -188,19 +188,19 @@ def to_log_record_data
     it 'removes the older log records from the batch if full' do
       processor = BatchLogRecordProcessor.new(TestExporter.new, max_queue_size: 1, max_export_batch_size: 1)
 
-      older_log_record = TestLogRecord.new
-      newer_log_record = TestLogRecord.new
-      newest_log_record = TestLogRecord.new
+      # Don't actually try to export, we're looking at the log records array
+      processor.stub(:work, nil) do
+        older_log_record = TestLogRecord.new
+        newest_log_record = TestLogRecord.new
 
-      processor.on_emit(older_log_record, mock_context)
-      processor.on_emit(newer_log_record, mock_context)
-      processor.on_emit(newest_log_record, mock_context)
+        processor.on_emit(older_log_record, mock_context)
+        processor.on_emit(newest_log_record, mock_context)
 
-      records = processor.instance_variable_get(:@log_records)
+        records = processor.instance_variable_get(:@log_records)
 
-      assert_includes(records, newest_log_record)
-      refute_includes(records, newer_log_record)
-      refute_includes(records, older_log_record)
+        assert_includes(records, newest_log_record)
+        refute_includes(records, older_log_record)
+      end
     end
 
     it 'logs a warning if a log record was emitted after the buffer is full' do
@@ -469,18 +469,21 @@ def shutdown(timeout: nil)
     let(:processor) { BatchLogRecordProcessor.new(exporter) }
 
     it 'reports export failures' do
-      mock_logger = Minitest::Mock.new
-      mock_logger.expect(:error, nil, [/Unable to export/])
-      mock_logger.expect(:error, nil, [/Result code: 1/])
-      mock_logger.expect(:error, nil, [/unexpected error in .*\#export_batch/])
-
-      OpenTelemetry.stub(:logger, mock_logger) do
-        log_records = [TestLogRecord.new, TestLogRecord.new, TestLogRecord.new, TestLogRecord.new]
-        log_records.each { |log_record| processor.on_emit(log_record, mock_context) }
-        processor.shutdown
-      end
+      # skip the work method's behavior, we rely on shutdown to get us to the failures
+      processor.stub(:work, nil) do
+        mock_logger = Minitest::Mock.new
+        mock_logger.expect(:error, nil, [/Unable to export/])
+        mock_logger.expect(:error, nil, [/Result code: 1/])
+        mock_logger.expect(:error, nil, [/unexpected error in .*\#export_batch/])
+
+        OpenTelemetry.stub(:logger, mock_logger) do
+          log_records = [TestLogRecord.new, TestLogRecord.new, TestLogRecord.new, TestLogRecord.new]
+          log_records.each { |log_record| processor.on_emit(log_record, mock_context) }
+          processor.shutdown
+        end
 
-      mock_logger.verify
+        mock_logger.verify
+      end
     end
   end