wuchong commented on a change in pull request #15534:
URL: https://github.com/apache/flink/pull/15534#discussion_r612283117
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/SqlClientTest.java
##########
@@ -225,7 +233,7 @@ public void testInitFile() throws IOException {
@Test
public void testExecuteSqlFile() throws IOException {
- List<String> statements = Collections.singletonList("HELP;");
+ List<String> statements = Collections.singletonList("HELP;\nQUIT;\n");
Review comment:
Is the QUIT necessary here?
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/cli/CliClientInputOutputSwitch.java
##########
@@ -18,33 +18,14 @@
package org.apache.flink.table.client.cli;
-import org.junit.rules.ExternalResource;
-
-import java.io.PrintStream;
-
/**
* Enables {@link org.apache.flink.table.client.SqlClient} to create a default
terminal using {@link
* System#in} and {@link System#out} as the input and output stream. This can
allows tests to easily
* mock input stream of the SqlClient by hijacking the standard stream.
*/
-public class TerminalStreamsResource extends ExternalResource {
-
- public static final TerminalStreamsResource INSTANCE = new
TerminalStreamsResource();
- public static final PrintStream STD_OUT = System.out;
-
- private TerminalStreamsResource() {
- // singleton
- }
-
- @Override
- protected void before() throws Throwable {
- CliClient.useSystemInOutStream = true;
- }
+public class CliClientInputOutputSwitch {
- @Override
- protected void after() {
- CliClient.useSystemInOutStream = false;
- // Reset the System.out
- System.setOut(STD_OUT);
+ public static void useSystemInOutStream(boolean useOrNot) {
+ CliClient.useSystemInOutStream = useOrNot;
Review comment:
I think it unnecessary to have a class for such a simple code. We can
just place the code in `SqlClientTest` and add comments on it. Would better add
`TODO: should be refactored in FLINK-22080` on it too.
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/SqlClientTest.java
##########
@@ -60,8 +60,6 @@
Review comment:
Could you add `@Rule public Timeout timeout = new Timeout(10,
TimeUnit.SECONDS);` in this test class? This can avoid the test hangs.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
