[GitHub] [hudi] xushiyan commented on a change in pull request #1746: [HUDI-996] Add functional test suite for hudi-utilities

[GitBox <gi...@apache.org>]

xushiyan commented on a change in pull request #1746:
URL: https://github.com/apache/hudi/pull/1746#discussion_r449814824



##########
File path: hudi-client/src/test/java/org/apache/hudi/testutils/FunctionalTestHarness.java
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.testutils;
+
+import org.apache.hudi.client.HoodieWriteClient;
+import org.apache.hudi.common.testutils.minicluster.HdfsTestService;
+
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdfs.DistributedFileSystem;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.spark.api.java.JavaSparkContext;
+import org.apache.spark.sql.SQLContext;
+import org.apache.spark.sql.SparkSession;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.IOException;
+
+public class FunctionalTestHarness implements SparkProvider, DFSProvider {
+
+  private static transient SparkSession spark;
+  private static transient SQLContext sqlContext;
+  private static transient JavaSparkContext jsc;
+
+  private static transient HdfsTestService hdfsTestService;
+  private static transient MiniDFSCluster dfsCluster;
+  private static transient DistributedFileSystem dfs;
+
+  /**
+   * An indicator of the initialization status.
+   */
+  protected boolean initialized = false;

Review comment:
       > Food for thought: if tests are run parallely, in the same jvm. (parallelism option in surefire)... this boolean may not be sufficient for synchronization.. i.e two tests can attempt to create these test resources in parallel.
   
   Yes, and this is taken care of in the `functional-tests` profile
   https://github.com/apache/hudi/blob/574dcf920c4677513f6fdc8d441bb42827afa5a2/pom.xml#L1024-L1025
   
   These 2 settings make sure creating only one new JVM process to execute all tests in one Maven module. So here we make sure all functional tests in utilities are run sequentially.
   
   > also how come this boolean is not `static`? otherwise in every before each it will try to create resourceS?
   
   The `initialized` is not used as lock, rather only an indicator. It is reset by checking `spark` and `hdfsTestService`
   https://github.com/apache/hudi/pull/1746/files/87eb111b4f40dc0129790c3043683e2e1b593ca0#diff-543bacddbb566cb15499906d64c783a5R87-R89
   This can be used for debugging purpose where subclass of this class prints this info...I can see that this may be confusing, maybe removing it later if not commonly used.




----------------------------------------------------------------
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:
users@infra.apache.org