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

(cherry picked from commit 9144fd9e9b)
(cherry picked from commit 2653cf4466)
This commit is contained in:
Akira Ajisaka 2017-07-12 11:35:50 +09:00
parent 83eacccc17
commit d1fb4948d2
No known key found for this signature in database
GPG Key ID: C1EDBB9CA400FD50
2 changed files with 56 additions and 55 deletions

View File

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

View File

@ -33,6 +33,7 @@ import org.apache.hadoop.hdfs.DistributedFileSystem;
import org.apache.hadoop.hdfs.HdfsConfiguration; import org.apache.hadoop.hdfs.HdfsConfiguration;
import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.hdfs.protocol.HdfsConstants; import org.apache.hadoop.hdfs.protocol.HdfsConstants;
import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.ipc.RemoteException;
import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.UserGroupInformation;
import org.junit.After; import org.junit.After;
@ -199,9 +200,9 @@ public class TestHDFSFileContextMainOperations extends
* accommodates rename * accommodates rename
*/ */
// rename uses dstdir quota=1 // 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 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 * 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 // dstDir quota = 1 and dst1 already uses it
createFile(src2); 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 * 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 // src1 has no quota to accommodate new rename node
fs.setQuota(src1.getParent(), 1, HdfsConstants.QUOTA_DONT_SET); 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 * 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); fs.setQuota(src1.getParent(), 100, HdfsConstants.QUOTA_DONT_SET);
createFile(src1); createFile(src1);
fs.setQuota(src1.getParent(), 1, HdfsConstants.QUOTA_DONT_SET); 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 { public void testRenameRoot() throws Exception {
Path src = getTestRootPath(fc, "test/testRenameRoot/srcdir/src1"); Path src = getTestRootPath(fc, "test/testRenameRoot/srcdir/src1");
Path dst = new Path("/"); Path dst = new Path("/");
createFile(src); createFile(src);
rename(src, dst, true, false, true, Rename.OVERWRITE); rename(dst, src, true, true, Rename.OVERWRITE);
rename(dst, src, true, false, 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); fs.setQuota(dst1.getParent(), 2, HdfsConstants.QUOTA_DONT_SET);
// Free up quota for a subsequent rename // Free up quota for a subsequent rename
fs.delete(dst1, true); 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 // Restart the cluster and ensure the above operations can be
// loaded from the edits log // loaded from the edits log
@ -325,19 +340,6 @@ public class TestHDFSFileContextMainOperations extends
Assert.assertEquals(renameSucceeds, exists(fc, dst)); 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 @Override
protected boolean listCorruptedBlocksSupported() { protected boolean listCorruptedBlocksSupported() {
return true; return true;