mgaido91 commented on code in PR #377:
URL: https://github.com/apache/incubator-livy/pull/377#discussion_r1051581091
##########
thriftserver/session/src/test/java/org/apache/livy/thriftserver/session/ThriftSessionTest.java:
##########
@@ -192,6 +192,27 @@ public void testThriftSession() throws Exception {
waitFor(new UnregisterSessionJob(s3));
}
+ @Test
+ public void testSessionState() throws Exception {
Review Comment:
why are we testing the RSCDriver in thriftserver? All changes are outside
the thriftserver, so the UTs should be in rsc as well. This will also avoid
adding dependencies to the thriftserver module.
##########
thriftserver/session/src/test/java/org/apache/livy/thriftserver/session/ThriftSessionTest.java:
##########
@@ -19,21 +19,21 @@
import java.net.URI;
import java.nio.file.Files;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
+import java.util.*;
Review Comment:
please avoid importing everything, just add what we need
##########
thriftserver/session/src/test/java/org/apache/livy/thriftserver/session/ThriftSessionTest.java:
##########
@@ -192,6 +192,27 @@ public void testThriftSession() throws Exception {
waitFor(new UnregisterSessionJob(s3));
}
+ @Test
+ public void testSessionState() throws Exception {
+
Review Comment:
please remove unneeded blank line
##########
thriftserver/session/src/test/java/org/apache/livy/thriftserver/session/ThriftSessionTest.java:
##########
@@ -214,7 +235,7 @@ private SqlJob newSqlJob(String session, String stId,
String statement) {
*/
private Exception expectError(Job<?> job, String expected) throws
TimeoutException {
try {
- livy.submit(job).get(10, TimeUnit.SECONDS);
+ livy.submit(job).get(10, SECONDS);
Review Comment:
these are unneeded changes, please avoid them and use `TimeUnit.SECONDS` in
the added code as well
--
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]
