HADOOP-14629. Improve exception checking in FileContext related JUnit tests. Contributed by Andras Bokor.

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java

@@ -24,7 +24,6 @@ import java.io.IOException;
 import java.util.EnumSet;
 import java.util.NoSuchElementException;
 
-import org.apache.commons.lang.RandomStringUtils;
 import org.apache.hadoop.HadoopIllegalArgumentException;
 import org.apache.hadoop.fs.Options.CreateOpts;
 import org.apache.hadoop.fs.Options.Rename;
@@ -881,14 +880,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path src = getTestRootPath(fc, "test/hadoop/nonExistent");
     Path dst = getTestRootPath(fc, "test/new/newpath");
     try {
-      rename(src, dst, false, false, false, Rename.NONE);
+      rename(src, dst, false, false, Rename.NONE);
       Assert.fail("Should throw FileNotFoundException");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
     }
 
     try {
-      rename(src, dst, false, false, false, Rename.OVERWRITE);
+      rename(src, dst, false, false, Rename.OVERWRITE);
       Assert.fail("Should throw FileNotFoundException");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
@@ -904,14 +903,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path dst = getTestRootPath(fc, "test/nonExistent/newfile");
     
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, false, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
     }
 
     try {
-      rename(src, dst, false, true, false, Rename.OVERWRITE);
+      rename(src, dst, true, false, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
@@ -928,13 +927,13 @@ public abstract class FileContextMainOperationsBaseTest  {
     createFile(dst.getParent());
     
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, false, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
     }
 
     try {
-      rename(src, dst, false, true, false, Rename.OVERWRITE);
+      rename(src, dst, true, false, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
     }
@@ -948,7 +947,7 @@ public abstract class FileContextMainOperationsBaseTest  {
     createFile(src);
     Path dst = getTestRootPath(fc, "test/new/newfile");
     fc.mkdir(dst.getParent(), FileContext.DEFAULT_PERM, true);
-    rename(src, dst, true, false, true, Rename.OVERWRITE);
+    rename(src, dst, false, true, Rename.OVERWRITE);
   }
 
   @Test
@@ -957,14 +956,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path src = getTestRootPath(fc, "test/hadoop/file");
     createFile(src);
     try {
-      rename(src, src, false, true, false, Rename.NONE);
+      rename(src, src, true, true, Rename.NONE);
       Assert.fail("Renamed file to itself");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
     }
     // Also fails with overwrite
     try {
-      rename(src, src, false, true, false, Rename.OVERWRITE);
+      rename(src, src, true, true, Rename.OVERWRITE);
       Assert.fail("Renamed file to itself");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
@@ -982,14 +981,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     
     // Fails without overwrite option
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, true, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
     }
     
     // Succeeds with overwrite option
-    rename(src, dst, true, false, true, Rename.OVERWRITE);
+    rename(src, dst, false, true, Rename.OVERWRITE);
   }
 
   @Test
@@ -1003,14 +1002,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     
     // Fails without overwrite option
     try {
-      rename(src, dst, false, false, true, Rename.NONE);
+      rename(src, dst, true, true, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
     }
     
     // File cannot be renamed as directory
     try {
-      rename(src, dst, false, false, true, Rename.OVERWRITE);
+      rename(src, dst, true, true, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
     }
@@ -1022,14 +1021,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path src = getTestRootPath(fc, "test/hadoop/dir");
     fc.mkdir(src, FileContext.DEFAULT_PERM, true);
     try {
-      rename(src, src, false, true, false, Rename.NONE);
+      rename(src, src, true, true, Rename.NONE);
       Assert.fail("Renamed directory to itself");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
     }
     // Also fails with overwrite
     try {
-      rename(src, src, false, true, false, Rename.OVERWRITE);
+      rename(src, src, true, true, Rename.OVERWRITE);
       Assert.fail("Renamed directory to itself");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);      
@@ -1045,14 +1044,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path dst = getTestRootPath(fc, "test/nonExistent/newdir");
     
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, false, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
     }
 
     try {
-      rename(src, dst, false, true, false, Rename.OVERWRITE);
+      rename(src, dst, true, false, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       Assert.assertTrue(unwrapException(e) instanceof FileNotFoundException);
@@ -1077,7 +1076,7 @@ public abstract class FileContextMainOperationsBaseTest  {
     Path dst = getTestRootPath(fc, "test/new/newdir");
     fc.mkdir(dst.getParent(), FileContext.DEFAULT_PERM, true);
     
-    rename(src, dst, true, false, true, options);
+    rename(src, dst, false, true, options);
     Assert.assertFalse("Nested file1 exists", 
         exists(fc, getTestRootPath(fc, "test/hadoop/dir/file1")));
     Assert.assertFalse("Nested file2 exists", 
@@ -1102,14 +1101,14 @@ public abstract class FileContextMainOperationsBaseTest  {
 
     // Fails without overwrite option
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, true, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       // Expected (cannot over-write non-empty destination)
       Assert.assertTrue(unwrapException(e) instanceof FileAlreadyExistsException);
     }
     // Succeeds with the overwrite option
-    rename(src, dst, true, false, true, Rename.OVERWRITE);
+    rename(src, dst, false, true, Rename.OVERWRITE);
   }
 
   @Test
@@ -1126,7 +1125,7 @@ public abstract class FileContextMainOperationsBaseTest  {
     createFile(getTestRootPath(fc, "test/new/newdir/file1"));
     // Fails without overwrite option
     try {
-      rename(src, dst, false, true, false, Rename.NONE);
+      rename(src, dst, true, true, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
       // Expected (cannot over-write non-empty destination)
@@ -1134,7 +1133,7 @@ public abstract class FileContextMainOperationsBaseTest  {
     }
     // Fails even with the overwrite option
     try {
-      rename(src, dst, false, true, false, Rename.OVERWRITE);
+      rename(src, dst, true, true, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException ex) {
       // Expected (cannot over-write non-empty destination)
@@ -1151,13 +1150,13 @@ public abstract class FileContextMainOperationsBaseTest  {
     createFile(dst);
     // Fails without overwrite option
     try {
-      rename(src, dst, false, true, true, Rename.NONE);
+      rename(src, dst, true, true, Rename.NONE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException e) {
     }
     // Directory cannot be renamed as existing file
     try {
-      rename(src, dst, false, true, true, Rename.OVERWRITE);
+      rename(src, dst, true, true, Rename.OVERWRITE);
       Assert.fail("Expected exception was not thrown");
     } catch (IOException ex) {
     }
@@ -1219,14 +1218,14 @@ public abstract class FileContextMainOperationsBaseTest  {
     out.close();
   }
 
-  private void rename(Path src, Path dst, boolean renameShouldSucceed,
-      boolean srcExists, boolean dstExists, Rename... options)
-      throws IOException {
-    fc.rename(src, dst, options);
-    if (!renameShouldSucceed)
-      Assert.fail("rename should have thrown exception");
-    Assert.assertEquals("Source exists", srcExists, exists(fc, src));
-    Assert.assertEquals("Destination exists", dstExists, exists(fc, dst));
+  protected void rename(Path src, Path dst, boolean srcExists,
+      boolean dstExists, Rename... options) throws IOException {
+    try {
+      fc.rename(src, dst, options);
+    } finally {
+      Assert.assertEquals("Source exists", srcExists, exists(fc, src));
+      Assert.assertEquals("Destination exists", dstExists, exists(fc, dst));
+    }
   }
   
   private boolean containsPath(Path path, FileStatus[] filteredPaths)

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestHDFSFileContextMainOperations.java

@@ -33,6 +33,7 @@ import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.HdfsConfiguration;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
+import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.junit.After;
@@ -199,9 +200,9 @@ public class TestHDFSFileContextMainOperations extends
      * accommodates rename
      */
     // rename uses dstdir quota=1
-    rename(src1, dst1, false, true, false, Rename.NONE);
+    rename(src1, dst1, false, true, Rename.NONE);
     // rename reuses dstdir quota=1
-    rename(src2, dst1, true, true, false, Rename.OVERWRITE);
+    rename(src2, dst1, false, true, Rename.OVERWRITE);
 
     /*
      * Test2: src does not exceed quota and dst has *no* quota to accommodate 
@@ -209,7 +210,10 @@ public class TestHDFSFileContextMainOperations extends
      */
     // dstDir quota = 1 and dst1 already uses it
     createFile(src2);
-    rename(src2, dst2, false, false, true, Rename.NONE);
+    try {
+      rename(src2, dst2, true, false, Rename.NONE);
+      fail("NSQuotaExceededException excepted");
+    } catch (NSQuotaExceededException e) {}
 
     /*
      * Test3: src exceeds quota and dst has *no* quota to accommodate rename
@@ -217,7 +221,11 @@ public class TestHDFSFileContextMainOperations extends
      */
     // src1 has no quota to accommodate new rename node
     fs.setQuota(src1.getParent(), 1, HdfsConstants.QUOTA_DONT_SET);
-    rename(dst1, src1, false, false, true, Rename.NONE);
+
+    try {
+      rename(dst1, src1, true, false, Rename.NONE);
+      fail("NSQuotaExceededException excepted");
+    } catch (NSQuotaExceededException e) {}
     
     /*
      * Test4: src exceeds quota and dst has *no* quota to accommodate rename
@@ -228,16 +236,23 @@ public class TestHDFSFileContextMainOperations extends
     fs.setQuota(src1.getParent(), 100, HdfsConstants.QUOTA_DONT_SET);
     createFile(src1);
     fs.setQuota(src1.getParent(), 1, HdfsConstants.QUOTA_DONT_SET);
-    rename(dst1, src1, true, true, false, Rename.OVERWRITE);
+    rename(dst1, src1, false, true, Rename.OVERWRITE);
   }
   
-  @Test
+  @Test(expected = RemoteException.class)
   public void testRenameRoot() throws Exception {
     Path src = getTestRootPath(fc, "test/testRenameRoot/srcdir/src1");
     Path dst = new Path("/");
     createFile(src);
-    rename(src, dst, true, false, true, Rename.OVERWRITE);
-    rename(dst, src, true, false, true, Rename.OVERWRITE);
+    rename(dst, src, true, true, Rename.OVERWRITE);
+  }
+
+  @Test(expected = RemoteException.class)
+  public void testRenameToRoot() throws Exception {
+    Path src = getTestRootPath(fc, "test/testRenameRoot/srcdir/src1");
+    Path dst = new Path("/");
+    createFile(src);
+    rename(src, dst, true, true, Rename.OVERWRITE);
   }
   
   /**
@@ -286,7 +301,7 @@ public class TestHDFSFileContextMainOperations extends
     fs.setQuota(dst1.getParent(), 2, HdfsConstants.QUOTA_DONT_SET);
     // Free up quota for a subsequent rename
     fs.delete(dst1, true);
-    rename(src1, dst1, true, true, false, Rename.OVERWRITE);
+    rename(src1, dst1, false, true, Rename.OVERWRITE);
     
     // Restart the cluster and ensure the above operations can be
     // loaded from the edits log
@@ -325,19 +340,6 @@ public class TestHDFSFileContextMainOperations extends
     Assert.assertEquals(renameSucceeds, exists(fc, dst));
   }
   
-  private void rename(Path src, Path dst, boolean dstExists,
-      boolean renameSucceeds, boolean exception, Options.Rename... options)
-      throws Exception {
-    try {
-      fc.rename(src, dst, options);
-      Assert.assertTrue(renameSucceeds);
-    } catch (Exception ex) {
-      Assert.assertTrue(exception);
-    }
-    Assert.assertEquals(renameSucceeds, !exists(fc, src));
-    Assert.assertEquals((dstExists||renameSucceeds), exists(fc, dst));
-  }
-  
   @Override
   protected boolean listCorruptedBlocksSupported() {
     return true;