Pull checkstyle#3259: Avoid catching of run-time exception in Propert…

…yCacheFile#persist
MEZk · Jun 9, 2016 · 6aaaacd · 6aaaacd
1 parent 05287de
commit 6aaaacd
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 60 deletions.
diff --git a/config/sevntu_suppressions.xml b/config/sevntu_suppressions.xml
@@ -55,5 +55,5 @@
     <!-- testing for catch Error is part of 100% coverage -->
     <suppress checks="IllegalCatchExtended"
               files="CheckerTest\.java"
-              lines="579"/>
+              lines="545"/>
 </suppressions>
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java
@@ -32,7 +32,6 @@
 import java.net.URI;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.MessageDigest;
@@ -144,7 +143,7 @@ public void persist() throws IOException {
                 Files.createDirectories(directory);
             }
         }
-        catch (InvalidPathException | AccessDeniedException ex) {
+        catch (AccessDeniedException ex) {
             throw new IllegalStateException(ex.getMessage(), ex);
         }
         FileOutputStream out = null;

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java
@@ -19,7 +19,6 @@
 
 package com.puppycrawl.tools.checkstyle;
 
-import static org.hamcrest.CoreMatchers.anyOf;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -33,15 +32,13 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Method;
-import java.nio.file.InvalidPathException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 import java.util.SortedSet;
 
-import org.junit.Assume;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -363,20 +360,17 @@ public void testSetupChildListener() throws Exception {
     }
 
     @Test
-    public void testDestroyNonExistingCache() throws Exception {
-        // We use assumption to satisfy coverage rate on OS Windows, since persist() method of
-        // class PropertyCacheFile does not throw IOException on OS Linux when path to a cache
-        // directory is invalid on OS Windows.
-        Assume.assumeTrue(System.getProperty("os.name")
-            .toLowerCase(Locale.ENGLISH).startsWith("windows"));
-
+    public void testDestroyCacheFileWithWrongFileNameLength() throws Exception {
         final Checker checker = new Checker();
         final PackageObjectFactory factory = new PackageObjectFactory(
             new HashSet<String>(), Thread.currentThread().getContextClassLoader());
         checker.setModuleFactory(factory);
         checker.configure(new DefaultConfiguration("default config"));
-        final String tempFilePath = temporaryFolder.newFile().getPath() + ".\\\'";
-        checker.setCacheFile(tempFilePath);
+        // The maximum file name length which is allowed in Windows (NTFS),
+        // most UNIX file systems, Mac OS HFS is 255 chars.
+        // See https://en.wikipedia.org/wiki/Filename
+        final int wrongFileNameLength = 300;
+        checker.setCacheFile(Strings.padEnd(File.separator, wrongFileNameLength, '*'));
         try {
             checker.destroy();
             fail("Exception did not happen");
@@ -386,34 +380,6 @@ public void testDestroyNonExistingCache() throws Exception {
         }
     }
 
-    @Test
-    public void testDestroyCacheFileWithInvalidPath() throws Exception {
-        final Checker checker = new Checker();
-        final PackageObjectFactory factory = new PackageObjectFactory(
-            new HashSet<String>(), Thread.currentThread().getContextClassLoader());
-        checker.setModuleFactory(factory);
-        checker.configure(new DefaultConfiguration("default config"));
-        if (System.getProperty("os.name")
-            .toLowerCase(Locale.ENGLISH).startsWith("windows")) {
-            // https://support.microsoft.com/en-us/kb/177506 but this only for NTFS
-            // WindowsServer 2012 use Resilient File System (ReFS), so any name is ok
-            final File file = new File("C\\:invalid");
-            checker.setCacheFile(file.getAbsolutePath());
-        }
-        else {
-            final int wrongFileNameLength = 300;
-            checker.setCacheFile(Strings.padEnd(File.separator, wrongFileNameLength, '*'));
-        }
-        try {
-            checker.destroy();
-            fail("Exception did not happen");
-        }
-        catch (IllegalStateException ex) {
-            assertThat(ex.getCause(), anyOf(instanceOf(IOException.class),
-                instanceOf(InvalidPathException.class)));
-        }
-    }
-
     @Test
     public void testCacheFile() throws Exception {
         final DefaultConfiguration checkConfig = createCheckConfig(HiddenFieldCheck.class);

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java
@@ -20,7 +20,6 @@
 package com.puppycrawl.tools.checkstyle;
 
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.core.StringEndsWith.endsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -40,7 +39,6 @@
 import java.lang.reflect.Method;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
 import java.nio.file.Paths;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
@@ -137,21 +135,6 @@ public void testPathToCacheContainsOnlyFileName() throws IOException {
         }
     }
 
-    @Test
-    public void testPathToCacheFileContainsIllegalCharacters() throws IOException {
-        final Configuration config = new DefaultConfiguration("myName");
-        final String filePath = "\\\0:FOO\\server.properties";
-        final PropertyCacheFile cache = new PropertyCacheFile(config, filePath);
-        try {
-            cache.persist();
-            fail("Exception is expected!");
-        }
-        catch (IllegalStateException ex) {
-            assertThat(ex.getCause(), instanceOf(InvalidPathException.class));
-            assertThat(ex.getMessage(), endsWith("server.properties"));
-        }
-    }
-
     @Test
     public void testNonAccessibleDirectory() throws Exception {