[findbugs] Do not ignore exceptional return value

java.io.File.delete() reports failure as an exceptional
return value false. Fix the code which silently ignored
this exceptional return value. Also remove some duplicate
deletion helper methods.

Change-Id: I80ed20ca1f07a2bc6e779957a4ad0c713789c5be
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

6 files changed, 17 insertions, 13 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java
index 4357f8b0c1..556fc12086 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/AddCommandTest.java
@@ -57,6 +57,7 @@ import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.util.FileUtils;
 
 public class AddCommandTest extends RepositoryTestCase {
 
@@ -172,7 +173,7 @@ public class AddCommandTest extends RepositoryTestCase {
 		DirCache dc = git.add().addFilepattern("a.txt").call();
 
 		dc.getEntry(0).getObjectId();
-		file.delete();
+		FileUtils.delete(file);
 
 		// is supposed to do nothing
 		dc = git.add().addFilepattern("a.txt").call();
@@ -195,7 +196,7 @@ public class AddCommandTest extends RepositoryTestCase {
 		git.commit().setMessage("commit a.txt").call();
 
 		dc.getEntry(0).getObjectId();
-		file.delete();
+		FileUtils.delete(file);
 
 		// is supposed to do nothing
 		dc = git.add().addFilepattern("a.txt").call();
@@ -392,7 +393,7 @@ public class AddCommandTest extends RepositoryTestCase {
 		writer.close();
 
 		// file sub/b.txt is deleted
-		file2.delete();
+		FileUtils.delete(file2);
 
 		git.add().addFilepattern("sub").call();
 		// change in sub/a.txt is staged
@@ -444,7 +445,7 @@ public class AddCommandTest extends RepositoryTestCase {
 		writer.print("modified content");
 		writer.close();
 
-		file2.delete();
+		FileUtils.delete(file2);
 
 		// change in sub/a.txt is staged
 		// deletion of sub/b.txt is staged
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java
index ea7820428f..f5b6bfb640 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.lib.RepositoryTestCase;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.WorkingTreeIterator;
+import org.eclipse.jgit.util.FileUtils;
 
 /**
  * Tests ignore node behavior on the local filesystem.
@@ -128,7 +129,7 @@ public class IgnoreNodeTest extends RepositoryTestCase {
 		assertEntry(F, tracked, ".gitignore");
 		assertEntry(F, tracked, "out");
 
-		new File(trash, "out").delete();
+		FileUtils.delete(new File(trash, "out"));
 		writeTrashFile("out/foo", "");
 
 		beginWalk();
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
index cb82389f6a..b4732e2dc1 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ReadTreeTest.java
@@ -200,7 +200,7 @@ public abstract class ReadTreeTest extends RepositoryTestCase {
 
 		// rule 11
 		setupCase(headMap, null, idxMap);
-		new File(trash, "foo").delete();
+		assertTrue(new File(trash, "foo").delete());
 		writeTrashFile("foo", "bar");
 		db.getIndex().getMembers()[0].forceRecheck();
 		go();
@@ -238,7 +238,7 @@ public abstract class ReadTreeTest extends RepositoryTestCase {
 
 		// rules 21
 		setupCase(idxMap, mergeMap, idxMap);
-		new File(trash, "foo").delete();
+		assertTrue(new File(trash, "foo").delete());
 		writeTrashFile("foo", "bar");
 		db.getIndex().getMembers()[0].forceRecheck();
 		go();
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
index 30cec12798..2e850ce9fb 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
@@ -62,6 +62,7 @@ import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
+import org.eclipse.jgit.util.FileUtils;
 
 /**
  * Base class for most JGit unit tests.
@@ -98,8 +99,7 @@ public abstract class RepositoryTestCase extends LocalDiskRepositoryTestCase {
 
 	protected void deleteTrashFile(final String name) throws IOException {
 		File path = new File(db.getWorkTree(), name);
-		if (!path.delete())
-			throw new IOException("Could not delete file " + path.getPath());
+		FileUtils.delete(path);
 	}
 
 	protected static void checkFile(File f, final String checkData)
@@ -323,7 +323,7 @@ public abstract class RepositoryTestCase extends LocalDiskRepositoryTestCase {
 			}
 			return actTime;
 		} finally {
-			tmp.delete();
+			FileUtils.delete(tmp);
 		}
 	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
index 7608a51f22..66b02cb46a 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
@@ -64,6 +64,7 @@ import org.eclipse.jgit.lib.RepositoryTestCase;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.pack.PackWriter;
+import org.eclipse.jgit.util.FileUtils;
 
 public class ConcurrentRepackTest extends RepositoryTestCase {
 	public void setUp() throws Exception {
@@ -236,10 +237,10 @@ public class ConcurrentRepackTest extends RepositoryTestCase {
 		touch(begin, files[0].getParentFile());
 	}
 
-	private static void delete(final File[] list) {
+	private static void delete(final File[] list) throws IOException {
 		final long begin = list[0].getParentFile().lastModified();
 		for (final File f : list) {
-			f.delete();
+			FileUtils.delete(f);
 			assertFalse(f + " was removed", f.exists());
 		}
 		touch(begin, list[0].getParentFile());
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
index 838a56c13f..ae519f4eab 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.util.FileUtils;
 import org.eclipse.jgit.util.RawParseUtils;
 
 public class FileTreeIteratorTest extends RepositoryTestCase {
@@ -170,7 +171,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase {
 
 		// Verify it was cached by removing the file and getting it again.
 		//
-		new File(trash, paths[0]).delete();
+		FileUtils.delete(new File(trash, paths[0]));
 		assertEquals(expect, top.getEntryObjectId());
 	}