You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by iw...@apache.org on 2022/03/14 01:17:34 UTC

[hadoop] branch branch-3.2 updated (6f815a5 -> 826d13a)

This is an automated email from the ASF dual-hosted git repository.

iwasakims pushed a change to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git.


    from 6f815a5  HDFS-16428. Source path with storagePolicy cause wrong typeConsumed while rename (#3898). Contributed by lei w.
     new d09ca8b  HADOOP-16811: Use JUnit TemporaryFolder Rule in TestFileUtils (#1811). Contributed by David Mollitor.
     new 826d13a  HADOOP-18155. Refactor tests in TestFileUtil (#4063)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../main/java/org/apache/hadoop/fs/FileUtil.java   |  36 +-
 .../java/org/apache/hadoop/fs/TestFileUtil.java    | 611 ++++++++++-----------
 2 files changed, 334 insertions(+), 313 deletions(-)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


[hadoop] 02/02: HADOOP-18155. Refactor tests in TestFileUtil (#4063)

Posted by iw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

iwasakims pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git

commit 826d13a59af7a23a2a1036751e48abcfc5bc60b2
Author: Wei-Chiu Chuang <we...@apache.org>
AuthorDate: Mon Mar 14 08:40:17 2022 +0800

    HADOOP-18155. Refactor tests in TestFileUtil (#4063)
    
    (cherry picked from commit d0fa9b5775185bd83e4a767a7dfc13ef89c5154a)
    
     Conflicts:
    	hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
    	hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
    
    Co-authored-by: Gautham B A <ga...@gmail.com>
    (cherry picked from commit 743db6e7b4d2acd893f527c9312e6dbe6eefef4f)
---
 .../main/java/org/apache/hadoop/fs/FileUtil.java   |  36 +-
 .../java/org/apache/hadoop/fs/TestFileUtil.java    | 394 +++++++++++++--------
 2 files changed, 271 insertions(+), 159 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
index 5c71284..bae69d4 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
@@ -40,6 +40,7 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
@@ -971,6 +972,14 @@ public class FileUtil {
           + " would create entry outside of " + outputDir);
     }
 
+    if (entry.isSymbolicLink() || entry.isLink()) {
+      String canonicalTargetPath = getCanonicalPath(entry.getLinkName(), outputDir);
+      if (!canonicalTargetPath.startsWith(targetDirPath)) {
+        throw new IOException(
+            "expanding " + entry.getName() + " would create entry outside of " + outputDir);
+      }
+    }
+
     if (entry.isDirectory()) {
       File subDir = new File(outputDir, entry.getName());
       if (!subDir.mkdirs() && !subDir.isDirectory()) {
@@ -986,10 +995,12 @@ public class FileUtil {
     }
 
     if (entry.isSymbolicLink()) {
-      // Create symbolic link relative to tar parent dir
-      Files.createSymbolicLink(FileSystems.getDefault()
-              .getPath(outputDir.getPath(), entry.getName()),
-          FileSystems.getDefault().getPath(entry.getLinkName()));
+      // Create symlink with canonical target path to ensure that we don't extract
+      // outside targetDirPath
+      String canonicalTargetPath = getCanonicalPath(entry.getLinkName(), outputDir);
+      Files.createSymbolicLink(
+          FileSystems.getDefault().getPath(outputDir.getPath(), entry.getName()),
+          FileSystems.getDefault().getPath(canonicalTargetPath));
       return;
     }
 
@@ -1001,7 +1012,8 @@ public class FileUtil {
     }
 
     if (entry.isLink()) {
-      File src = new File(outputDir, entry.getLinkName());
+      String canonicalTargetPath = getCanonicalPath(entry.getLinkName(), outputDir);
+      File src = new File(canonicalTargetPath);
       HardLink.createHardLink(src, outputFile);
       return;
     }
@@ -1020,6 +1032,20 @@ public class FileUtil {
   }
 
   /**
+   * Gets the canonical path for the given path.
+   *
+   * @param path      The path for which the canonical path needs to be computed.
+   * @param parentDir The parent directory to use if the path is a relative path.
+   * @return The canonical path of the given path.
+   */
+  private static String getCanonicalPath(String path, File parentDir) throws IOException {
+    java.nio.file.Path targetPath = Paths.get(path);
+    return (targetPath.isAbsolute() ?
+        new File(path) :
+        new File(parentDir, path)).getCanonicalPath();
+  }
+
+  /**
    * Class for creating hardlinks.
    * Supports Unix, WindXP.
    * @deprecated Use {@link org.apache.hadoop.fs.HardLink}
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
index 2a7bfb4..56a8619 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
@@ -42,13 +42,14 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.UnknownHostException;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.FileSystems;
 import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.jar.Attributes;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
@@ -60,9 +61,12 @@ import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.test.GenericTestUtils;
+import org.apache.hadoop.test.LambdaTestUtils;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.tools.tar.TarEntry;
 import org.apache.tools.tar.TarOutputStream;
+
+import org.assertj.core.api.Assertions;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -158,13 +162,12 @@ public class TestFileUtil {
     FileUtils.forceMkdir(dir1);
     FileUtils.forceMkdir(dir2);
 
-    new File(del, FILE).createNewFile();
-    File tmpFile = new File(tmp, FILE);
-    tmpFile.createNewFile();
+    Verify.createNewFile(new File(del, FILE));
+    File tmpFile = Verify.createNewFile(new File(tmp, FILE));
 
     // create files
-    new File(dir1, FILE).createNewFile();
-    new File(dir2, FILE).createNewFile();
+    Verify.createNewFile(new File(dir1, FILE));
+    Verify.createNewFile(new File(dir2, FILE));
 
     // create a symlink to file
     File link = new File(del, LINK);
@@ -173,7 +176,7 @@ public class TestFileUtil {
     // create a symlink to dir
     File linkDir = new File(del, "tmpDir");
     FileUtil.symLink(tmp.toString(), linkDir.toString());
-    Assert.assertEquals(5, del.listFiles().length);
+    Assert.assertEquals(5, Objects.requireNonNull(del.listFiles()).length);
 
     // create files in partitioned directories
     createFile(partitioned, "part-r-00000", "foo");
@@ -200,13 +203,9 @@ public class TestFileUtil {
   private File createFile(File directory, String name, String contents)
       throws IOException {
     File newFile = new File(directory, name);
-    PrintWriter pw = new PrintWriter(newFile);
-    try {
+    try (PrintWriter pw = new PrintWriter(newFile)) {
       pw.println(contents);
     }
-    finally {
-      pw.close();
-    }
     return newFile;
   }
 
@@ -218,11 +217,11 @@ public class TestFileUtil {
 
     //Test existing directory with no files case 
     File newDir = new File(tmp.getPath(),"test");
-    newDir.mkdir();
+    Verify.mkdir(newDir);
     Assert.assertTrue("Failed to create test dir", newDir.exists());
     files = FileUtil.listFiles(newDir);
     Assert.assertEquals(0, files.length);
-    newDir.delete();
+    assertTrue(newDir.delete());
     Assert.assertFalse("Failed to delete test dir", newDir.exists());
     
     //Test non-existing directory case, this throws 
@@ -244,11 +243,11 @@ public class TestFileUtil {
 
     //Test existing directory with no files case 
     File newDir = new File(tmp.getPath(),"test");
-    newDir.mkdir();
+    Verify.mkdir(newDir);
     Assert.assertTrue("Failed to create test dir", newDir.exists());
     files = FileUtil.list(newDir);
     Assert.assertEquals("New directory unexpectedly contains files", 0, files.length);
-    newDir.delete();
+    assertTrue(newDir.delete());
     Assert.assertFalse("Failed to delete test dir", newDir.exists());
     
     //Test non-existing directory case, this throws 
@@ -266,7 +265,7 @@ public class TestFileUtil {
   public void testFullyDelete() throws IOException {
     boolean ret = FileUtil.fullyDelete(del);
     Assert.assertTrue(ret);
-    Assert.assertFalse(del.exists());
+    Verify.notExists(del);
     validateTmpDir();
   }
 
@@ -279,13 +278,13 @@ public class TestFileUtil {
   @Test (timeout = 30000)
   public void testFullyDeleteSymlinks() throws IOException {
     File link = new File(del, LINK);
-    Assert.assertEquals(5, del.list().length);
+    assertDelListLength(5);
     // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not
     // delete contents of tmp. See setupDirs for details.
     boolean ret = FileUtil.fullyDelete(link);
     Assert.assertTrue(ret);
-    Assert.assertFalse(link.exists());
-    Assert.assertEquals(4, del.list().length);
+    Verify.notExists(link);
+    assertDelListLength(4);
     validateTmpDir();
 
     File linkDir = new File(del, "tmpDir");
@@ -293,8 +292,8 @@ public class TestFileUtil {
     // delete contents of tmp. See setupDirs for details.
     ret = FileUtil.fullyDelete(linkDir);
     Assert.assertTrue(ret);
-    Assert.assertFalse(linkDir.exists());
-    Assert.assertEquals(3, del.list().length);
+    Verify.notExists(linkDir);
+    assertDelListLength(3);
     validateTmpDir();
   }
 
@@ -310,16 +309,16 @@ public class TestFileUtil {
     // to make y as a dangling link to file tmp/x
     boolean ret = FileUtil.fullyDelete(tmp);
     Assert.assertTrue(ret);
-    Assert.assertFalse(tmp.exists());
+    Verify.notExists(tmp);
 
     // dangling symlink to file
     File link = new File(del, LINK);
-    Assert.assertEquals(5, del.list().length);
+    assertDelListLength(5);
     // Even though 'y' is dangling symlink to file tmp/x, fullyDelete(y)
     // should delete 'y' properly.
     ret = FileUtil.fullyDelete(link);
     Assert.assertTrue(ret);
-    Assert.assertEquals(4, del.list().length);
+    assertDelListLength(4);
 
     // dangling symlink to directory
     File linkDir = new File(del, "tmpDir");
@@ -327,22 +326,22 @@ public class TestFileUtil {
     // delete tmpDir properly.
     ret = FileUtil.fullyDelete(linkDir);
     Assert.assertTrue(ret);
-    Assert.assertEquals(3, del.list().length);
+    assertDelListLength(3);
   }
 
   @Test (timeout = 30000)
   public void testFullyDeleteContents() throws IOException {
     boolean ret = FileUtil.fullyDeleteContents(del);
     Assert.assertTrue(ret);
-    Assert.assertTrue(del.exists());
-    Assert.assertEquals(0, del.listFiles().length);
+    Verify.exists(del);
+    Assert.assertEquals(0, Objects.requireNonNull(del.listFiles()).length);
     validateTmpDir();
   }
 
   private void validateTmpDir() {
-    Assert.assertTrue(tmp.exists());
-    Assert.assertEquals(1, tmp.listFiles().length);
-    Assert.assertTrue(new File(tmp, FILE).exists());
+    Verify.exists(tmp);
+    Assert.assertEquals(1, Objects.requireNonNull(tmp.listFiles()).length);
+    Verify.exists(new File(tmp, FILE));
   }
 
   /**
@@ -366,15 +365,15 @@ public class TestFileUtil {
    * @throws IOException
    */
   private void setupDirsAndNonWritablePermissions() throws IOException {
-    new MyFile(del, FILE_1_NAME).createNewFile();
+    Verify.createNewFile(new MyFile(del, FILE_1_NAME));
 
     // "file1" is non-deletable by default, see MyFile.delete().
 
-    xSubDir.mkdirs();
-    file2.createNewFile();
+    Verify.mkdirs(xSubDir);
+    Verify.createNewFile(file2);
 
-    xSubSubDir.mkdirs();
-    file22.createNewFile();
+    Verify.mkdirs(xSubSubDir);
+    Verify.createNewFile(file22);
 
     revokePermissions(file22);
     revokePermissions(xSubSubDir);
@@ -382,8 +381,8 @@ public class TestFileUtil {
     revokePermissions(file2);
     revokePermissions(xSubDir);
 
-    ySubDir.mkdirs();
-    file3.createNewFile();
+    Verify.mkdirs(ySubDir);
+    Verify.createNewFile(file3);
 
     File tmpFile = new File(tmp, FILE);
     tmpFile.createNewFile();
@@ -449,6 +448,88 @@ public class TestFileUtil {
   }
 
   /**
+   * Asserts if the {@link TestFileUtil#del} meets the given expected length.
+   *
+   * @param expectedLength The expected length of the {@link TestFileUtil#del}.
+   */
+  private void assertDelListLength(int expectedLength) {
+    Assertions.assertThat(del.list()).describedAs("del list").isNotNull().hasSize(expectedLength);
+  }
+
+  /**
+   * Helper class to perform {@link File} operation and also verify them.
+   */
+  public static class Verify {
+    /**
+     * Invokes {@link File#createNewFile()} on the given {@link File} instance.
+     *
+     * @param file The file to call {@link File#createNewFile()} on.
+     * @return The result of {@link File#createNewFile()}.
+     * @throws IOException As per {@link File#createNewFile()}.
+     */
+    public static File createNewFile(File file) throws IOException {
+      assertTrue("Unable to create new file " + file, file.createNewFile());
+      return file;
+    }
+
+    /**
+     * Invokes {@link File#mkdir()} on the given {@link File} instance.
+     *
+     * @param file The file to call {@link File#mkdir()} on.
+     * @return The result of {@link File#mkdir()}.
+     */
+    public static File mkdir(File file) {
+      assertTrue("Unable to mkdir for " + file, file.mkdir());
+      return file;
+    }
+
+    /**
+     * Invokes {@link File#mkdirs()} on the given {@link File} instance.
+     *
+     * @param file The file to call {@link File#mkdirs()} on.
+     * @return The result of {@link File#mkdirs()}.
+     */
+    public static File mkdirs(File file) {
+      assertTrue("Unable to mkdirs for " + file, file.mkdirs());
+      return file;
+    }
+
+    /**
+     * Invokes {@link File#delete()} on the given {@link File} instance.
+     *
+     * @param file The file to call {@link File#delete()} on.
+     * @return The result of {@link File#delete()}.
+     */
+    public static File delete(File file) {
+      assertTrue("Unable to delete " + file, file.delete());
+      return file;
+    }
+
+    /**
+     * Invokes {@link File#exists()} on the given {@link File} instance.
+     *
+     * @param file The file to call {@link File#exists()} on.
+     * @return The result of {@link File#exists()}.
+     */
+    public static File exists(File file) {
+      assertTrue("Expected file " + file + " doesn't exist", file.exists());
+      return file;
+    }
+
+    /**
+     * Invokes {@link File#exists()} on the given {@link File} instance to check if the
+     * {@link File} doesn't exists.
+     *
+     * @param file The file to call {@link File#exists()} on.
+     * @return The negation of the result of {@link File#exists()}.
+     */
+    public static File notExists(File file) {
+      assertFalse("Expected file " + file + " must not exist", file.exists());
+      return file;
+    }
+  }
+
+  /**
    * Extend {@link File}. Same as {@link File} except for two things: (1) This
    * treats file1Name as a very special file which is not delete-able
    * irrespective of it's parent-dir's permissions, a peculiar file instance for
@@ -580,14 +661,13 @@ public class TestFileUtil {
       FileUtil.chmod(partitioned.getAbsolutePath(), "0777", true/*recursive*/);
     }
   }
-  
+
   @Test (timeout = 30000)
-  public void testUnTar() throws IOException {
+  public void testUnTar() throws Exception {
     // make a simple tar:
     final File simpleTar = new File(del, FILE);
-    OutputStream os = new FileOutputStream(simpleTar); 
-    TarOutputStream tos = new TarOutputStream(os);
-    try {
+    OutputStream os = new FileOutputStream(simpleTar);
+    try (TarOutputStream tos = new TarOutputStream(os)) {
       TarEntry te = new TarEntry("/bar/foo");
       byte[] data = "some-content".getBytes("UTF-8");
       te.setSize(data.length);
@@ -596,55 +676,42 @@ public class TestFileUtil {
       tos.closeEntry();
       tos.flush();
       tos.finish();
-    } finally {
-      tos.close();
     }
 
     // successfully untar it into an existing dir:
     FileUtil.unTar(simpleTar, tmp);
     // check result:
-    assertTrue(new File(tmp, "/bar/foo").exists());
+    Verify.exists(new File(tmp, "/bar/foo"));
     assertEquals(12, new File(tmp, "/bar/foo").length());
-    
-    final File regularFile = new File(tmp, "QuickBrownFoxJumpsOverTheLazyDog");
-    regularFile.createNewFile();
-    assertTrue(regularFile.exists());
-    try {
-      FileUtil.unTar(simpleTar, regularFile);
-      assertTrue("An IOException expected.", false);
-    } catch (IOException ioe) {
-      // okay
-    }
+
+    final File regularFile =
+        Verify.createNewFile(new File(tmp, "QuickBrownFoxJumpsOverTheLazyDog"));
+    LambdaTestUtils.intercept(IOException.class, () -> FileUtil.unTar(simpleTar, regularFile));
   }
   
   @Test (timeout = 30000)
   public void testReplaceFile() throws IOException {
-    final File srcFile = new File(tmp, "src");
-    
     // src exists, and target does not exist:
-    srcFile.createNewFile();
-    assertTrue(srcFile.exists());
+    final File srcFile = Verify.createNewFile(new File(tmp, "src"));
     final File targetFile = new File(tmp, "target");
-    assertTrue(!targetFile.exists());
+    Verify.notExists(targetFile);
     FileUtil.replaceFile(srcFile, targetFile);
-    assertTrue(!srcFile.exists());
-    assertTrue(targetFile.exists());
+    Verify.notExists(srcFile);
+    Verify.exists(targetFile);
 
     // src exists and target is a regular file: 
-    srcFile.createNewFile();
-    assertTrue(srcFile.exists());
+    Verify.createNewFile(srcFile);
+    Verify.exists(srcFile);
     FileUtil.replaceFile(srcFile, targetFile);
-    assertTrue(!srcFile.exists());
-    assertTrue(targetFile.exists());
+    Verify.notExists(srcFile);
+    Verify.exists(targetFile);
     
     // src exists, and target is a non-empty directory: 
-    srcFile.createNewFile();
-    assertTrue(srcFile.exists());
-    targetFile.delete();
-    targetFile.mkdirs();
-    File obstacle = new File(targetFile, "obstacle");
-    obstacle.createNewFile();
-    assertTrue(obstacle.exists());
+    Verify.createNewFile(srcFile);
+    Verify.exists(srcFile);
+    Verify.delete(targetFile);
+    Verify.mkdirs(targetFile);
+    File obstacle = Verify.createNewFile(new File(targetFile, "obstacle"));
     assertTrue(targetFile.exists() && targetFile.isDirectory());
     try {
       FileUtil.replaceFile(srcFile, targetFile);
@@ -653,9 +720,9 @@ public class TestFileUtil {
       // okay
     }
     // check up the post-condition: nothing is deleted:
-    assertTrue(srcFile.exists());
+    Verify.exists(srcFile);
     assertTrue(targetFile.exists() && targetFile.isDirectory());
-    assertTrue(obstacle.exists());
+    Verify.exists(obstacle);
   }
   
   @Test (timeout = 30000)
@@ -668,13 +735,13 @@ public class TestFileUtil {
     assertTrue(tmp1.exists() && tmp2.exists());
     assertTrue(tmp1.canWrite() && tmp2.canWrite());
     assertTrue(tmp1.canRead() && tmp2.canRead());
-    tmp1.delete();
-    tmp2.delete();
+    Verify.delete(tmp1);
+    Verify.delete(tmp2);
     assertTrue(!tmp1.exists() && !tmp2.exists());
   }
   
   @Test (timeout = 30000)
-  public void testUnZip() throws IOException {
+  public void testUnZip() throws Exception {
     // make sa simple zip
     final File simpleZip = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleZip); 
@@ -695,18 +762,12 @@ public class TestFileUtil {
     // successfully unzip it into an existing dir:
     FileUtil.unZip(simpleZip, tmp);
     // check result:
-    assertTrue(new File(tmp, "foo").exists());
+    Verify.exists(new File(tmp, "foo"));
     assertEquals(12, new File(tmp, "foo").length());
-    
-    final File regularFile = new File(tmp, "QuickBrownFoxJumpsOverTheLazyDog");
-    regularFile.createNewFile();
-    assertTrue(regularFile.exists());
-    try {
-      FileUtil.unZip(simpleZip, regularFile);
-      assertTrue("An IOException expected.", false);
-    } catch (IOException ioe) {
-      // okay
-    }
+
+    final File regularFile =
+        Verify.createNewFile(new File(tmp, "QuickBrownFoxJumpsOverTheLazyDog"));
+    LambdaTestUtils.intercept(IOException.class, () -> FileUtil.unZip(simpleZip, regularFile));
   }
 
   @Test (timeout = 30000)
@@ -752,24 +813,24 @@ public class TestFileUtil {
     final File dest = new File(del, "dest");
     boolean result = FileUtil.copy(fs, srcPath, dest, false, conf);
     assertTrue(result);
-    assertTrue(dest.exists());
+    Verify.exists(dest);
     assertEquals(content.getBytes().length 
         + System.getProperty("line.separator").getBytes().length, dest.length());
-    assertTrue(srcFile.exists()); // should not be deleted
+    Verify.exists(srcFile); // should not be deleted
     
     // copy regular file, delete src:
-    dest.delete();
-    assertTrue(!dest.exists());
+    Verify.delete(dest);
+    Verify.notExists(dest);
     result = FileUtil.copy(fs, srcPath, dest, true, conf);
     assertTrue(result);
-    assertTrue(dest.exists());
+    Verify.exists(dest);
     assertEquals(content.getBytes().length 
         + System.getProperty("line.separator").getBytes().length, dest.length());
-    assertTrue(!srcFile.exists()); // should be deleted
+    Verify.notExists(srcFile); // should be deleted
     
     // copy a dir:
-    dest.delete();
-    assertTrue(!dest.exists());
+    Verify.delete(dest);
+    Verify.notExists(dest);
     srcPath = new Path(partitioned.toURI());
     result = FileUtil.copy(fs, srcPath, dest, true, conf);
     assertTrue(result);
@@ -781,7 +842,7 @@ public class TestFileUtil {
       assertEquals(3 
           + System.getProperty("line.separator").getBytes().length, f.length());
     }
-    assertTrue(!partitioned.exists()); // should be deleted
+    Verify.notExists(partitioned); // should be deleted
   }  
 
   @Test (timeout = 30000)
@@ -869,8 +930,8 @@ public class TestFileUtil {
     // create the symlink
     FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
 
-    Assert.assertTrue(file.exists());
-    Assert.assertTrue(link.exists());
+    Verify.exists(file);
+    Verify.exists(link);
 
     File link2 = new File(del, "_link2");
 
@@ -880,10 +941,10 @@ public class TestFileUtil {
     // Make sure the file still exists
     // (NOTE: this would fail on Java6 on Windows if we didn't
     // copy the file in FileUtil#symlink)
-    Assert.assertTrue(file.exists());
+    Verify.exists(file);
 
-    Assert.assertTrue(link2.exists());
-    Assert.assertFalse(link.exists());
+    Verify.exists(link2);
+    Verify.notExists(link);
   }
 
   /**
@@ -898,13 +959,13 @@ public class TestFileUtil {
     // create the symlink
     FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
 
-    Assert.assertTrue(file.exists());
-    Assert.assertTrue(link.exists());
+    Verify.exists(file);
+    Verify.exists(link);
 
     // make sure that deleting a symlink works properly
-    Assert.assertTrue(link.delete());
-    Assert.assertFalse(link.exists());
-    Assert.assertTrue(file.exists());
+    Verify.delete(link);
+    Verify.notExists(link);
+    Verify.exists(file);
   }
 
   /**
@@ -931,13 +992,13 @@ public class TestFileUtil {
     Assert.assertEquals(data.length, file.length());
     Assert.assertEquals(data.length, link.length());
 
-    file.delete();
-    Assert.assertFalse(file.exists());
+    Verify.delete(file);
+    Verify.notExists(file);
 
     Assert.assertEquals(0, link.length());
 
-    link.delete();
-    Assert.assertFalse(link.exists());
+    Verify.delete(link);
+    Verify.notExists(link);
   }
 
   /**
@@ -1003,7 +1064,7 @@ public class TestFileUtil {
   public void testSymlinkSameFile() throws IOException {
     File file = new File(del, FILE);
 
-    file.delete();
+    Verify.delete(file);
 
     // Create a symbolic link
     // The operation should succeed
@@ -1076,21 +1137,21 @@ public class TestFileUtil {
 
     String parentDir = untarDir.getCanonicalPath() + Path.SEPARATOR + "name";
     File testFile = new File(parentDir + Path.SEPARATOR + "version");
-    Assert.assertTrue(testFile.exists());
+    Verify.exists(testFile);
     Assert.assertTrue(testFile.length() == 0);
     String imageDir = parentDir + Path.SEPARATOR + "image";
     testFile = new File(imageDir + Path.SEPARATOR + "fsimage");
-    Assert.assertTrue(testFile.exists());
+    Verify.exists(testFile);
     Assert.assertTrue(testFile.length() == 157);
     String currentDir = parentDir + Path.SEPARATOR + "current";
     testFile = new File(currentDir + Path.SEPARATOR + "fsimage");
-    Assert.assertTrue(testFile.exists());
+    Verify.exists(testFile);
     Assert.assertTrue(testFile.length() == 4331);
     testFile = new File(currentDir + Path.SEPARATOR + "edits");
-    Assert.assertTrue(testFile.exists());
+    Verify.exists(testFile);
     Assert.assertTrue(testFile.length() == 1033);
     testFile = new File(currentDir + Path.SEPARATOR + "fstime");
-    Assert.assertTrue(testFile.exists());
+    Verify.exists(testFile);
     Assert.assertTrue(testFile.length() == 8);
   }
 
@@ -1151,9 +1212,9 @@ public class TestFileUtil {
     }
 
     // create non-jar files, which we expect to not be included in the classpath
-    Assert.assertTrue(new File(tmp, "text.txt").createNewFile());
-    Assert.assertTrue(new File(tmp, "executable.exe").createNewFile());
-    Assert.assertTrue(new File(tmp, "README").createNewFile());
+    Verify.createNewFile(new File(tmp, "text.txt"));
+    Verify.createNewFile(new File(tmp, "executable.exe"));
+    Verify.createNewFile(new File(tmp, "README"));
 
     // create classpath jar
     String wildcardPath = tmp.getCanonicalPath() + File.separator + "*";
@@ -1239,9 +1300,9 @@ public class TestFileUtil {
     }
 
     // create non-jar files, which we expect to not be included in the result
-    assertTrue(new File(tmp, "text.txt").createNewFile());
-    assertTrue(new File(tmp, "executable.exe").createNewFile());
-    assertTrue(new File(tmp, "README").createNewFile());
+    Verify.createNewFile(new File(tmp, "text.txt"));
+    Verify.createNewFile(new File(tmp, "executable.exe"));
+    Verify.createNewFile(new File(tmp, "README"));
 
     // pass in the directory
     String directory = tmp.getCanonicalPath();
@@ -1275,7 +1336,7 @@ public class TestFileUtil {
       uri4 = new URI(uris4);
       uri5 = new URI(uris5);
       uri6 = new URI(uris6);
-    } catch (URISyntaxException use) {
+    } catch (URISyntaxException ignored) {
     }
     // Set up InetAddress
     inet1 = mock(InetAddress.class);
@@ -1298,7 +1359,7 @@ public class TestFileUtil {
       when(InetAddress.getByName(uris3)).thenReturn(inet3);
       when(InetAddress.getByName(uris4)).thenReturn(inet4);
       when(InetAddress.getByName(uris5)).thenReturn(inet5);
-    } catch (UnknownHostException ue) {
+    } catch (UnknownHostException ignored) {
     }
 
     fs1 = mock(FileSystem.class);
@@ -1318,62 +1379,87 @@ public class TestFileUtil {
   @Test
   public void testCompareFsNull() throws Exception {
     setupCompareFs();
-    assertEquals(FileUtil.compareFs(null,fs1),false);
-    assertEquals(FileUtil.compareFs(fs1,null),false);
+    assertFalse(FileUtil.compareFs(null, fs1));
+    assertFalse(FileUtil.compareFs(fs1, null));
   }
 
   @Test
   public void testCompareFsDirectories() throws Exception {
     setupCompareFs();
-    assertEquals(FileUtil.compareFs(fs1,fs1),true);
-    assertEquals(FileUtil.compareFs(fs1,fs2),false);
-    assertEquals(FileUtil.compareFs(fs1,fs5),false);
-    assertEquals(FileUtil.compareFs(fs3,fs4),true);
-    assertEquals(FileUtil.compareFs(fs1,fs6),false);
+    assertTrue(FileUtil.compareFs(fs1, fs1));
+    assertFalse(FileUtil.compareFs(fs1, fs2));
+    assertFalse(FileUtil.compareFs(fs1, fs5));
+    assertTrue(FileUtil.compareFs(fs3, fs4));
+    assertFalse(FileUtil.compareFs(fs1, fs6));
   }
 
   @Test(timeout = 8000)
   public void testCreateSymbolicLinkUsingJava() throws IOException {
     final File simpleTar = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleTar);
-    TarArchiveOutputStream tos = new TarArchiveOutputStream(os);
-    File untarFile = null;
-    try {
+    try (TarArchiveOutputStream tos = new TarArchiveOutputStream(os)) {
       // Files to tar
       final String tmpDir = "tmp/test";
       File tmpDir1 = new File(tmpDir, "dir1/");
       File tmpDir2 = new File(tmpDir, "dir2/");
-      // Delete the directories if they already exist
-      tmpDir1.mkdirs();
-      tmpDir2.mkdirs();
+      Verify.mkdirs(tmpDir1);
+      Verify.mkdirs(tmpDir2);
 
-      java.nio.file.Path symLink = FileSystems
-          .getDefault().getPath(tmpDir1.getPath() + "/sl");
+      java.nio.file.Path symLink = Paths.get(tmpDir1.getPath(), "sl");
 
       // Create Symbolic Link
-      Files.createSymbolicLink(symLink,
-          FileSystems.getDefault().getPath(tmpDir2.getPath())).toString();
+      Files.createSymbolicLink(symLink, Paths.get(tmpDir2.getPath()));
       assertTrue(Files.isSymbolicLink(symLink.toAbsolutePath()));
-      // put entries in tar file
+      // Put entries in tar file
       putEntriesInTar(tos, tmpDir1.getParentFile());
       tos.close();
 
-      untarFile = new File(tmpDir, "2");
-      // Untar using java
+      File untarFile = new File(tmpDir, "2");
+      // Untar using Java
       FileUtil.unTarUsingJava(simpleTar, untarFile, false);
 
       // Check symbolic link and other directories are there in untar file
       assertTrue(Files.exists(untarFile.toPath()));
-      assertTrue(Files.exists(FileSystems.getDefault().getPath(untarFile
-          .getPath(), tmpDir)));
-      assertTrue(Files.isSymbolicLink(FileSystems.getDefault().getPath(untarFile
-          .getPath().toString(), symLink.toString())));
-
+      assertTrue(Files.exists(Paths.get(untarFile.getPath(), tmpDir)));
+      assertTrue(Files.isSymbolicLink(Paths.get(untarFile.getPath(), symLink.toString())));
     } finally {
       FileUtils.deleteDirectory(new File("tmp"));
-      tos.close();
     }
+  }
+
+  @Test(expected = IOException.class)
+  public void testCreateArbitrarySymlinkUsingJava() throws IOException {
+    final File simpleTar = new File(del, FILE);
+    OutputStream os = new FileOutputStream(simpleTar);
 
+    File rootDir = new File("tmp");
+    try (TarArchiveOutputStream tos = new TarArchiveOutputStream(os)) {
+      tos.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);
+
+      // Create arbitrary dir
+      File arbitraryDir = new File(rootDir, "arbitrary-dir/");
+      Verify.mkdirs(arbitraryDir);
+
+      // We will tar from the tar-root lineage
+      File tarRoot = new File(rootDir, "tar-root/");
+      File symlinkRoot = new File(tarRoot, "dir1/");
+      Verify.mkdirs(symlinkRoot);
+
+      // Create Symbolic Link to an arbitrary dir
+      java.nio.file.Path symLink = Paths.get(symlinkRoot.getPath(), "sl");
+      Files.createSymbolicLink(symLink, arbitraryDir.toPath().toAbsolutePath());
+
+      // Put entries in tar file
+      putEntriesInTar(tos, tarRoot);
+      putEntriesInTar(tos, new File(symLink.toFile(), "dir-outside-tar-root/"));
+      tos.close();
+
+      // Untar using Java
+      File untarFile = new File(rootDir, "extracted");
+      FileUtil.unTarUsingJava(simpleTar, untarFile, false);
+    } finally {
+      FileUtils.deleteDirectory(rootDir);
+    }
   }
 
   private void putEntriesInTar(TarArchiveOutputStream tos, File f)
@@ -1450,7 +1536,7 @@ public class TestFileUtil {
     String result = FileUtil.readLink(file);
     Assert.assertEquals("", result);
 
-    file.delete();
+    Verify.delete(file);
   }
 
   /**

---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


[hadoop] 01/02: HADOOP-16811: Use JUnit TemporaryFolder Rule in TestFileUtils (#1811). Contributed by David Mollitor.

Posted by iw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

iwasakims pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git

commit d09ca8bcebc1cb04c00a047f01bc630b7a99072e
Author: belugabehr <12...@users.noreply.github.com>
AuthorDate: Sat Jan 25 10:12:21 2020 -0500

    HADOOP-16811: Use JUnit TemporaryFolder Rule in TestFileUtils (#1811). Contributed by David Mollitor.
    
    (cherry picked from commit 1afd54fbbb858a58112e6290b3063216eea82206)
---
 .../java/org/apache/hadoop/fs/TestFileUtil.java    | 221 ++++++---------------
 1 file changed, 65 insertions(+), 156 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
index 00d2a74..2a7bfb4 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
@@ -67,22 +67,38 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class TestFileUtil {
   private static final Logger LOG = LoggerFactory.getLogger(TestFileUtil.class);
 
-  private static final File TEST_DIR = GenericTestUtils.getTestDir("fu");
+  @Rule
+  public TemporaryFolder testFolder = new TemporaryFolder();
+
   private static final String FILE = "x";
   private static final String LINK = "y";
   private static final String DIR = "dir";
-  private final File del = new File(TEST_DIR, "del");
-  private final File tmp = new File(TEST_DIR, "tmp");
-  private final File dir1 = new File(del, DIR + "1");
-  private final File dir2 = new File(del, DIR + "2");
-  private final File partitioned = new File(TEST_DIR, "partitioned");
+
+  private static final String FILE_1_NAME = "file1";
+
+  private File del;
+  private File tmp;
+  private File dir1;
+  private File dir2;
+  private File partitioned;
+
+  private File xSubDir;
+  private File xSubSubDir;
+  private File ySubDir;
+
+  private File file2;
+  private File file22;
+  private File file3;
+  private File zlink;
 
   private InetAddress inet1;
   private InetAddress inet2;
@@ -119,21 +135,34 @@ public class TestFileUtil {
    *   file: part-r-00000, contents: "foo"
    *   file: part-r-00001, contents: "bar"
    */
-  @Ignore
-  private void setupDirs() throws IOException {
-    Assert.assertFalse(del.exists());
-    Assert.assertFalse(tmp.exists());
-    Assert.assertFalse(partitioned.exists());
-    del.mkdirs();
-    tmp.mkdirs();
-    partitioned.mkdirs();
+  @Before
+  public void setup() throws IOException {
+    del = testFolder.newFolder("del");
+    tmp = testFolder.newFolder("tmp");
+    partitioned = testFolder.newFolder("partitioned");
+
+    zlink = new File(del, "zlink");
+
+    xSubDir = new File(del, "xSubDir");
+    xSubSubDir = new File(xSubDir, "xSubSubDir");
+    ySubDir = new File(del, "ySubDir");
+
+
+    file2 = new File(xSubDir, "file2");
+    file22 = new File(xSubSubDir, "file22");
+    file3 = new File(ySubDir, "file3");
+
+    dir1 = new File(del, DIR + "1");
+    dir2 = new File(del, DIR + "2");
+
+    FileUtils.forceMkdir(dir1);
+    FileUtils.forceMkdir(dir2);
+
     new File(del, FILE).createNewFile();
     File tmpFile = new File(tmp, FILE);
     tmpFile.createNewFile();
 
-    // create directories 
-    dir1.mkdirs();
-    dir2.mkdirs();
+    // create files
     new File(dir1, FILE).createNewFile();
     new File(dir2, FILE).createNewFile();
 
@@ -154,6 +183,11 @@ public class TestFileUtil {
     FileUtil.symLink(del.toString(), dir1.toString() + "/cycle");
   }
 
+  @After
+  public void tearDown() throws IOException {
+    testFolder.delete();
+  }
+
   /**
    * Creates a new file in the specified directory, with the specified name and
    * the specified file contents.  This method will add a newline terminator to
@@ -178,7 +212,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testListFiles() throws IOException {
-    setupDirs();
     //Test existing files case 
     File[] files = FileUtil.listFiles(partitioned);
     Assert.assertEquals(2, files.length);
@@ -205,7 +238,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testListAPI() throws IOException {
-    setupDirs();
     //Test existing files case 
     String[] files = FileUtil.list(partitioned);
     Assert.assertEquals("Unexpected number of pre-existing files", 2, files.length);
@@ -230,30 +262,8 @@ public class TestFileUtil {
     }
   }
 
-  @Before
-  public void before() throws IOException {
-    cleanupImpl();
-  }
-  
-  @After
-  public void tearDown() throws IOException {
-    cleanupImpl();
-  }
-  
-  private void cleanupImpl() throws IOException  {
-    FileUtil.fullyDelete(del, true);
-    Assert.assertTrue(!del.exists());
-    
-    FileUtil.fullyDelete(tmp, true);
-    Assert.assertTrue(!tmp.exists());
-    
-    FileUtil.fullyDelete(partitioned, true);
-    Assert.assertTrue(!partitioned.exists());
-  }
-
   @Test (timeout = 30000)
   public void testFullyDelete() throws IOException {
-    setupDirs();
     boolean ret = FileUtil.fullyDelete(del);
     Assert.assertTrue(ret);
     Assert.assertFalse(del.exists());
@@ -268,8 +278,6 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testFullyDeleteSymlinks() throws IOException {
-    setupDirs();
-    
     File link = new File(del, LINK);
     Assert.assertEquals(5, del.list().length);
     // Since tmpDir is symlink to tmp, fullyDelete(tmpDir) should not
@@ -298,7 +306,6 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testFullyDeleteDanglingSymlinks() throws IOException {
-    setupDirs();
     // delete the directory tmp to make tmpDir a dangling link to dir tmp and
     // to make y as a dangling link to file tmp/x
     boolean ret = FileUtil.fullyDelete(tmp);
@@ -325,7 +332,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testFullyDeleteContents() throws IOException {
-    setupDirs();
     boolean ret = FileUtil.fullyDeleteContents(del);
     Assert.assertTrue(ret);
     Assert.assertTrue(del.exists());
@@ -339,15 +345,6 @@ public class TestFileUtil {
     Assert.assertTrue(new File(tmp, FILE).exists());
   }
 
-  private final File xSubDir = new File(del, "xSubDir");
-  private final File xSubSubDir = new File(xSubDir, "xSubSubDir");
-  private final File ySubDir = new File(del, "ySubDir");
-  private static final String file1Name = "file1";
-  private final File file2 = new File(xSubDir, "file2");
-  private final File file22 = new File(xSubSubDir, "file22");
-  private final File file3 = new File(ySubDir, "file3");
-  private final File zlink = new File(del, "zlink");
-  
   /**
    * Creates a directory which can not be deleted completely.
    * 
@@ -369,36 +366,30 @@ public class TestFileUtil {
    * @throws IOException
    */
   private void setupDirsAndNonWritablePermissions() throws IOException {
-    Assert.assertFalse("The directory del should not have existed!",
-        del.exists());
-    del.mkdirs();
-    new MyFile(del, file1Name).createNewFile();
+    new MyFile(del, FILE_1_NAME).createNewFile();
 
     // "file1" is non-deletable by default, see MyFile.delete().
 
     xSubDir.mkdirs();
     file2.createNewFile();
-    
+
     xSubSubDir.mkdirs();
     file22.createNewFile();
-    
+
     revokePermissions(file22);
     revokePermissions(xSubSubDir);
-    
+
     revokePermissions(file2);
     revokePermissions(xSubDir);
-    
+
     ySubDir.mkdirs();
     file3.createNewFile();
 
-    Assert.assertFalse("The directory tmp should not have existed!",
-        tmp.exists());
-    tmp.mkdirs();
     File tmpFile = new File(tmp, FILE);
     tmpFile.createNewFile();
     FileUtil.symLink(tmpFile.toString(), zlink.toString());
   }
-  
+
   private static void grantPermissions(final File f) {
     FileUtil.setReadable(f, true);
     FileUtil.setWritable(f, true);
@@ -420,7 +411,7 @@ public class TestFileUtil {
     
     Assert.assertFalse("The return value should have been false.", ret);
     Assert.assertTrue("The file file1 should not have been deleted.",
-        new File(del, file1Name).exists());
+        new File(del, FILE_1_NAME).exists());
     
     Assert.assertEquals(
         "The directory xSubDir *should* not have been deleted.",
@@ -448,7 +439,7 @@ public class TestFileUtil {
     boolean ret = FileUtil.fullyDelete(new MyFile(del));
     validateAndSetWritablePermissions(true, ret);
   }
-  
+
   @Test (timeout = 30000)
   public void testFailFullyDeleteGrantPermissions() throws IOException {
     setupDirsAndNonWritablePermissions();
@@ -485,7 +476,7 @@ public class TestFileUtil {
     public boolean delete() {
       LOG.info("Trying to delete myFile " + getAbsolutePath());
       boolean bool = false;
-      if (getName().equals(file1Name)) {
+      if (getName().equals(FILE_1_NAME)) {
         bool = false;
       } else {
         bool = super.delete();
@@ -535,7 +526,7 @@ public class TestFileUtil {
     // this time the directories with revoked permissions *should* be deleted:
     validateAndSetWritablePermissions(false, ret);
   }
-  
+
   /**
    * Test that getDU is able to handle cycles caused due to symbolic links
    * and that directory sizes are not added to the final calculated size
@@ -543,9 +534,7 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testGetDU() throws Exception {
-    setupDirs();
-
-    long du = FileUtil.getDU(TEST_DIR);
+    long du = FileUtil.getDU(testFolder.getRoot());
     // Only two files (in partitioned).  Each has 3 characters + system-specific
     // line separator.
     final long expected = 2 * (3 + System.getProperty("line.separator").length());
@@ -594,8 +583,6 @@ public class TestFileUtil {
   
   @Test (timeout = 30000)
   public void testUnTar() throws IOException {
-    setupDirs();
-    
     // make a simple tar:
     final File simpleTar = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleTar); 
@@ -632,7 +619,6 @@ public class TestFileUtil {
   
   @Test (timeout = 30000)
   public void testReplaceFile() throws IOException {
-    setupDirs();
     final File srcFile = new File(tmp, "src");
     
     // src exists, and target does not exist:
@@ -674,7 +660,6 @@ public class TestFileUtil {
   
   @Test (timeout = 30000)
   public void testCreateLocalTempFile() throws IOException {
-    setupDirs();
     final File baseFile = new File(tmp, "base");
     File tmp1 = FileUtil.createLocalTempFile(baseFile, "foo", false);
     File tmp2 = FileUtil.createLocalTempFile(baseFile, "foo", true);
@@ -690,8 +675,7 @@ public class TestFileUtil {
   
   @Test (timeout = 30000)
   public void testUnZip() throws IOException {
-    setupDirs();
-    // make a simple zip
+    // make sa simple zip
     final File simpleZip = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleZip); 
     ZipOutputStream tos = new ZipOutputStream(os);
@@ -727,7 +711,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testUnZip2() throws IOException {
-    setupDirs();
     // make a simple zip
     final File simpleZip = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleZip);
@@ -758,8 +741,6 @@ public class TestFileUtil {
    * Test method copy(FileSystem srcFS, Path src, File dst, boolean deleteSource, Configuration conf)
    */
   public void testCopy5() throws IOException {
-    setupDirs();
-    
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.newInstance(uri, conf);
@@ -849,9 +830,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testSymlink() throws Exception {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     byte[] data = "testSymLink".getBytes();
 
     File file = new File(del, FILE);
@@ -884,9 +862,6 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testSymlinkRenameTo() throws Exception {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
     file.createNewFile();
     File link = new File(del, "_link");
@@ -916,9 +891,6 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testSymlinkDelete() throws Exception {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
     file.createNewFile();
     File link = new File(del, "_link");
@@ -940,9 +912,6 @@ public class TestFileUtil {
    */
   @Test (timeout = 30000)
   public void testSymlinkLength() throws Exception {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     byte[] data = "testSymLinkData".getBytes();
 
     File file = new File(del, FILE);
@@ -979,9 +948,6 @@ public class TestFileUtil {
    */
   @Test
   public void testSymlinkWithNullInput() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
     File link = new File(del, "_link");
 
@@ -999,9 +965,6 @@ public class TestFileUtil {
     // The operation should fail and returns 1
     result = FileUtil.symLink(null, link.getAbsolutePath());
     Assert.assertEquals(1, result);
-
-    file.delete();
-    link.delete();
   }
 
   /**
@@ -1012,9 +975,6 @@ public class TestFileUtil {
    */
   @Test
   public void testSymlinkFileAlreadyExists() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
     File link = new File(del, "_link");
 
@@ -1030,9 +990,6 @@ public class TestFileUtil {
     result1 = FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
 
     Assert.assertEquals(1, result1);
-
-    file.delete();
-    link.delete();
   }
 
   /**
@@ -1044,19 +1001,16 @@ public class TestFileUtil {
    */
   @Test
   public void testSymlinkSameFile() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
 
+    file.delete();
+
     // Create a symbolic link
     // The operation should succeed
     int result =
         FileUtil.symLink(file.getAbsolutePath(), file.getAbsolutePath());
 
     Assert.assertEquals(0, result);
-
-    file.delete();
   }
 
   /**
@@ -1068,8 +1022,6 @@ public class TestFileUtil {
    */
   @Test
   public void testSymlink2DifferentFile() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
     File file = new File(del, FILE);
     File fileSecond = new File(del, FILE + "_1");
     File link = new File(del, "_link");
@@ -1086,10 +1038,6 @@ public class TestFileUtil {
         FileUtil.symLink(fileSecond.getAbsolutePath(), link.getAbsolutePath());
 
     Assert.assertEquals(1, result);
-
-    file.delete();
-    fileSecond.delete();
-    link.delete();
   }
 
   /**
@@ -1101,8 +1049,6 @@ public class TestFileUtil {
    */
   @Test
   public void testSymlink2DifferentLinks() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
     File file = new File(del, FILE);
     File link = new File(del, "_link");
     File linkSecond = new File(del, "_link_1");
@@ -1119,10 +1065,6 @@ public class TestFileUtil {
         FileUtil.symLink(file.getAbsolutePath(), linkSecond.getAbsolutePath());
 
     Assert.assertEquals(0, result);
-
-    file.delete();
-    link.delete();
-    linkSecond.delete();
   }
 
   private void doUntarAndVerify(File tarFile, File untarDir) 
@@ -1199,10 +1141,6 @@ public class TestFileUtil {
 
   @Test (timeout = 30000)
   public void testCreateJarWithClassPath() throws Exception {
-    // setup test directory for files
-    Assert.assertFalse(tmp.exists());
-    Assert.assertTrue(tmp.mkdirs());
-
     // create files expected to match a wildcard
     List<File> wildcardMatches = Arrays.asList(new File(tmp, "wildcard1.jar"),
       new File(tmp, "wildcard2.jar"), new File(tmp, "wildcard3.JAR"),
@@ -1291,9 +1229,6 @@ public class TestFileUtil {
     assertTrue("no jars should be returned for a bogus path",
         jars.isEmpty());
 
-    // setup test directory for files
-    assertFalse(tmp.exists());
-    assertTrue(tmp.mkdirs());
 
     // create jar files to be returned
     File jar1 = new File(tmp, "wildcard1.jar");
@@ -1399,7 +1334,6 @@ public class TestFileUtil {
 
   @Test(timeout = 8000)
   public void testCreateSymbolicLinkUsingJava() throws IOException {
-    setupDirs();
     final File simpleTar = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleTar);
     TarArchiveOutputStream tos = new TarArchiveOutputStream(os);
@@ -1493,9 +1427,6 @@ public class TestFileUtil {
    */
   @Test
   public void testReadSymlink() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
     File link = new File(del, "_link");
 
@@ -1504,9 +1435,6 @@ public class TestFileUtil {
 
     String result = FileUtil.readLink(link);
     Assert.assertEquals(file.getAbsolutePath(), result);
-
-    file.delete();
-    link.delete();
   }
 
   /**
@@ -1517,9 +1445,6 @@ public class TestFileUtil {
    */
   @Test
   public void testReadSymlinkWithAFileAsInput() throws IOException {
-    Assert.assertFalse(del.exists());
-    del.mkdirs();
-
     File file = new File(del, FILE);
 
     String result = FileUtil.readLink(file);
@@ -1533,8 +1458,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteBytesFileSystem() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.get(uri, conf);
@@ -1555,8 +1478,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringsFileSystem() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.get(uri, conf);
@@ -1577,8 +1498,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringFileSystem() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.get(uri, conf);
@@ -1600,8 +1519,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringNoCharSetFileSystem() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.get(uri, conf);
@@ -1621,8 +1538,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteBytesFileContext() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileContext fc = FileContext.getFileContext(uri, conf);
@@ -1643,8 +1558,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringsFileContext() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileContext fc = FileContext.getFileContext(uri, conf);
@@ -1665,8 +1578,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringFileContext() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileContext fc = FileContext.getFileContext(uri, conf);
@@ -1688,8 +1599,6 @@ public class TestFileUtil {
    */
   @Test
   public void testWriteStringNoCharSetFileContext() throws IOException {
-    setupDirs();
-
     URI uri = tmp.toURI();
     Configuration conf = new Configuration();
     FileContext fc = FileContext.getFileContext(uri, conf);

---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org