chia7712 commented on a change in pull request #10976:
URL: https://github.com/apache/kafka/pull/10976#discussion_r702987181
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderTest.java
##########
@@ -70,99 +63,90 @@
private final Statistics statisticsToAdd2 = mock(Statistics.class);
private final Statistics statisticsToAdd3 = mock(Statistics.class);
- private final Sensor bytesWrittenToDatabaseSensor =
createMock(Sensor.class);
- private final Sensor bytesReadFromDatabaseSensor =
createMock(Sensor.class);
- private final Sensor memtableBytesFlushedSensor = createMock(Sensor.class);
- private final Sensor memtableHitRatioSensor = createMock(Sensor.class);
- private final Sensor writeStallDurationSensor = createMock(Sensor.class);
- private final Sensor blockCacheDataHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheIndexHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheFilterHitRatioSensor =
createMock(Sensor.class);
- private final Sensor bytesReadDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor bytesWrittenDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor numberOfOpenFilesSensor = createMock(Sensor.class);
- private final Sensor numberOfFileErrorsSensor = createMock(Sensor.class);
-
- private final StreamsMetricsImpl streamsMetrics =
niceMock(StreamsMetricsImpl.class);
+ private final Sensor bytesWrittenToDatabaseSensor = mock(Sensor.class);
+ private final Sensor bytesReadFromDatabaseSensor = mock(Sensor.class);
+ private final Sensor memtableBytesFlushedSensor = mock(Sensor.class);
+ private final Sensor memtableHitRatioSensor = mock(Sensor.class);
+ private final Sensor writeStallDurationSensor = mock(Sensor.class);
+ private final Sensor blockCacheDataHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheIndexHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheFilterHitRatioSensor = mock(Sensor.class);
+ private final Sensor bytesReadDuringCompactionSensor = mock(Sensor.class);
+ private final Sensor bytesWrittenDuringCompactionSensor =
mock(Sensor.class);
+ private final Sensor numberOfOpenFilesSensor = mock(Sensor.class);
+ private final Sensor numberOfFileErrorsSensor = mock(Sensor.class);
+
+ private final StreamsMetricsImpl streamsMetrics =
mock(StreamsMetricsImpl.class);
private final RocksDBMetricsRecordingTrigger recordingTrigger =
mock(RocksDBMetricsRecordingTrigger.class);
private final RocksDBMetricsRecorder recorder = new
RocksDBMetricsRecorder(METRICS_SCOPE, STORE_NAME);
+ private MockedStatic<RocksDBMetrics> rocksDBMetricsMockedStatic;
@Before
public void setUp() {
setUpMetricsStubMock();
-
expect(streamsMetrics.rocksDBMetricsRecordingTrigger()).andStubReturn(recordingTrigger);
- replay(streamsMetrics);
+
when(streamsMetrics.rocksDBMetricsRecordingTrigger()).thenReturn(recordingTrigger);
recorder.init(streamsMetrics, TASK_ID1);
}
+ @After
+ public void tearDown() {
+ rocksDBMetricsMockedStatic.close();
+ }
+
@Test
public void shouldInitMetricsRecorder() {
setUpMetricsMock();
recorder.init(streamsMetrics, TASK_ID1);
- verify(RocksDBMetrics.class);
assertThat(recorder.taskId(), is(TASK_ID1));
}
@Test
public void shouldThrowIfMetricRecorderIsReInitialisedWithDifferentTask() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
-
assertThrows(
- IllegalStateException.class,
- () -> recorder.init(streamsMetrics, TASK_ID2)
+ IllegalStateException.class,
Review comment:
There are many unnecessary indentation in this PR. Could you please
remove them?
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderTest.java
##########
@@ -70,99 +63,90 @@
private final Statistics statisticsToAdd2 = mock(Statistics.class);
private final Statistics statisticsToAdd3 = mock(Statistics.class);
- private final Sensor bytesWrittenToDatabaseSensor =
createMock(Sensor.class);
- private final Sensor bytesReadFromDatabaseSensor =
createMock(Sensor.class);
- private final Sensor memtableBytesFlushedSensor = createMock(Sensor.class);
- private final Sensor memtableHitRatioSensor = createMock(Sensor.class);
- private final Sensor writeStallDurationSensor = createMock(Sensor.class);
- private final Sensor blockCacheDataHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheIndexHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheFilterHitRatioSensor =
createMock(Sensor.class);
- private final Sensor bytesReadDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor bytesWrittenDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor numberOfOpenFilesSensor = createMock(Sensor.class);
- private final Sensor numberOfFileErrorsSensor = createMock(Sensor.class);
-
- private final StreamsMetricsImpl streamsMetrics =
niceMock(StreamsMetricsImpl.class);
+ private final Sensor bytesWrittenToDatabaseSensor = mock(Sensor.class);
+ private final Sensor bytesReadFromDatabaseSensor = mock(Sensor.class);
+ private final Sensor memtableBytesFlushedSensor = mock(Sensor.class);
+ private final Sensor memtableHitRatioSensor = mock(Sensor.class);
+ private final Sensor writeStallDurationSensor = mock(Sensor.class);
+ private final Sensor blockCacheDataHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheIndexHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheFilterHitRatioSensor = mock(Sensor.class);
+ private final Sensor bytesReadDuringCompactionSensor = mock(Sensor.class);
+ private final Sensor bytesWrittenDuringCompactionSensor =
mock(Sensor.class);
+ private final Sensor numberOfOpenFilesSensor = mock(Sensor.class);
+ private final Sensor numberOfFileErrorsSensor = mock(Sensor.class);
+
+ private final StreamsMetricsImpl streamsMetrics =
mock(StreamsMetricsImpl.class);
private final RocksDBMetricsRecordingTrigger recordingTrigger =
mock(RocksDBMetricsRecordingTrigger.class);
private final RocksDBMetricsRecorder recorder = new
RocksDBMetricsRecorder(METRICS_SCOPE, STORE_NAME);
+ private MockedStatic<RocksDBMetrics> rocksDBMetricsMockedStatic;
@Before
public void setUp() {
setUpMetricsStubMock();
-
expect(streamsMetrics.rocksDBMetricsRecordingTrigger()).andStubReturn(recordingTrigger);
- replay(streamsMetrics);
+
when(streamsMetrics.rocksDBMetricsRecordingTrigger()).thenReturn(recordingTrigger);
recorder.init(streamsMetrics, TASK_ID1);
}
+ @After
+ public void tearDown() {
+ rocksDBMetricsMockedStatic.close();
+ }
+
@Test
public void shouldInitMetricsRecorder() {
setUpMetricsMock();
Review comment:
There is another setup-related method `setUpMetricsStubMock`. Could we
merge them?
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderTest.java
##########
@@ -70,99 +63,90 @@
private final Statistics statisticsToAdd2 = mock(Statistics.class);
private final Statistics statisticsToAdd3 = mock(Statistics.class);
- private final Sensor bytesWrittenToDatabaseSensor =
createMock(Sensor.class);
- private final Sensor bytesReadFromDatabaseSensor =
createMock(Sensor.class);
- private final Sensor memtableBytesFlushedSensor = createMock(Sensor.class);
- private final Sensor memtableHitRatioSensor = createMock(Sensor.class);
- private final Sensor writeStallDurationSensor = createMock(Sensor.class);
- private final Sensor blockCacheDataHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheIndexHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheFilterHitRatioSensor =
createMock(Sensor.class);
- private final Sensor bytesReadDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor bytesWrittenDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor numberOfOpenFilesSensor = createMock(Sensor.class);
- private final Sensor numberOfFileErrorsSensor = createMock(Sensor.class);
-
- private final StreamsMetricsImpl streamsMetrics =
niceMock(StreamsMetricsImpl.class);
+ private final Sensor bytesWrittenToDatabaseSensor = mock(Sensor.class);
+ private final Sensor bytesReadFromDatabaseSensor = mock(Sensor.class);
+ private final Sensor memtableBytesFlushedSensor = mock(Sensor.class);
+ private final Sensor memtableHitRatioSensor = mock(Sensor.class);
+ private final Sensor writeStallDurationSensor = mock(Sensor.class);
+ private final Sensor blockCacheDataHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheIndexHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheFilterHitRatioSensor = mock(Sensor.class);
+ private final Sensor bytesReadDuringCompactionSensor = mock(Sensor.class);
+ private final Sensor bytesWrittenDuringCompactionSensor =
mock(Sensor.class);
+ private final Sensor numberOfOpenFilesSensor = mock(Sensor.class);
+ private final Sensor numberOfFileErrorsSensor = mock(Sensor.class);
+
+ private final StreamsMetricsImpl streamsMetrics =
mock(StreamsMetricsImpl.class);
private final RocksDBMetricsRecordingTrigger recordingTrigger =
mock(RocksDBMetricsRecordingTrigger.class);
private final RocksDBMetricsRecorder recorder = new
RocksDBMetricsRecorder(METRICS_SCOPE, STORE_NAME);
+ private MockedStatic<RocksDBMetrics> rocksDBMetricsMockedStatic;
@Before
public void setUp() {
setUpMetricsStubMock();
-
expect(streamsMetrics.rocksDBMetricsRecordingTrigger()).andStubReturn(recordingTrigger);
- replay(streamsMetrics);
+
when(streamsMetrics.rocksDBMetricsRecordingTrigger()).thenReturn(recordingTrigger);
recorder.init(streamsMetrics, TASK_ID1);
}
+ @After
+ public void tearDown() {
+ rocksDBMetricsMockedStatic.close();
+ }
+
@Test
public void shouldInitMetricsRecorder() {
setUpMetricsMock();
recorder.init(streamsMetrics, TASK_ID1);
- verify(RocksDBMetrics.class);
assertThat(recorder.taskId(), is(TASK_ID1));
}
@Test
public void shouldThrowIfMetricRecorderIsReInitialisedWithDifferentTask() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
-
assertThrows(
- IllegalStateException.class,
- () -> recorder.init(streamsMetrics, TASK_ID2)
+ IllegalStateException.class,
+ () -> recorder.init(streamsMetrics, TASK_ID2)
);
}
@Test
public void
shouldThrowIfMetricRecorderIsReInitialisedWithDifferentStreamsMetrics() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
Review comment:
why removing this line? It is intentional to initialize the recorder
with different metrics.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderTest.java
##########
@@ -70,99 +63,90 @@
private final Statistics statisticsToAdd2 = mock(Statistics.class);
private final Statistics statisticsToAdd3 = mock(Statistics.class);
- private final Sensor bytesWrittenToDatabaseSensor =
createMock(Sensor.class);
- private final Sensor bytesReadFromDatabaseSensor =
createMock(Sensor.class);
- private final Sensor memtableBytesFlushedSensor = createMock(Sensor.class);
- private final Sensor memtableHitRatioSensor = createMock(Sensor.class);
- private final Sensor writeStallDurationSensor = createMock(Sensor.class);
- private final Sensor blockCacheDataHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheIndexHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheFilterHitRatioSensor =
createMock(Sensor.class);
- private final Sensor bytesReadDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor bytesWrittenDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor numberOfOpenFilesSensor = createMock(Sensor.class);
- private final Sensor numberOfFileErrorsSensor = createMock(Sensor.class);
-
- private final StreamsMetricsImpl streamsMetrics =
niceMock(StreamsMetricsImpl.class);
+ private final Sensor bytesWrittenToDatabaseSensor = mock(Sensor.class);
+ private final Sensor bytesReadFromDatabaseSensor = mock(Sensor.class);
+ private final Sensor memtableBytesFlushedSensor = mock(Sensor.class);
+ private final Sensor memtableHitRatioSensor = mock(Sensor.class);
+ private final Sensor writeStallDurationSensor = mock(Sensor.class);
+ private final Sensor blockCacheDataHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheIndexHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheFilterHitRatioSensor = mock(Sensor.class);
+ private final Sensor bytesReadDuringCompactionSensor = mock(Sensor.class);
+ private final Sensor bytesWrittenDuringCompactionSensor =
mock(Sensor.class);
+ private final Sensor numberOfOpenFilesSensor = mock(Sensor.class);
+ private final Sensor numberOfFileErrorsSensor = mock(Sensor.class);
+
+ private final StreamsMetricsImpl streamsMetrics =
mock(StreamsMetricsImpl.class);
private final RocksDBMetricsRecordingTrigger recordingTrigger =
mock(RocksDBMetricsRecordingTrigger.class);
private final RocksDBMetricsRecorder recorder = new
RocksDBMetricsRecorder(METRICS_SCOPE, STORE_NAME);
+ private MockedStatic<RocksDBMetrics> rocksDBMetricsMockedStatic;
@Before
public void setUp() {
setUpMetricsStubMock();
-
expect(streamsMetrics.rocksDBMetricsRecordingTrigger()).andStubReturn(recordingTrigger);
- replay(streamsMetrics);
+
when(streamsMetrics.rocksDBMetricsRecordingTrigger()).thenReturn(recordingTrigger);
recorder.init(streamsMetrics, TASK_ID1);
}
+ @After
+ public void tearDown() {
+ rocksDBMetricsMockedStatic.close();
+ }
+
@Test
public void shouldInitMetricsRecorder() {
setUpMetricsMock();
recorder.init(streamsMetrics, TASK_ID1);
- verify(RocksDBMetrics.class);
assertThat(recorder.taskId(), is(TASK_ID1));
}
@Test
public void shouldThrowIfMetricRecorderIsReInitialisedWithDifferentTask() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
-
assertThrows(
- IllegalStateException.class,
- () -> recorder.init(streamsMetrics, TASK_ID2)
+ IllegalStateException.class,
+ () -> recorder.init(streamsMetrics, TASK_ID2)
);
}
@Test
public void
shouldThrowIfMetricRecorderIsReInitialisedWithDifferentStreamsMetrics() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
-
assertThrows(
- IllegalStateException.class,
- () -> recorder.init(
- new StreamsMetricsImpl(new Metrics(), "test-client",
StreamsConfig.METRICS_LATEST, new MockTime()),
- TASK_ID1
- )
+ IllegalStateException.class,
+ () -> recorder.init(
+ new StreamsMetricsImpl(new Metrics(), "test-client",
StreamsConfig.METRICS_LATEST, new MockTime()),
+ TASK_ID1
+ )
);
}
@Test
public void
shouldSetStatsLevelToExceptDetailedTimersWhenValueProvidersWithStatisticsAreAdded()
{
statisticsToAdd1.setStatsLevel(StatsLevel.EXCEPT_DETAILED_TIMERS);
Review comment:
It needs to make sure this code
`statisticsToAdd1.setStatsLevel(StatsLevel.EXCEPT_DETAILED_TIMERS);` is invoked
when executing `recorder.addValueProviders(SEGMENT_STORE_NAME_1, dbToAdd1,
cacheToAdd1, statisticsToAdd1);`. Otherwise, this test is no-op.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderTest.java
##########
@@ -70,99 +63,90 @@
private final Statistics statisticsToAdd2 = mock(Statistics.class);
private final Statistics statisticsToAdd3 = mock(Statistics.class);
- private final Sensor bytesWrittenToDatabaseSensor =
createMock(Sensor.class);
- private final Sensor bytesReadFromDatabaseSensor =
createMock(Sensor.class);
- private final Sensor memtableBytesFlushedSensor = createMock(Sensor.class);
- private final Sensor memtableHitRatioSensor = createMock(Sensor.class);
- private final Sensor writeStallDurationSensor = createMock(Sensor.class);
- private final Sensor blockCacheDataHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheIndexHitRatioSensor =
createMock(Sensor.class);
- private final Sensor blockCacheFilterHitRatioSensor =
createMock(Sensor.class);
- private final Sensor bytesReadDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor bytesWrittenDuringCompactionSensor =
createMock(Sensor.class);
- private final Sensor numberOfOpenFilesSensor = createMock(Sensor.class);
- private final Sensor numberOfFileErrorsSensor = createMock(Sensor.class);
-
- private final StreamsMetricsImpl streamsMetrics =
niceMock(StreamsMetricsImpl.class);
+ private final Sensor bytesWrittenToDatabaseSensor = mock(Sensor.class);
+ private final Sensor bytesReadFromDatabaseSensor = mock(Sensor.class);
+ private final Sensor memtableBytesFlushedSensor = mock(Sensor.class);
+ private final Sensor memtableHitRatioSensor = mock(Sensor.class);
+ private final Sensor writeStallDurationSensor = mock(Sensor.class);
+ private final Sensor blockCacheDataHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheIndexHitRatioSensor = mock(Sensor.class);
+ private final Sensor blockCacheFilterHitRatioSensor = mock(Sensor.class);
+ private final Sensor bytesReadDuringCompactionSensor = mock(Sensor.class);
+ private final Sensor bytesWrittenDuringCompactionSensor =
mock(Sensor.class);
+ private final Sensor numberOfOpenFilesSensor = mock(Sensor.class);
+ private final Sensor numberOfFileErrorsSensor = mock(Sensor.class);
+
+ private final StreamsMetricsImpl streamsMetrics =
mock(StreamsMetricsImpl.class);
private final RocksDBMetricsRecordingTrigger recordingTrigger =
mock(RocksDBMetricsRecordingTrigger.class);
private final RocksDBMetricsRecorder recorder = new
RocksDBMetricsRecorder(METRICS_SCOPE, STORE_NAME);
+ private MockedStatic<RocksDBMetrics> rocksDBMetricsMockedStatic;
@Before
public void setUp() {
setUpMetricsStubMock();
-
expect(streamsMetrics.rocksDBMetricsRecordingTrigger()).andStubReturn(recordingTrigger);
- replay(streamsMetrics);
+
when(streamsMetrics.rocksDBMetricsRecordingTrigger()).thenReturn(recordingTrigger);
recorder.init(streamsMetrics, TASK_ID1);
}
+ @After
+ public void tearDown() {
+ rocksDBMetricsMockedStatic.close();
+ }
+
@Test
public void shouldInitMetricsRecorder() {
setUpMetricsMock();
recorder.init(streamsMetrics, TASK_ID1);
- verify(RocksDBMetrics.class);
assertThat(recorder.taskId(), is(TASK_ID1));
}
@Test
public void shouldThrowIfMetricRecorderIsReInitialisedWithDifferentTask() {
- setUpMetricsStubMock();
- recorder.init(streamsMetrics, TASK_ID1);
Review comment:
why removing this line? this test is used to make sure we can't
initialize the `recorder` multiple times with different task id.
--
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
