[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r321151966
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -260,6 +277,66 @@ class BinaryThriftServerSuite extends
ThriftServerBaseTest with CommonThriftTest
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+
+// first session, we get the default value of the session status
+withJdbcStatement { statement =>
+ val rs1 = statement.executeQuery("SET spark.sql.shuffle.partitions")
+ rs1.next()
+ assert("spark.sql.shuffle.partitions" === rs1.getString(1))
+ defaultV1 = rs1.getString(2)
+ rs1.close()
+
+ val rs2 = statement.executeQuery("SET hive.cli.print.header")
+ rs2.next()
+
+ assert("hive.cli.print.header" === rs2.getString(1))
+ defaultV2 = rs2.getString(2)
+ rs2.close()
+}
+
+// second session, we update the session status
+withJdbcStatement { statement =>
+ val queries = Seq(
+"SET spark.sql.shuffle.partitions=291",
+"SET hive.cli.print.header=true"
+ )
+
+ queries.map(statement.execute)
+ val rs1 = statement.executeQuery("SET spark.sql.shuffle.partitions")
+ rs1.next()
+ assert("spark.sql.shuffle.partitions" === rs1.getString(1))
+ assert("291" === rs1.getString(2))
+ rs1.close()
+
+ val rs2 = statement.executeQuery("SET hive.cli.print.header")
+ rs2.next()
+ assert("hive.cli.print.header" === rs2.getString(1))
+ assert("true" === rs2.getString(2))
+ rs2.close()
+}
+
+// third session, we get the latest session status, supposed to be the
+// default value
+withJdbcStatement { statement =>
+ val rs1 = statement.executeQuery("SET spark.sql.shuffle.partitions")
+ rs1.next()
+ assert("spark.sql.shuffle.partitions" === rs1.getString(1))
+ assert(defaultV1 === rs1.getString(2))
+ rs1.close()
+
+ val rs2 = statement.executeQuery("SET hive.cli.print.header")
+ rs2.next()
+ assert("hive.cli.print.header" === rs2.getString(1))
+ assert(defaultV2 === rs2.getString(2))
+ rs2.close()
+}
+
Review comment:
Got it
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r321144138
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -260,6 +277,104 @@ class BinaryThriftServerSuite extends
ThriftServerBaseTest with CommonThriftTest
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+try {
+ // create table
+ withJdbcStatement { statement =>
+val queries = Seq(
+ "CREATE TABLE test_map(key INT, value STRING) USING json",
+ "CACHE TABLE test_table AS SELECT key FROM test_map ORDER BY key
DESC")
+
+queries.foreach(statement.execute)
+
+val plan = statement.executeQuery("explain select * from test_table")
+plan.next()
+plan.next()
+assert(plan.getString(1).contains("InMemoryTableScan"))
+
+val rs1 = statement.executeQuery("SELECT key FROM test_table ORDER BY
KEY DESC")
Review comment:
In fact, I think this create table test hasn’t much to do with multiple
sessions. And this test overlaps with other tests. I should delete this.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r320725961 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: Now we really don't have a timeout problem after simplifying the test. I will restore this place. If a timeout occurs later, we can modify it in a separate PR 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319729436
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,137 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ def getTestDataFilePath(): URL = {
+
Thread.currentThread().getContextClassLoader.getResource("data/files/small_kv.txt")
+ }
+
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+try {
+ // create table
+ withJdbcStatement { statement =>
+val queries = Seq(
+ "CREATE TABLE test_map(key INT, value STRING)",
+ s"LOAD DATA LOCAL INPATH '${getTestDataFilePath}' OVERWRITE INTO
TABLE test_map",
+ "CACHE TABLE test_table AS SELECT key FROM test_map ORDER BY key
DESC",
+ "CREATE DATABASE db1")
+
+queries.foreach(statement.execute)
+
+val plan = statement.executeQuery("explain select * from test_table")
+plan.next()
+plan.next()
+assert(plan.getString(1).contains("InMemoryTableScan"))
+
+val rs1 = statement.executeQuery("SELECT key FROM test_table ORDER BY
KEY DESC")
+val buf1 = new collection.mutable.ArrayBuffer[Int]()
+while (rs1.next()) {
+ buf1 += rs1.getInt(1)
+}
+rs1.close()
+
+val rs2 = statement.executeQuery("SELECT key FROM test_map ORDER BY
KEY DESC")
+val buf2 = new collection.mutable.ArrayBuffer[Int]()
+while (rs2.next()) {
+ buf2 += rs2.getInt(1)
+}
+rs2.close()
+
+assert(buf1 === buf2)
+
+data = buf1
+ }
+
+ // first session, we get the default value of the session status
+ withJdbcStatement { statement =>
+val rs1 = statement.executeQuery(s"SET spark.sql.shuffle.partitions")
+rs1.next()
+defaultV1 = rs1.getString(1)
+assert(defaultV1 != "200")
+rs1.close()
+
+val rs2 = statement.executeQuery("SET hive.cli.print.header")
+rs2.next()
+
+defaultV2 = rs2.getString(1)
+assert(defaultV1 != "true")
+rs2.close()
+ }
+
+ // second session, we update the session status
+ withJdbcStatement { statement =>
+val queries = Seq(
+ s"SET spark.sql.shuffle.partitions=291",
+ "SET hive.cli.print.header=true"
+)
+
+queries.map(statement.execute)
+val rs1 = statement.executeQuery(s"SET spark.sql.shuffle.partitions")
+rs1.next()
+assert("spark.sql.shuffle.partitions" === rs1.getString(1))
+assert("291" === rs1.getString(2))
+rs1.close()
+
+val rs2 = statement.executeQuery("SET hive.cli.print.header")
+rs2.next()
+assert("hive.cli.print.header" === rs2.getString(1))
+assert("true" === rs2.getString(2))
+rs2.close()
+ }
+
+ // third session, we get the latest session status, supposed to be the
+ // default value
+ withJdbcStatement { statement =>
+val rs1 = statement.executeQuery(s"SET spark.sql.shuffle.partitions")
+rs1.next()
+assert(defaultV1 === rs1.getString(1))
+rs1.close()
+
+val rs2 = statement.executeQuery("SET hive.cli.print.header")
+rs2.next()
+assert(defaultV2 === rs2.getString(1))
+rs2.close()
+ }
+
+ // switch another database
Review comment:
For testing multiple sessions, this section is really not relevant. I'll
delete this part
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319729380
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,137 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ def getTestDataFilePath(): URL = {
+
Thread.currentThread().getContextClassLoader.getResource("data/files/small_kv.txt")
+ }
+
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+try {
+ // create table
+ withJdbcStatement { statement =>
+val queries = Seq(
+ "CREATE TABLE test_map(key INT, value STRING)",
+ s"LOAD DATA LOCAL INPATH '${getTestDataFilePath}' OVERWRITE INTO
TABLE test_map",
+ "CACHE TABLE test_table AS SELECT key FROM test_map ORDER BY key
DESC",
+ "CREATE DATABASE db1")
+
+queries.foreach(statement.execute)
+
+val plan = statement.executeQuery("explain select * from test_table")
+plan.next()
+plan.next()
+assert(plan.getString(1).contains("InMemoryTableScan"))
+
+val rs1 = statement.executeQuery("SELECT key FROM test_table ORDER BY
KEY DESC")
+val buf1 = new collection.mutable.ArrayBuffer[Int]()
+while (rs1.next()) {
+ buf1 += rs1.getInt(1)
+}
+rs1.close()
+
+val rs2 = statement.executeQuery("SELECT key FROM test_map ORDER BY
KEY DESC")
+val buf2 = new collection.mutable.ArrayBuffer[Int]()
+while (rs2.next()) {
+ buf2 += rs2.getInt(1)
+}
+rs2.close()
+
+assert(buf1 === buf2)
+
+data = buf1
+ }
+
+ // first session, we get the default value of the session status
+ withJdbcStatement { statement =>
+val rs1 = statement.executeQuery(s"SET spark.sql.shuffle.partitions")
+rs1.next()
+defaultV1 = rs1.getString(1)
+assert(defaultV1 != "200")
+rs1.close()
+
+val rs2 = statement.executeQuery("SET hive.cli.print.header")
+rs2.next()
+
+defaultV2 = rs2.getString(1)
+assert(defaultV1 != "true")
+rs2.close()
+ }
+
+ // second session, we update the session status
+ withJdbcStatement { statement =>
+val queries = Seq(
+ s"SET spark.sql.shuffle.partitions=291",
Review comment:
I’ll fix this
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319729330
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,137 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ def getTestDataFilePath(): URL = {
+
Thread.currentThread().getContextClassLoader.getResource("data/files/small_kv.txt")
+ }
+
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+try {
+ // create table
+ withJdbcStatement { statement =>
+val queries = Seq(
+ "CREATE TABLE test_map(key INT, value STRING)",
+ s"LOAD DATA LOCAL INPATH '${getTestDataFilePath}' OVERWRITE INTO
TABLE test_map",
+ "CACHE TABLE test_table AS SELECT key FROM test_map ORDER BY key
DESC",
+ "CREATE DATABASE db1")
+
+queries.foreach(statement.execute)
+
+val plan = statement.executeQuery("explain select * from test_table")
+plan.next()
+plan.next()
+assert(plan.getString(1).contains("InMemoryTableScan"))
+
+val rs1 = statement.executeQuery("SELECT key FROM test_table ORDER BY
KEY DESC")
+val buf1 = new collection.mutable.ArrayBuffer[Int]()
+while (rs1.next()) {
+ buf1 += rs1.getInt(1)
+}
+rs1.close()
+
+val rs2 = statement.executeQuery("SELECT key FROM test_map ORDER BY
KEY DESC")
+val buf2 = new collection.mutable.ArrayBuffer[Int]()
+while (rs2.next()) {
+ buf2 += rs2.getInt(1)
+}
+rs2.close()
+
+assert(buf1 === buf2)
+
+data = buf1
+ }
+
+ // first session, we get the default value of the session status
+ withJdbcStatement { statement =>
+val rs1 = statement.executeQuery(s"SET spark.sql.shuffle.partitions")
+rs1.next()
+defaultV1 = rs1.getString(1)
+assert(defaultV1 != "200")
Review comment:
Got it. I'll change this test
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319729284
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -17,9 +17,13 @@
package org.apache.livy.thriftserver
+import java.net.URL
import java.sql.{Connection, Date, SQLException, Statement, Types}
-import org.apache.hive.jdbc.HiveStatement
+import scala.collection.mutable.ArrayBuffer
+import scala.io.Source
+
+import org.apache.hive.jdbc.{HiveDriver, HiveStatement}
Review comment:
My fault. I forgot to delete it after I used it as a test
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319528249 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: Without the two profiles, both thriftserver and integration-test will be executed in ITs, which may cause timeouts. So we need these two profiles. Then separate thriftserver and integration-test in travis.yml. Like this: Env: MVN_FLAG = '- Pthriftserver - DskipTests' Env: MVN_FLAG = '- Pintegration - test - Pcoverage - DskipTests' 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319526712 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: Without the two profiles, both thriftserver and integration-test will be executed in ITs, which may cause timeouts. So we need these two profiles. Then separate thriftserver and integration-test in travis.yml. Like this: Env: MVN_FLAG = '- Pthriftserver - DskipTests' Env: MVN_FLAG = '- Pintegration - test - Pcoverage - DskipTests' 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319526712 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: Without the two profiles, both thriftserver and integration-test will be executed in ITs, which may cause timeouts. So we need these two profiles. Then separate thriftserver and integration-test in travis.yml. Like this: Env: MVN_FLAG = '- Pthriftserver - DskipTests' Env: MVN_FLAG = '- Pintegration - test - Pcoverage - DskipTests' 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319526712 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: Without the two profiles, both thriftserver and integration-test will be executed in ITs, which may cause timeouts. So we need these two profiles. Then separate thriftserver and integration-test in travis.yml. Like this: -name: "Spark 2.2 ITs" Env: MVN_FLAG = '- Pthriftserver - DskipTests' - name: "Spark 2.2 ITs Without Thrift" Env: MVN_FLAG = '- Pintegration - test - Pcoverage - DskipTests' 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319413752
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +263,171 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ def getTestDataFilePath(): URL = {
+
Thread.currentThread().getContextClassLoader.getResource("data/files/small_kv.txt")
+ }
+
+ test("result set containing NULL") {
Review comment:
And there is a overlap here with the dataTypesTest. I'm going to delete this
test.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319411850
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +263,171 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ def getTestDataFilePath(): URL = {
+
Thread.currentThread().getContextClassLoader.getResource("data/files/small_kv.txt")
+ }
+
+ test("result set containing NULL") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"CREATE TABLE test_null(key INT, val STRING)",
+"INSERT INTO test_null VALUES(null, 'val_01')")
+ queries.foreach(statement.execute)
+ assertResult(1, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test_null
WHERE key IS NULL")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test_null")
+}
+ }
+
+ test("Binary type support") {
Review comment:
Yes, this can test in dataTypesTest.I'll change it here
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319409743
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -119,13 +130,17 @@ trait CommonThriftTests {
assert(tablesResultSet.getString(3) == "test_get_tables")
assert(tablesResultSet.getString(4) == "TABLE")
assert(!tablesResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_tables")
Review comment:
I'm going to add a try finally here and put this part in finally
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319409203 ## File path: pom.xml ## @@ -1009,6 +1007,20 @@ + Review comment: These two parts are added in order to separate the thriftserver and the ITs of integration-test(Coverage has depends on integration-test). then manually specified with -p in travis.yml 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319406484 ## File path: .travis.yml ## @@ -24,16 +24,22 @@ matrix: include: - name: "Spark 2.2 Unit Tests" env: MVN_FLAG='-Pthriftserver -DskipITs' - - name: "Spark 2.2 ITs" + - name: "Spark 2.2 Thrift ITs" Review comment: You're right. This place should not need to be highlighted. I'll change this to "Spark 2.2 ITs" 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319342405
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -17,12 +17,21 @@
package org.apache.livy.thriftserver
+import java.net.URL
import java.sql.{Connection, Date, SQLException, Statement, Types}
-import org.apache.hive.jdbc.HiveStatement
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.hive.jdbc.{HiveDriver, HiveStatement}
import org.apache.livy.LivyConf
+object TestData {
+ def getTestDataFilePath(name: String): URL = {
+
Thread.currentThread().getContextClassLoader.getResource(s"data/files/$name")
+ }
+ val smallKv = getTestDataFilePath("small_kv.txt")
Review comment:
Got it, I’ll change it here
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319342207 ## File path: .travis.yml ## @@ -23,17 +23,23 @@ language: scala matrix: include: - name: "Spark 2.2 Unit Tests" -env: MVN_FLAG='-Pthriftserver -DskipITs' - - name: "Spark 2.2 ITs" +env: MVN_FLAG='-Pthriftserver -Pintegration-test -Pcoverage -DskipITs' Review comment: I confirmed that there are no Unit Tests in integration-test and coverage(Coverage has depends on integration-test). So I'm going to delete ntegration-test and coverage in this place. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319340706 ## File path: pom.xml ## @@ -1009,6 +1007,23 @@ + + integration-test + +true Review comment: It's my fault. I added this for testing, but it didn't work. I'm going to delete this 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319064000
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
Review comment:
Get it, thanks Marco. I will delete some tests that are not related to livy.
Keep only livy-related 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319051929
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ test("JDBC query execution") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"SET spark.sql.shuffle.partitions=3",
+"DROP TABLE IF EXISTS test",
+"CREATE TABLE test(key INT, val STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test",
+"CACHE TABLE test")
+
+ queries.foreach(statement.execute)
+
+ assertResult(5, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test")
+ statement.close()
+}
+ }
+
+ test("test add jar and udf") {
Review comment:
What I understand here is that we tested livy thriftserver to perform some
common SQL usage.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319049842
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
Review comment:
We added this section because we found that none of livy-thrift's UTs had
created hive tables for testing. But we can see this in spark-thrift UTs. So we
added this for alignment
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319040975
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -178,6 +193,8 @@ trait CommonThriftTests {
assert(columnsResultSet.getString(22) == null)
assert(columnsResultSet.getString(23) == "NO")
assert(!columnsResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_columns")
Review comment:
Okay, I'm going to delete the previous drop statement
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318963375
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -178,6 +193,8 @@ trait CommonThriftTests {
assert(columnsResultSet.getString(22) == null)
assert(columnsResultSet.getString(23) == "NO")
assert(!columnsResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_columns")
Review comment:
Same as above
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319040831
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -119,13 +130,17 @@ trait CommonThriftTests {
assert(tablesResultSet.getString(3) == "test_get_tables")
assert(tablesResultSet.getString(4) == "TABLE")
assert(!tablesResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_tables")
Review comment:
Okay, I'm going to delete the previous drop statement
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319030285
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ test("JDBC query execution") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"SET spark.sql.shuffle.partitions=3",
+"DROP TABLE IF EXISTS test",
+"CREATE TABLE test(key INT, val STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test",
+"CACHE TABLE test")
+
+ queries.foreach(statement.execute)
+
+ assertResult(5, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test")
+ statement.close()
+}
+ }
+
+ test("test add jar and udf") {
+withJdbcStatement { statement =>
+ val jarPath = "src/test/resources/TestUDTF.jar"
+ val jarURL = s"file://${System.getProperty("user.dir")}/$jarPath"
+ Seq(
+"SET foo=bar",
+s"ADD JAR $jarURL",
+s"""CREATE TEMPORARY FUNCTION udtf_count2
+ |AS
'org.apache.spark.sql.hive.execution.GenericUDTFCount2'""".stripMargin
Review comment:
yes，StripMargin can be removed here
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319022573
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -17,12 +17,21 @@
package org.apache.livy.thriftserver
+import java.net.URL
import java.sql.{Connection, Date, SQLException, Statement, Types}
-import org.apache.hive.jdbc.HiveStatement
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.hive.jdbc.{HiveDriver, HiveStatement}
import org.apache.livy.LivyConf
+object TestData {
+ def getTestDataFilePath(name: String): URL = {
+
Thread.currentThread().getContextClassLoader.getResource(s"data/files/$name")
Review comment:
Use "getClass.getResource(s"data/files/$name")" will get exception "LOAD
DATA input path does not exist: null"
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r319022573
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -17,12 +17,21 @@
package org.apache.livy.thriftserver
+import java.net.URL
import java.sql.{Connection, Date, SQLException, Statement, Types}
-import org.apache.hive.jdbc.HiveStatement
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.hive.jdbc.{HiveDriver, HiveStatement}
import org.apache.livy.LivyConf
+object TestData {
+ def getTestDataFilePath(name: String): URL = {
+
Thread.currentThread().getContextClassLoader.getResource(s"data/files/$name")
Review comment:
use "getClass.getResource(s"data/files/$name")" I will get exception "LOAD
DATA input path does not exist: null"
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318973514
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ test("JDBC query execution") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"SET spark.sql.shuffle.partitions=3",
+"DROP TABLE IF EXISTS test",
+"CREATE TABLE test(key INT, val STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test",
+"CACHE TABLE test")
+
+ queries.foreach(statement.execute)
+
+ assertResult(5, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test")
+ statement.close()
+}
+ }
+
+ test("test add jar and udf") {
+withJdbcStatement { statement =>
+ val jarPath = "src/test/resources/TestUDTF.jar"
+ val jarURL = s"file://${System.getProperty("user.dir")}/$jarPath"
+ Seq(
+"SET foo=bar",
+s"ADD JAR $jarURL",
+s"""CREATE TEMPORARY FUNCTION udtf_count2
+ |AS
'org.apache.spark.sql.hive.execution.GenericUDTFCount2'""".stripMargin
+ ).foreach(statement.execute)
+
+ val rs1 = statement.executeQuery("SET foo")
+
+ assert(rs1.next())
+ assert(rs1.getString(1) === "foo")
+ assert(rs1.getString(2) === "bar")
+
+ val rs2 = statement.executeQuery("DESCRIBE FUNCTION udtf_count2")
+
+ assert(rs2.next())
+ assert(rs2.getString(1) === "Function: udtf_count2")
+
+ assert(rs2.next())
+ assertResult("Class:
org.apache.spark.sql.hive.execution.GenericUDTFCount2") {
+rs2.getString(1)
+ }
+
+ assert(rs2.next())
+ assert(rs2.getString(1) === "Usage: N/A.")
+
+ statement.executeQuery("DROP TEMPORARY FUNCTION udtf_count2")
+ statement.close()
+
+}
+ }
+
+ test("result set iterator") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"DROP TABLE IF EXISTS test_iterator",
+"CREATE TABLE test_iterator(key INT, val STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test_iterator")
+
+ queries.foreach(statement.execute)
+
+ val resultSet = statement.executeQuery("SELECT key FROM test_iterator")
+
+ Seq(238, 86, 311, 27, 165).foreach { key =>
+resultSet.next()
+assert(resultSet.getInt(1) === key)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test_iterator")
+ statement.close()
+}
+ }
+
+ test("result set containing NULL") {
+withJdbcStatement { statement =>
+
+ val queries = Seq(
+"DROP TABLE IF EXISTS test_null",
+"CREATE TABLE test_null(key INT, val STRING)",
+"INSERT INTO test_null VALUES(null, 'val_01')")
+ queries.foreach(statement.execute)
+ assertResult(1, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test_null
WHERE key IS NULL")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test_null")
+ statement.close()
+}
+ }
+
+ test("Binary type support") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"DROP TABLE IF EXISTS test_binary",
+"CREATE TABLE test_binary(key INT, value STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test_binary")
+
+ queries.foreach(statement.execute)
+
+ val expected: Array[Byte] = "val_238".getBytes
+ assertResult(expected) {
+val resultSet = statement.executeQuery(
+ "SELECT CAST(value as BINARY) FROM test_binary LIMIT 1")
+resultSet.next()
+resultSet.getObject(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test_binary")
+ statement.close()
+}
+ }
+
+ test("test multiple session") {
+var defaultV1: String = null
+var defaultV2: String = null
+var data: ArrayBuffer[Int] = null
+
+// first delete test db and table with same name
+withJdbcStatement { statement =>
+
+ val queries = Seq(
+"DROP TABLE IF EXISTS test_map",
+"DROP TABLE IF EXISTS db1.test_map2",
+"DROP DATABASE IF EXISTS db1")
+
+ queries.foreach(statement.execute)
+}
+
+// create table
+withJdbcStatement {
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318970583
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
+
+ test("JDBC query execution") {
+withJdbcStatement { statement =>
+ val queries = Seq(
+"SET spark.sql.shuffle.partitions=3",
+"DROP TABLE IF EXISTS test",
+"CREATE TABLE test(key INT, val STRING)",
+s"LOAD DATA LOCAL INPATH '${TestData.smallKv}' OVERWRITE INTO TABLE
test",
+"CACHE TABLE test")
+
+ queries.foreach(statement.execute)
+
+ assertResult(5, "Row count mismatch") {
+val resultSet = statement.executeQuery("SELECT COUNT(*) FROM test")
+resultSet.next()
+resultSet.getInt(1)
+ }
+ statement.executeQuery("DROP TABLE IF EXISTS test")
+ statement.close()
Review comment:
yes，I will modify this place
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318969732
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -259,6 +276,268 @@ trait CommonThriftTests {
class BinaryThriftServerSuite extends ThriftServerBaseTest with
CommonThriftTests {
override def mode: ServerMode.Value = ServerMode.binary
override def port: Int = 2
+ // In BinaryThriftServerSuite, we set ENABLE_HIVE_CONTEXT=true to support
the creation
+ // of Hive tables and the creation of udf.
+ livyConf.set(LivyConf.ENABLE_HIVE_CONTEXT, true)
Review comment:
In spark-thrift tests ENABLE_HIVE_CONTEXT = true is default . this can
override all test scenarios where ENABLE_HIVE_CONTEXT = false. And in
livy-thrift, the IT test ran for nearly 40 minutes, and there was a risk of
timeout. (Travis default timeout is 50 minutes)
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318963375
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -178,6 +193,8 @@ trait CommonThriftTests {
assert(columnsResultSet.getString(22) == null)
assert(columnsResultSet.getString(23) == "NO")
assert(!columnsResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_columns")
Review comment:
Same as above
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for
ThriftServer
URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318963277
##
File path:
thriftserver/server/src/test/scala/org/apache/livy/thriftserver/ThriftServerSuites.scala
##
@@ -119,13 +130,17 @@ trait CommonThriftTests {
assert(tablesResultSet.getString(3) == "test_get_tables")
assert(tablesResultSet.getString(4) == "TABLE")
assert(!tablesResultSet.next())
+statement.execute("DROP TABLE IF EXISTS test_get_tables")
Review comment:
After setting ENABLE_HIVE_CONTEXT=true, the tables created by the previous
test are still visible in the next test. This can affect the results of unit
tests. So at the end of each test, the currently created table needs to be
deleted
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318957528 ## File path: .travis.yml ## @@ -23,17 +23,23 @@ language: scala matrix: include: - name: "Spark 2.2 Unit Tests" -env: MVN_FLAG='-Pthriftserver -DskipITs' - - name: "Spark 2.2 ITs" +env: MVN_FLAG='-Pthriftserver -Pintegration-test -Pcoverage -DskipITs' + - name: "Spark 2.2 ITs without integration-test and coverage" env: MVN_FLAG='-Pthriftserver -DskipTests' + - name: "Spark 2.2 ITs without thriftserver" +env: MVN_FLAG='-Pintegration-test -Pcoverage -DskipTests' - name: "Spark 2.3 Unit Tests" -env: MVN_FLAG='-Pspark-2.3 -Pthriftserver -DskipITs' - - name: "Spark 2.3 ITs" +env: MVN_FLAG='-Pspark-2.3 -Pthriftserver -Pintegration-test -Pcoverage -DskipITs' + - name: "Spark 2.3 ITs without integration-test and coverage" Review comment: I will modify this place 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer
captainzmc commented on a change in pull request #209: [LIVY-640] Add tests for ThriftServer URL: https://github.com/apache/incubator-livy/pull/209#discussion_r318956453 ## File path: .travis.yml ## @@ -23,17 +23,23 @@ language: scala matrix: include: - name: "Spark 2.2 Unit Tests" -env: MVN_FLAG='-Pthriftserver -DskipITs' - - name: "Spark 2.2 ITs" +env: MVN_FLAG='-Pthriftserver -Pintegration-test -Pcoverage -DskipITs' + - name: "Spark 2.2 ITs without integration-test and coverage" Review comment: "Spark 2.2 ITs without integration-test and coverage" I will change to "Spark 2.2 Thrift ITs". coverage has dependencies on integration-test, so it needs to be taken out 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: us...@infra.apache.org With regards, Apache Git Services
