Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2024-06-02 Thread via GitHub


chia7712 commented on PR #13931:
URL: https://github.com/apache/kafka/pull/13931#issuecomment-2143994107

   @joobisb any updates?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2024-02-15 Thread via GitHub


cadonna commented on code in PR #13931:
URL: https://github.com/apache/kafka/pull/13931#discussion_r1489430066


##
streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java:
##
@@ -138,11 +150,30 @@ public class RecordCollectorTest {
 private StreamsProducer streamsProducer;
 private ProcessorTopology topology;
 private final InternalProcessorContext context = new 
InternalMockProcessorContext<>();
-
 private RecordCollectorImpl collector;
+final Sensor mockSensor = Mockito.mock(Sensor.class);

Review Comment:
   ```suggestion
   private final Sensor mockSensor = Mockito.mock(Sensor.class);
   ```



##
streams/src/test/java/org/apache/kafka/streams/processor/internals/RecordCollectorTest.java:
##
@@ -1279,49 +1268,50 @@ public void 
shouldThrowStreamsExceptionOnSubsequentCloseIfFatalEvenWithContinueE
 
 @Test
 public void 
shouldNotThrowStreamsExceptionOnSubsequentCallIfASendFailsWithContinueExceptionHandler()
 {
-final RecordCollector collector = new RecordCollectorImpl(
-logContext,
-taskId,
-getExceptionalStreamsProducerOnSend(new Exception()),
-new AlwaysContinueProductionExceptionHandler(),
-streamsMetrics,
-topology
-);
+try (final MockedStatic taskMetrics = 
mockStatic(TaskMetrics.class)) {
+final Sensor droppedRecordsSensor = Mockito.mock(Sensor.class);
+when(TaskMetrics.droppedRecordsSensor(
+Mockito.anyString(),
+eq(taskId.toString()),
+eq(mockStreamsMetrics))
+).thenReturn(droppedRecordsSensor);
+
+final RecordCollector collector = new RecordCollectorImpl(
+logContext,
+taskId,
+getExceptionalStreamsProducerOnSend(new Exception()),
+new AlwaysContinueProductionExceptionHandler(),
+mockStreamsMetrics,
+topology
+);
 
-try (final LogCaptureAppender logCaptureAppender =
- 
LogCaptureAppender.createAndRegister(RecordCollectorImpl.class)) {
-logCaptureAppender.setThreshold(Level.INFO);
+try (final LogCaptureAppender logCaptureAppender =
+ 
LogCaptureAppender.createAndRegister(RecordCollectorImpl.class)) {
+logCaptureAppender.setThreshold(Level.INFO);
 
-collector.send(topic, "3", "0", null, null, stringSerializer, 
stringSerializer, null, null, streamPartitioner);
-collector.flush();
+collector.send(topic, "3", "0", null, null, stringSerializer, 
stringSerializer, null, null, streamPartitioner);
+collector.flush();
 
-final List messages = logCaptureAppender.getMessages();
-final StringBuilder errorMessage = new StringBuilder("Messages 
received:");
-for (final String error : messages) {
-errorMessage.append("\n - ").append(error);
+final List messages = logCaptureAppender.getMessages();
+final StringBuilder errorMessage = new StringBuilder("Messages 
received:");
+for (final String error : messages) {
+errorMessage.append("\n - ").append(error);
+}
+assertTrue(
+errorMessage.toString(),
+messages.get(messages.size() - 1)
+.endsWith("Exception handler choose to CONTINUE 
processing in spite of this error but written offsets would not be recorded.")
+);
 }
-assertTrue(
-errorMessage.toString(),
-messages.get(messages.size() - 1)
-.endsWith("Exception handler choose to CONTINUE processing 
in spite of this error but written offsets would not be recorded.")
-);
-}
 
-final Metric metric = streamsMetrics.metrics().get(new MetricName(
-"dropped-records-total",
-"stream-task-metrics",
-"The total number of dropped records",
-mkMap(
-mkEntry("thread-id", Thread.currentThread().getName()),
-mkEntry("task-id", taskId.toString())
-)
-));
-assertEquals(1.0, metric.metricValue());
+Mockito.verify(droppedRecordsSensor, Mockito.times(1)).record();
 
-collector.send(topic, "3", "0", null, null, stringSerializer, 
stringSerializer, null, null, streamPartitioner);
-collector.flush();
-collector.closeClean();
+collector.send(topic, "3", "0", null, null, stringSerializer, 
stringSerializer, null, null, streamPartitioner);
+collector.flush();
+collector.closeClean();
+}
 }
+

Review Comment:
   nit:
   ```suggestion
   ```



##
streams/src/te

Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2024-02-15 Thread via GitHub


joobisb commented on PR #13931:
URL: https://github.com/apache/kafka/pull/13931#issuecomment-1945902179

   > @joobisb Could you please fix the compilation errors?
   
   @cadonna done


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2024-02-14 Thread via GitHub


cadonna commented on PR #13931:
URL: https://github.com/apache/kafka/pull/13931#issuecomment-1943728460

   @joobisb Could you please fix the compilation errors?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2024-02-14 Thread via GitHub


joobisb commented on PR #13931:
URL: https://github.com/apache/kafka/pull/13931#issuecomment-1943338949

   @cadonna i've addressed the changes and fixed merge conflicts, could you 
please take a look
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] KAFKA-8977: Remove MockStreamsMetrics since it is not a mock [kafka]

2023-11-03 Thread via GitHub


github-actions[bot] commented on PR #13931:
URL: https://github.com/apache/kafka/pull/13931#issuecomment-1793324234

   This PR is being marked as stale since it has not had any activity in 90 
days. If you would like to keep this PR alive, please ask a committer for 
review. If the PR has  merge conflicts, please update it with the latest from 
trunk (or appropriate release branch)  If this PR is no longer valid or 
desired, please feel free to close it. If no activity occurs in the next 30 
days, it will be automatically closed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org