handle non archive files on classpath

pitest/src/main/java/org/pitest/classpath/ArchiveClassPathRoot.java

@@ -21,9 +21,11 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
 
 import java.util.Optional;
@@ -43,37 +45,30 @@ public ArchiveClassPathRoot(final File file) {
 
   @Override
   public InputStream getData(final String name) throws IOException {
-    try (ZipFile zip = getRoot()) {
+    try (ZipHandle zip = getRoot()) {
       final ZipEntry entry = zip.getEntry(name.replace('.', '/') + ".class");
       if (entry == null) {
         return null;
       }
       return StreamUtil.copyStream(zip.getInputStream(entry));
+    } catch (Exception e) {
+      throw Unchecked.translateCheckedException(e);
     }
   }
 
   @Override
   public URL getResource(final String name) throws MalformedURLException {
-    final ZipFile zip = getRoot();
-    try {
+    try (ZipHandle zip = getRoot()) {
       final ZipEntry entry = zip.getEntry(name);
       if (entry != null) {
         return new URL("jar:file:" + zip.getName() + "!/" + entry.getName());
       } else {
         return null;
       }
-    } finally {
-      closeQuietly(zip);
-    }
-
-  }
-
-  private static void closeQuietly(final ZipFile zip) {
-    try {
-      zip.close();
-    } catch (final IOException e) {
+    } catch (Exception e) {
       throw Unchecked.translateCheckedException(e);
     }
+
   }
 
   @Override
@@ -84,8 +79,7 @@ public String toString() {
   @Override
   public Collection<String> classNames() {
     final List<String> names = new ArrayList<>();
-    final ZipFile root = getRoot();
-    try {
+    try (ZipHandle root = getRoot()) {
       final Enumeration<? extends ZipEntry> entries = root.entries();
       while (entries.hasMoreElements()) {
         final ZipEntry entry = entries.nextElement();
@@ -94,8 +88,8 @@ public Collection<String> classNames() {
         }
       }
       return names;
-    } finally {
-      closeQuietly(root);
+    } catch (Exception e) {
+      throw Unchecked.translateCheckedException(e);
     }
 
   }
@@ -110,13 +104,90 @@ public Optional<String> cacheLocation() {
     return Optional.ofNullable(this.file.getAbsolutePath());
   }
 
-  private ZipFile getRoot() {
+  private ZipHandle getRoot() {
     try {
-      return new ZipFile(this.file);
-    } catch (final IOException ex) {
+      return new WrappedZip(new ZipFile(this.file));
+    } catch (ZipException ex) {
+      // We might be passed files that are not archives on the classpath
+      // rather than trying to filter these out by naming convention we've opted to
+      // handle the error quietly here
+      return new NotAZip();
+    } catch (IOException ex) {
       throw Unchecked.translateCheckedException(ex.getMessage() + " ("
           + this.file + ")", ex);
     }
   }
 
 }
+
+interface ZipHandle extends AutoCloseable {
+  ZipEntry getEntry(String name);
+
+  Enumeration<? extends ZipEntry> entries();
+
+  String getName();
+
+  InputStream getInputStream(ZipEntry entry) throws IOException;
+}
+
+
+class WrappedZip implements ZipHandle {
+
+  private final ZipFile zip;
+
+  WrappedZip(ZipFile zip) {
+    this.zip = zip;
+  }
+
+  @Override
+  public ZipEntry getEntry(String name) {
+    return zip.getEntry(name);
+  }
+
+  @Override
+  public Enumeration<? extends ZipEntry> entries() {
+    return zip.entries();
+  }
+
+  @Override
+  public String getName() {
+    return zip.getName();
+  }
+
+  @Override
+  public InputStream getInputStream(ZipEntry entry) throws IOException {
+    return zip.getInputStream(entry);
+  }
+
+  @Override
+  public void close() throws Exception {
+    zip.close();
+  }
+}
+
+class NotAZip implements ZipHandle {
+
+  @Override
+  public ZipEntry getEntry(String name) {
+    return null;
+  }
+
+  @Override
+  public Enumeration<? extends ZipEntry> entries() {
+    return Collections.emptyEnumeration();
+  }
+
+  @Override
+  public String getName() {
+    return "";
+  }
+
+  @Override
+  public InputStream getInputStream(ZipEntry entry) {
+    return null;
+  }
+
+  @Override
+  public void close() throws Exception {
+  }
+}

pitest/src/test/java/org/pitest/classpath/ClassPathTest.java

@@ -14,21 +14,27 @@
  */
 package org.pitest.classpath;
 
+import static java.util.Arrays.asList;
 import static java.util.function.Predicate.isEqual;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.function.Predicate;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
@@ -44,6 +50,9 @@ public class ClassPathTest {
   @Mock
   private ClassPathRoot secondComponent;
 
+  @Rule
+  public TemporaryFolder dir = new TemporaryFolder();
+
   @Before
   public void setUp() {
     MockitoAnnotations.openMocks(this);
@@ -68,13 +77,13 @@ public void shouldReturnBytesFromClasspathInputStream() throws IOException {
 
   @Test
   public void shouldReturnAllClassNames() {
-    assertEquals(Arrays.asList("FooClass", "BarClass"),
+    assertEquals(asList("FooClass", "BarClass"),
         this.testee.classNames());
   }
 
   @Test
   public void shouldFindMatchingClasses() {
-    assertEquals(Arrays.asList("FooClass"),
+    assertEquals(asList("FooClass"),
         this.testee.findClasses(isEqual("FooClass")));
   }
 
@@ -84,6 +93,21 @@ public void shouldAllowSubComponentsToBeSelected() {
         .getComponent(rootIsEqualTo("foo")).classNames());
   }
 
+  @Test
+  public void handlesEmptyFilesWithoutError() throws IOException {
+    File empty = dir.newFile("empty.jar");
+    ClassPath underTest = new ClassPath(asList(empty));
+    assertThat(underTest.getClassData("")).isNull();
+  }
+
+  @Test
+  public void doesNotErrorWhenNonArchiveFilesOnClasspath() throws IOException {
+    File notAJar = dir.newFile("notajar");
+    Files.write(notAJar.toPath(), "some content".getBytes());
+    ClassPath underTest = new ClassPath(asList(notAJar));
+    assertThat(underTest.getClassData("")).isNull();
+  }
+
   private Predicate<ClassPathRoot> rootIsEqualTo(final String value) {
     return a -> a.cacheLocation().get().equals(value);
   }