Merge pull request #1349 from hcoles/feature/powermock_stability

limit backdoor coverage by powermock

pitest-maven-verification/src/test/java/org/pitest/PitMojoIT.java

@@ -122,9 +122,9 @@ public void shouldExcludeSpecifiedJUnitCategories() throws Exception {
   }
 
   @Test
-  @Ignore("test is flakey, possibly due to real non deterministic issue with powermock")
+  //@Ignore("test is flakey, possibly due to real non deterministic issue with powermock")
   public void shouldWorkWithPowerMock() throws Exception {
-    skipIfJavaVersionNotSupportByThirdParty();
+    runForJava8Only();
     File testDir = prepare("/pit-powermock");
     verifier.addCliOption("-DtimeoutConstant=10000");
     verifier.executeGoal("test");
@@ -465,6 +465,11 @@ public void resolvesCorrectFilesForKotlinMultiModules() throws Exception {
 
   }
 
+  private void runForJava8Only() {
+    String javaVersion = System.getProperty("java.version");
+    assumeTrue(javaVersion.startsWith("1.8"));
+  }
+
   private void skipIfJavaVersionNotSupportByThirdParty() {
     String javaVersion = System.getProperty("java.version");
     assumeFalse(javaVersion.startsWith("9") || javaVersion.startsWith("10") || javaVersion.startsWith("11"));

pitest-maven-verification/src/test/resources/pit-powermock/pom.xml

@@ -17,10 +17,10 @@
 			<artifactId>powermock-module-junit4</artifactId>
 			<version>${powermock.version}</version>
 			<scope>test</scope>
-		</dependency>
+          </dependency>
 		<dependency>
 			<groupId>org.powermock</groupId>
-			<artifactId>powermock-api-mockito</artifactId>
+			<artifactId>powermock-api-mockito2</artifactId>
 			<version>${powermock.version}</version>
 			<scope>test</scope>
 		</dependency>
@@ -70,7 +70,7 @@
 
 	<properties>
 		<junit.version>4.13.1</junit.version>	
-		<powermock.version>1.7.3</powermock.version>
+		<powermock.version>2.0.9</powermock.version>
 	</properties>
 
 </project>

pitest-maven-verification/src/test/resources/pit-powermock/src/test/java/com/example/PowerMockAgentTest.java

@@ -32,7 +32,7 @@ public void testReplaceStaticCallToOtherClass() {
 
     new PowerMockAgentCallFoo().call();
 
-    PowerMockito.verifyStatic();
+    PowerMockito.verifyStatic(PowerMockAgentFoo.class);
     PowerMockAgentFoo.foo();
 
   }

pitest-maven-verification/src/test/resources/pit-powermock/src/test/java/com/example/PowerMockTest.java

@@ -34,7 +34,7 @@ public void testReplaceStaticCallToOtherClass() {
 
     new PowerMockCallFoo().call();
 
-    PowerMockito.verifyStatic();
+    PowerMockito.verifyStatic(PowerMockFoo.class);
     PowerMockFoo.foo();
 
   }
@@ -45,7 +45,7 @@ public void testReplaceStaticCallToMutatedClass() {
 
     new PowerMockCallsOwnMethod().call();
 
-    PowerMockito.verifyStatic();
+    PowerMockito.verifyStatic(PowerMockCallsOwnMethod.class);
     PowerMockCallsOwnMethod.foo();
 
   }

pitest/src/main/java/org/pitest/coverage/analysis/CoverageAnalyser.java

@@ -35,7 +35,7 @@ public CoverageAnalyser(final CoverageClassVisitor parent, final int classId,
 
   @Override
   public void visitEnd() {
-    final List<Block> blocks = findRequriedProbeLocations();
+    final List<Block> blocks = findRequiredProbeLocations();
 
     this.parent.registerProbes(blocks.size());
 
@@ -49,7 +49,7 @@ public void visitEnd() {
             this.probeOffset), counter));
   }
 
-  private List<Block> findRequriedProbeLocations() {
+  private List<Block> findRequiredProbeLocations() {
     return ControlFlowAnalyser.analyze(this);
   }
 }

pitest/src/main/java/org/pitest/mutationtest/mocksupport/JavassistCoverageInterceptor.java

@@ -21,6 +21,7 @@
 import org.pitest.classpath.ClassloaderByteArraySource;
 import org.pitest.coverage.AlreadyInstrumentedException;
 import org.pitest.coverage.CoverageClassVisitor;
+import org.pitest.functional.prelude.Prelude;
 import org.pitest.reflection.Reflection;
 import org.pitest.util.IsolationUtils;
 import org.pitest.util.StreamUtil;
@@ -32,27 +33,41 @@
 import java.io.InputStream;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Predicate;
 
 public final class JavassistCoverageInterceptor {
 
+  private static final Predicate<String> EXCLUDED_CLASSES = excluded();
+
+
   private static final Map<String, String> COMPUTE_CACHE = new ConcurrentHashMap<>();
 
   private JavassistCoverageInterceptor() {
 
   }
 
+  // not referenced directly, but called from transformed bytecode
   public static InputStream openClassfile(final Object classPath, // NO_UCD
       final String name) {
 
     try {
         final byte[] bs = getOriginalBytes(classPath, name);
-        return new ByteArrayInputStream(transformBytes(IsolationUtils.getContextClassLoader(), name, bs));
+        if (shouldInclude(name)) {
+          return new ByteArrayInputStream(transformBytes(IsolationUtils.getContextClassLoader(), name, bs));
+        } else {
+          return new ByteArrayInputStream(bs);
+        }
     } catch (final IOException ex) {
       throw Unchecked.translateCheckedException(ex);
     }
 
   }
 
+
+  private static boolean shouldInclude(String name) {
+    return EXCLUDED_CLASSES.negate().test(name);
+  }
+
   private static byte[] getOriginalBytes(final Object classPath,
       final String name) throws IOException {
     try (InputStream is = returnNormalBytes(classPath,name)) {
@@ -79,7 +94,7 @@ private static byte[] transformBytes(final ClassLoader loader,
           ClassReader.EXPAND_FRAMES);
       return writer.toByteArray();
     } catch (AlreadyInstrumentedException ex) {
-      return null;
+      return classfileBuffer;
     }
   }
 
@@ -93,4 +108,25 @@ private static InputStream returnNormalBytes(final Object classPath,
     }
   }
 
+    // Classes loaded by pitest's coverage system are not
+    // instrumented unless they are in scope for mutation.
+    // They would however be instrumented here (including pitest's own classes).
+    // We don't have knowlege of which classes are in scope for mutation here
+    // so the best we can do is exclude pitest itself and some common classes.
+    private static Predicate<String> excluded() {
+      return Prelude.or(
+              startsWith("javassist."),
+              startsWith("org.pitest."),
+              startsWith("java."),
+              startsWith("javax."),
+              startsWith("com.sun."),
+              startsWith("org.junit."),
+              startsWith("org.powermock."),
+              startsWith("org.mockito."),
+              startsWith("sun."));
+    }
+
+    private static Predicate<String> startsWith(String start) {
+      return s -> s.startsWith(start);
+    }
 }