rangareddy commented on code in PR #19477:
URL: https://github.com/apache/hudi/pull/19477#discussion_r3703016233
##########
hudi-common/src/test/java/org/apache/hudi/metrics/TestMetricsReporterFactory.java:
##########
@@ -137,22 +139,50 @@ void
metricsReporterFactoryRewritesClassNotFoundIntoAnActionableMessage() {
}
/**
- * The other direction, and the branch most likely to regress: a failure
that is not a missing class must
- * pass through untouched, so an unrelated instantiation error is never
rewritten into "add hudi-aws-bundle".
+ * A missing constructor means hudi-aws resolved but was built against a
different Hudi version, which is
+ * a mismatch rather than something absent. Reflection reports it as the
same opaque HoodieException as a
+ * missing class, and the bare "Unable to instantiate class" that resulted
took a maintainer reading the
+ * buried NoSuchMethodException to explain (#12902).
*/
@Test
- void metricsReporterFactoryLeavesNonClassNotFoundFailuresUntouched() {
+ void metricsReporterFactoryExplainsAConstructorMismatch() {
when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.CLOUDWATCH);
try (MockedStatic<ReflectionUtils> mockedStatic =
Mockito.mockStatic(ReflectionUtils.class)) {
mockedStatic.when(() -> ReflectionUtils.loadClass(
eq(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
any(Class[].class), eq(metricsConfig), eq(registry)))
.thenThrow(new HoodieException("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
new NoSuchMethodException("<init>")));
+ HoodieException exception = assertThrows(HoodieException.class,
+ () -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
+ String message = exception.getMessage();
+ assertTrue(message.contains("constructor"),
+ () -> "The failure should say the constructor did not match, but
was: " + message);
+ assertTrue(message.contains("different Hudi version"),
+ () -> "The failure should name version skew as the likely cause, but
was: " + message);
+ assertFalse(message.contains("was not found on the classpath"),
+ () -> "A class that resolved must not be reported as missing, but
was: " + message);
+ }
+ }
+
+ /**
+ * The other direction, and the branch most likely to regress: a failure
that is neither a missing class
+ * nor a missing constructor must pass through untouched, so an error raised
by the reporter's own
+ * constructor is never rewritten into a classpath diagnosis.
+ */
+ @Test
+ void metricsReporterFactoryLeavesOtherFailuresUntouched() {
+
when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.CLOUDWATCH);
+ try (MockedStatic<ReflectionUtils> mockedStatic =
Mockito.mockStatic(ReflectionUtils.class)) {
+ mockedStatic.when(() -> ReflectionUtils.loadClass(
+ eq(MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS),
any(Class[].class), eq(metricsConfig), eq(registry)))
+ .thenThrow(new HoodieException("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
+ new InvocationTargetException(new IllegalStateException("no AWS
region configured"))));
+
HoodieException exception = assertThrows(HoodieException.class,
() -> MetricsReporterFactory.createReporter(metricsConfig,
registry));
Review Comment:
Fixed — it was a dropped word, exactly as you read it. Now:
```java
assertEquals("Unable to instantiate class " +
MetricsReporterFactory.CLOUDWATCH_REPORTER_CLASS,
exception.getMessage(),
"A failure that is neither a missing class nor a missing constructor
must not be rewritten");
```
`Tests run: 12, Failures: 0, Errors: 0`, checkstyle clean.
--
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]
