[GitHub] [flink] zentol commented on a diff in pull request #17228: [FLINK-24236] Migrate tests to factory approach

[GitBox <gi...@apache.org>]

zentol commented on code in PR #17228:
URL: https://github.com/apache/flink/pull/17228#discussion_r852908015


##########
flink-runtime/src/test/java/org/apache/flink/runtime/metrics/ReporterSetupTest.java:
##########
@@ -47,17 +50,30 @@
 import static org.junit.Assert.assertTrue;
 
 /** Tests for the {@link ReporterSetup}. */
-public class ReporterSetupTest extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+class ReporterSetupTest {
+
+    @RegisterExtension
+    static final ContextClassLoaderExtension CONTEXT_CLASS_LOADER_EXTENSION =
+            ContextClassLoaderExtension.builder()
+                    .withServiceEntry(
+                            MetricReporterFactory.class,
+                            TestReporter1.class.getName(),
+                            TestReporter2.class.getName(),
+                            TestReporter11.class.getName(),
+                            TestReporter12.class.getName(),
+                            TestReporter13.class.getName())
+                    .build();

Review Comment:
   In order for the service loader to work you need to list the implementations somewhere.
   One option is adding it as service entry file under `src/test/META-INF/resources`, which is what most tests tend to use. The downside of this approach is that it this entry becomes part of the test-jar, and thus those entries can also be picked up in tests of downstream modules. This can lead to surprising behavior (e.g., something being loaded although we didn't expect it) or noisy logging messages (e.g., if you misconfigure a reporter we list all available implementations).
   Essentially, this classloader extension provides better isolation between tests.



-- 
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: issues-unsubscribe@flink.apache.org

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