divijvaidya commented on code in PR #14623:
URL: https://github.com/apache/kafka/pull/14623#discussion_r1372120378
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImplTest.java:
##########
@@ -1293,43 +1204,33 @@ private void verifyMetric(final String name,
public void shouldMeasureLatency() {
final long startTime = 6;
final long endTime = 10;
- final Sensor sensor = createMock(Sensor.class);
- expect(sensor.shouldRecord()).andReturn(true);
- expect(sensor.hasMetrics()).andReturn(true);
- sensor.record(endTime - startTime);
+ final Sensor sensor = mock(Sensor.class);
+ when(sensor.shouldRecord()).thenReturn(true);
+ when(sensor.hasMetrics()).thenReturn(true);
+ doNothing().when(sensor).record(endTime - startTime);
Review Comment:
for my curiosity, does strict subs verify usage of stubs defined using
`doNothin()` as well or just for subs defined using `when()`
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/metrics/StreamsMetricsImplTest.java:
##########
@@ -146,69 +135,26 @@ public class StreamsMetricsImplTest {
private final MockTime time = new MockTime(0);
private final StreamsMetricsImpl streamsMetrics = new
StreamsMetricsImpl(metrics, CLIENT_ID, VERSION, time);
- private static MetricConfig eqMetricConfig(final MetricConfig
metricConfig) {
- EasyMock.reportMatcher(new IArgumentMatcher() {
- private final StringBuffer message = new StringBuffer();
-
- @Override
- public boolean matches(final Object argument) {
- if (argument instanceof MetricConfig) {
- final MetricConfig otherMetricConfig = (MetricConfig)
argument;
- final boolean equalsComparisons =
- (otherMetricConfig.quota() == metricConfig.quota() ||
-
otherMetricConfig.quota().equals(metricConfig.quota())) &&
- otherMetricConfig.tags().equals(metricConfig.tags());
- if (otherMetricConfig.eventWindow() ==
metricConfig.eventWindow() &&
- otherMetricConfig.recordLevel() ==
metricConfig.recordLevel() &&
- equalsComparisons &&
- otherMetricConfig.samples() == metricConfig.samples()
&&
- otherMetricConfig.timeWindowMs() ==
metricConfig.timeWindowMs()) {
-
- return true;
- } else {
- message.append("{ ");
- message.append("eventWindow=");
- message.append(otherMetricConfig.eventWindow());
- message.append(", ");
- message.append("recordLevel=");
- message.append(otherMetricConfig.recordLevel());
- message.append(", ");
- message.append("quota=");
- message.append(otherMetricConfig.quota().toString());
- message.append(", ");
- message.append("samples=");
- message.append(otherMetricConfig.samples());
- message.append(", ");
- message.append("tags=");
- message.append(otherMetricConfig.tags().toString());
- message.append(", ");
- message.append("timeWindowMs=");
- message.append(otherMetricConfig.timeWindowMs());
- message.append(" }");
- }
- }
- message.append("not a MetricConfig object");
- return false;
- }
-
- @Override
- public void appendTo(final StringBuffer buffer) {
- buffer.append(message);
- }
- });
- return null;
- }
-
- private Capture<String> addSensorsOnAllLevels(final Metrics metrics, final
StreamsMetricsImpl streamsMetrics) {
- final Capture<String> sensorKeys = newCapture(CaptureType.ALL);
+ private static boolean eqMetricConfig(final MetricConfig thisMetricConfig,
final MetricConfig thatMetricConfig) {
Review Comment:
This we are not printing details when assertion fails, can you please add a
message to print the thatMetricConfig similar to
`assertTrue(eqMetricConfig(expected, actual), "my message here")`
Will help in debugging failed tests and maintain parity with existing code.
Maybe someone added such detailed logging in one place because debugging this
was very hard!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
