Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/984#discussion_r145515947
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/test/BaseDirTestWatcher.java ---
@@ -0,0 +1,184 @@
+/*
+ * 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.drill.test;
+
+import com.google.common.base.Charsets;
+import org.apache.commons.io.FileUtils;
+import org.apache.drill.common.util.TestTools;
+import org.apache.drill.exec.util.TestUtilities;
+import org.junit.runner.Description;
+
+import java.io.File;
+import java.io.IOException;
+
+public class BaseDirTestWatcher extends DirTestWatcher {
+ public enum DirType {
+ ROOT,
+ TEST_TMP;
+ }
+
+ private File tmpDir;
+ private File storeDir;
+ private File dfsTestTmpParentDir;
+ private File dfsTestTmpDir;
+ private File rootDir;
+
+ public BaseDirTestWatcher() {
+ super();
+ }
+
+ @Override
+ protected void starting(Description description) {
+ super.starting(description);
+
+ rootDir = makeSubDir("root");
+ tmpDir = new File(rootDir, "tmp");
+ storeDir = new File(rootDir, "store");
+ dfsTestTmpParentDir = new File(rootDir, "dfsTestTmp");
+
+ tmpDir.mkdirs();
+ storeDir.mkdirs();
+ dfsTestTmpParentDir.mkdirs();
+ }
+
+ public File getTmpDir() {
+ return tmpDir;
+ }
+
+ public File getStoreDir() {
+ return storeDir;
+ }
+
+ public File getDfsTestTmpParentDir() {
+ return dfsTestTmpParentDir;
+ }
+
+ public File getDfsTestTmpDir() {
+ return dfsTestTmpDir;
+ }
+
+ public String getDfsTestTmpDirPath() {
+ return dfsTestTmpDir.getAbsolutePath().replaceAll("/\\./", "/");
+ }
+
+ public File getRootDir() {
+ return rootDir;
+ }
+
+ public String getRootDirPath() {
+ return rootDir.getAbsolutePath().replaceAll("/\\./", "/");
+ }
+
+ public void newDfsTestTmpDir() {
+ dfsTestTmpDir =
TestUtilities.createTempDir(BaseTestQuery.dirTestWatcher.getDfsTestTmpParentDir());
+ }
+
+ private File getDir(DirType type) {
+ switch (type) {
+ case ROOT:
+ return rootDir;
+ case TEST_TMP:
+ return dfsTestTmpDir;
+ default:
+ throw new IllegalArgumentException(String.format("Unsupported type
%s", type));
+ }
+ }
+
+ public File makeRootSubDir(String relPath) {
--- End diff --
Here is where we might have a bit of a discussion. Should we represent
directory paths as:
* Strings (as done here)
* Java NIO `Path` objects (which Java seems to want to be the new standard)
* Hadoop `Path` objects (since that is what the Drill file system uses)
* Good old fashioned Java `File` objects (which, while old, works well in
common cases)
I would suggest we use `File` for references to file system paths:
* The Java `Path` has the same name as the Hadoop `Path`, causing endless
confusion.
* Hadoop `Path` is overkill for simple uses in tests.
* `String` encourages people to write their own code to join paths by
concatenating.
The use of `File` keeps things simple and standard. Concatenation is simple.
Other solutions are, of course, possible. We could use `String` and
replicate the methods on `File` or either of the `Path` classes, say. The point
is, let's pick a standard and use it everywhere.
---
