1996fanrui commented on code in PR #25653:
URL: https://github.com/apache/flink/pull/25653#discussion_r1845409849
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskexecutor/SystemOutRedirectionUtilsTest.java:
##########
@@ -77,26 +76,42 @@ void testDefaultSystemOutAndErr() {
assertThat(errStream.toString()).isEqualTo(logContext);
}
- @ParameterizedTest
- @EnumSource(
- value = SystemOutMode.class,
- names = {"IGNORE", "LOG"})
- void testSystemOutAndErrAreRedirected(SystemOutMode systemOutMode) {
+ @Test
+ void testSystemOutAndErrAreRedirectedToLog() {
+ ByteArrayOutputStream outStream = new ByteArrayOutputStream();
+ ByteArrayOutputStream errStream = new ByteArrayOutputStream();
+ System.setOut(new PrintStream(outStream));
+ System.setErr(new PrintStream(errStream));
+
+ Configuration conf = new Configuration();
+ conf.set(TASK_MANAGER_SYSTEM_OUT_MODE, SystemOutMode.LOG);
+ SystemOutRedirectionUtils.redirectSystemOutAndError(conf);
+
+ String logContext = "This is log context!";
+ System.out.print(logContext);
+
assertThat(outStream.toString()).isEqualTo(SystemOutRedirectionUtils.OUT_TO_LOG_TIPS);
+
+ System.err.print(logContext);
+
assertThat(errStream.toString()).isEqualTo(SystemOutRedirectionUtils.ERR_TO_LOG_TIPS);
+ }
+
+ @Test
+ void testSystemOutAndErrAreRedirectedToIgnore() {
Review Comment:
We don't need 2 test methods here. The `SystemOutMode`, out tips and err
tips could be argument.
You could refer to `DynamicFilteringITCase#testReuseDimSide` test[1], it
uses 2 arguments. And `parameters` method [2]as the argument provider.
[1]
https://github.com/apache/flink/blob/77e7ac076c35d229fab89b8b6ae15138b0e41a22/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/batch/sql/DynamicFilteringITCase.java#L245
[2]
https://github.com/apache/flink/blob/77e7ac076c35d229fab89b8b6ae15138b0e41a22/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/batch/sql/DynamicFilteringITCase.java#L49
##########
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/SystemOutRedirectionUtils.java:
##########
@@ -40,6 +40,18 @@ public class SystemOutRedirectionUtils {
private static final Logger LOG =
LoggerFactory.getLogger(SystemOutRedirectionUtils.class);
+ static final String OUT_TO_LOG_TIPS =
+ "Tips: System.out is redirected to LOG.info as configured."
+ + " View the log file and search
[SystemOutRedirectionUtils] for output.\n";
+ static final String ERR_TO_LOG_TIPS =
+ "Tips: System.err is redirected to LOG.error as configured."
+ + " View the log file and search
[SystemOutRedirectionUtils] for output.\n";
+
+ static final String OUT_IGNORE_TIPS =
+ "Tips: System.out will be directly ignored as configured.\n";
+ static final String ERR_IGNORE_TIPS =
+ "Tips: System.err will be directly ignored as configured.\n";
Review Comment:
IIUC, for testing purposes, we are using the default scope here instead of
private scope for these 4 fields.
For this scenario, it's better to add `@VisibleForTesting`.
--
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]
