mchades commented on code in PR #4996:
URL: https://github.com/apache/gravitino/pull/4996#discussion_r1795404339


##########
trino-connector/integration-test/src/test/java/org/apache/gravitino/trino/connector/integration/test/TrinoQueryTestTool.java:
##########
@@ -217,8 +217,9 @@ public static void main(String[] args) throws Exception {
       TrinoQueryITBase.autoStart = autoStart;
       TrinoQueryITBase.autoStartGravitino = autoStartGravitino;
 
-      TrinoQueryIT.setup();
+      //      TrinoQueryIT.setup();

Review Comment:
   plz remove this



##########
spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/integration/test/sql/SparkQueryRunner.java:
##########
@@ -83,6 +84,7 @@ public SparkQueryRunner(SparkTestConfig sparkTestConfig) {
     }
     initSparkEnv();
 
+    abstractIT = new AbstractIT();

Review Comment:
   Why not use inheritance instead of a new instance?



##########
integration-test-common/build.gradle.kts:
##########
@@ -43,6 +43,8 @@ dependencies {
   testImplementation(libs.httpclient5)
   testImplementation(libs.testcontainers)
   testImplementation(libs.testcontainers.mysql)
+  testImplementation(libs.mysql.driver)
+  testImplementation(libs.postgresql.driver)

Review Comment:
   why add these dependencies?



##########
trino-connector/integration-test/src/test/java/org/apache/gravitino/trino/connector/integration/test/TrinoQueryITBase.java:
##########
@@ -68,14 +68,17 @@ public class TrinoQueryITBase {
   protected static final String metalakeName = "test";
   protected static GravitinoMetalake metalake;
 
-  private static void setEnv() throws Exception {
+  private static AbstractIT abstractIT;
+
+  private void setEnv() throws Exception {
+    abstractIT = new AbstractIT();

Review Comment:
   Here as well, it is strange to create an instance named "abstractXX".



-- 
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]

Reply via email to