Try to execute unparseable test class files

Instead of failing the build when a test class file cannot be parsed,
e.g. if it's compiled for a JVM that is not yet supported by the
version of ASM that we use, we now assume it's a test class and pass
it to the testing framework which usually ignores classes that are not
tests.

Related issue: #7059

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/fixture/JUnitMultiVersionIntegrationSpec.groovy

@@ -116,11 +116,11 @@ abstract class JUnitMultiVersionIntegrationSpec extends MultiVersionIntegrationS
         }
     }
 
-    private static boolean isVintage() {
+    static boolean isVintage() {
         return version.toString().startsWith("Vintage")
     }
 
-    private static boolean isJupiter() {
+    static boolean isJupiter() {
         return version.toString().startsWith("Jupiter")
     }
 

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/junit/JUnitIntegrationTest.groovy

@@ -32,7 +32,10 @@ import static org.gradle.testing.fixture.JUnitCoverage.JUNIT_4_LATEST
 import static org.gradle.testing.fixture.JUnitCoverage.JUNIT_VINTAGE_JUPITER
 import static org.gradle.util.Matchers.containsLine
 import static org.gradle.util.Matchers.matchesRegexp
-import static org.hamcrest.Matchers.*
+import static org.hamcrest.Matchers.containsString
+import static org.hamcrest.Matchers.equalTo
+import static org.hamcrest.Matchers.not
+import static org.hamcrest.Matchers.startsWith
 import static org.junit.Assert.assertThat
 
 @TargetCoverage({ JUNIT_4_LATEST + JUNIT_VINTAGE_JUPITER })
@@ -468,4 +471,32 @@ class JUnitIntegrationTest extends JUnitMultiVersionIntegrationSpec {
         result.testClass("org.gradle.SomeSuite").assertStderr(containsString("stderr in TestSetup#setup"))
         result.testClass("org.gradle.SomeSuite").assertStderr(containsString("stderr in TestSetup#teardown"))
     }
+
+    def "tries to execute unparseable test classes"() {
+        given:
+        testDirectory.file('build/classes/java/test/com/example/Foo.class').text = "invalid class file"
+        buildFile << """
+            apply plugin: 'java'
+            ${mavenCentralRepository()}
+            dependencies {
+                testCompile '$dependencyNotation'
+            }
+        """
+
+        when:
+        fails('test', '-x', 'compileTestJava')
+
+        then:
+        failureCauseContains("There were failing tests")
+        DefaultTestExecutionResult result = new DefaultTestExecutionResult(testDirectory)
+        if (isVintage() || isJupiter()) {
+            result.testClassStartsWith('Gradle Test Executor')
+                .assertTestCount(1, 1, 0)
+                .assertTestFailed("failed to execute tests", containsString("Could not execute test class 'com.example.Foo'"))
+        } else {
+            result.testClass('com.example.Foo')
+                .assertTestCount(1, 1, 0)
+                .assertTestFailed("initializationError", containsString('ClassFormatError'))
+        }
+    }
 }

subprojects/testing-jvm/src/integTest/groovy/org/gradle/testing/testng/TestNGIntegrationTest.groovy

@@ -261,6 +261,21 @@ class TestNGIntegrationTest extends MultiVersionIntegrationSpec {
         result.assertTestsFailed()
     }
 
+    def "tries to execute unparseable test classes"() {
+        given:
+        testDirectory.file('build/classes/java/test/com/example/Foo.class').text = "invalid class file"
+
+        when:
+        fails('test', '-x', 'compileTestJava')
+
+        then:
+        failureCauseContains("There were failing tests")
+        DefaultTestExecutionResult result = new DefaultTestExecutionResult(testDirectory)
+        result.testClassStartsWith('Gradle Test Executor')
+            .assertTestCount(1, 1, 0)
+            .assertTestFailed("failed to execute tests", containsString("Could not execute test class 'com.example.Foo'"))
+    }
+
     private static String testListener() {
         return '''
             def listener = new TestListenerImpl()

subprojects/testing-jvm/src/main/java/org/gradle/api/internal/tasks/testing/detection/AbstractTestFrameworkDetector.java

@@ -18,9 +18,12 @@
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
-import org.gradle.api.GradleException;
+import org.gradle.api.internal.file.RelativeFile;
 import org.gradle.api.internal.tasks.testing.DefaultTestClassRunInfo;
 import org.gradle.api.internal.tasks.testing.TestClassProcessor;
+import org.gradle.internal.Factories;
+import org.gradle.internal.Factory;
+import org.gradle.internal.IoActions;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.Type;
 import org.slf4j.Logger;
@@ -114,69 +117,74 @@ public void setTestClasspath(Set<File> testClasspath) {
         this.testClasspath = testClasspath;
     }
 
-    protected TestClassVisitor classVisitor(final File testClassFile) {
+    private TestClass readClassFile(File testClassFile, Factory<String> fallbackClassNameProvider) {
         final TestClassVisitor classVisitor = createClassVisitor();
 
         InputStream classStream = null;
         try {
             classStream = new BufferedInputStream(new FileInputStream(testClassFile));
             final ClassReader classReader = new ClassReader(IOUtils.toByteArray(classStream));
             classReader.accept(classVisitor, ClassReader.SKIP_DEBUG | ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES);
+            return TestClass.forParseableFile(classVisitor);
         } catch (Throwable e) {
-            throw new GradleException("failed to read class file " + testClassFile.getAbsolutePath(), e);
+            LOGGER.debug("Failed to read class file " + testClassFile.getAbsolutePath() + "; assuming it's a test class and continuing", e);
+            return TestClass.forUnparseableFile(fallbackClassNameProvider.create());
         } finally {
             IOUtils.closeQuietly(classStream);
         }
-
-        return classVisitor;
     }
 
     @Override
-    public boolean processTestClass(File testClassFile) {
-        return processTestClass(testClassFile, false);
+    public boolean processTestClass(final RelativeFile testClassFile) {
+        return processTestClass(testClassFile.getFile(), false, new Factory<String>() {
+            @Override
+            public String create() {
+                return testClassFile.getRelativePath().getPathString().replace(".class", "");
+            }
+        });
     }
 
     /**
      * Uses a TestClassVisitor to detect whether the class in the testClassFile is a test class. <p/> If the class is not a test, this function will go up the inheritance tree to check if a parent
      * class is a test class. First the package of the parent class is checked, if it is a java.lang or groovy.lang the class can't be a test class, otherwise the parent class is scanned. <p/> When a
      * parent class is a test class all the extending classes are marked as test classes.
      */
-    private boolean processTestClass(final File testClassFile, boolean superClass) {
-        final TestClassVisitor classVisitor = classVisitor(testClassFile);
+    private boolean processTestClass(File testClassFile, boolean superClass, Factory<String> fallbackClassNameProvider) {
+        TestClass testClass = readClassFile(testClassFile, fallbackClassNameProvider);
 
-        boolean isTest = classVisitor.isTest();
+        boolean isTest = testClass.isTest();
 
         if (!isTest) { // scan parent class
-            final String superClassName = classVisitor.getSuperClassName();
+            String superClassName = testClass.getSuperClassName();
 
             if (isKnownTestCaseClassName(superClassName)) {
                 isTest = true;
             } else {
-                final File superClassFile = getSuperTestClassFile(superClassName);
+                File superClassFile = getSuperTestClassFile(superClassName);
 
                 if (superClassFile != null) {
-                    isTest = processSuperClass(superClassFile);
+                    isTest = processSuperClass(superClassFile, superClassName);
                 } else {
                     LOGGER.debug("test-class-scan : failed to scan parent class {}, could not find the class file",
                         superClassName);
                 }
             }
         }
 
-        publishTestClass(isTest, classVisitor, superClass);
+        publishTestClass(isTest, testClass, superClass);
 
         return isTest;
     }
 
     protected abstract boolean isKnownTestCaseClassName(String testCaseClassName);
 
-    private boolean processSuperClass(File testClassFile) {
+    private boolean processSuperClass(File testClassFile, String superClassName) {
         boolean isTest;
 
         Boolean isSuperTest = superClasses.get(testClassFile);
 
         if (isSuperTest == null) {
-            isTest = processTestClass(testClassFile, true);
+            isTest = processTestClass(testClassFile, true, Factories.constant(superClassName));
 
             superClasses.put(testClassFile, isTest);
         } else {
@@ -190,9 +198,9 @@ private boolean processSuperClass(File testClassFile) {
      * In none super class mode a test class is published when the class is a test and it is not abstract. In super class mode it must not publish the class otherwise it will get published multiple
      * times (for each extending class).
      */
-    private void publishTestClass(boolean isTest, TestClassVisitor classVisitor, boolean superClass) {
-        if (isTest && !classVisitor.isAbstract() && !superClass) {
-            String className = Type.getObjectType(classVisitor.getClassName()).getClassName();
+    private void publishTestClass(boolean isTest, TestClass testClass, boolean superClass) {
+        if (isTest && !testClass.isAbstract() && !superClass) {
+            String className = Type.getObjectType(testClass.getClassName()).getClassName();
             testClassProcessor.processTestClass(new DefaultTestClassRunInfo(className));
         }
     }
@@ -201,4 +209,43 @@ private void publishTestClass(boolean isTest, TestClassVisitor classVisitor, boo
     public void startDetection(TestClassProcessor testClassProcessor) {
         this.testClassProcessor = testClassProcessor;
     }
+
+    private static class TestClass {
+        private final boolean test;
+        private final boolean isAbstract;
+        private final String className;
+        private final String superClassName;
+
+        static TestClass forParseableFile(TestClassVisitor testClassVisitor) {
+            return new TestClass(testClassVisitor.isTest(), testClassVisitor.isAbstract(), testClassVisitor.getClassName(), testClassVisitor.getSuperClassName());
+        }
+
+        static TestClass forUnparseableFile(String className) {
+            return new TestClass(true, false, className, null);
+        }
+
+        private TestClass(boolean test, boolean isAbstract, String className, String superClassName) {
+            this.test = test;
+            this.isAbstract = isAbstract;
+            this.className = className;
+            this.superClassName = superClassName;
+        }
+
+        boolean isTest() {
+            return test;
+        }
+
+        boolean isAbstract() {
+            return isAbstract;
+        }
+
+        String getClassName() {
+            return className;
+        }
+
+        String getSuperClassName() {
+            return superClassName;
+        }
+    }
+
 }

subprojects/testing-jvm/src/main/java/org/gradle/api/internal/tasks/testing/detection/DefaultTestClassScanner.java

@@ -19,6 +19,7 @@
 import org.gradle.api.file.EmptyFileVisitor;
 import org.gradle.api.file.FileTree;
 import org.gradle.api.file.FileVisitDetails;
+import org.gradle.api.internal.file.RelativeFile;
 import org.gradle.api.internal.tasks.testing.DefaultTestClassRunInfo;
 import org.gradle.api.internal.tasks.testing.TestClassProcessor;
 import org.gradle.api.internal.tasks.testing.TestClassRunInfo;
@@ -55,7 +56,7 @@ private void detectionScan() {
         testFrameworkDetector.startDetection(testClassProcessor);
         candidateClassFiles.visit(new ClassFileVisitor() {
             public void visitClassFile(FileVisitDetails fileDetails) {
-                testFrameworkDetector.processTestClass(fileDetails.getFile());
+                testFrameworkDetector.processTestClass(new RelativeFile(fileDetails.getFile(), fileDetails.getRelativePath()));
             }
         });
     }

subprojects/testing-jvm/src/main/java/org/gradle/api/internal/tasks/testing/detection/TestFrameworkDetector.java

@@ -15,6 +15,7 @@
  */
 package org.gradle.api.internal.tasks.testing.detection;
 
+import org.gradle.api.internal.file.RelativeFile;
 import org.gradle.api.internal.tasks.testing.TestClassProcessor;
 
 import java.io.File;
@@ -23,7 +24,7 @@
 public interface TestFrameworkDetector {
     void startDetection(TestClassProcessor testClassProcessor);
 
-    boolean processTestClass(File testClassFile);
+    boolean processTestClass(RelativeFile testClassFile);
 
     void setTestClasses(Set<File> testClasses);
 

subprojects/testing-jvm/src/test/groovy/org/gradle/api/internal/tasks/testing/detection/DefaultTestClassScannerTest.groovy

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-
 package org.gradle.api.internal.tasks.testing.detection
 
 import org.gradle.api.file.FileTree
@@ -25,15 +24,21 @@ import org.gradle.api.internal.file.DefaultFileVisitDetails
 import org.gradle.api.internal.tasks.testing.TestClassProcessor
 import org.junit.Test
 import spock.lang.Specification
+import spock.lang.Subject
 
 class DefaultTestClassScannerTest extends Specification {
-    private final TestFrameworkDetector detector = Mock()
-    private final TestClassProcessor processor = Mock()
-    private final FileTree files = Mock()
+    def files = Mock(FileTree)
+    def detector = Mock(TestFrameworkDetector)
+    def processor = Stub(TestClassProcessor)
+
+    @Subject
+    def scanner = new DefaultTestClassScanner(files, detector, processor)
 
     @Test
     void passesEachClassFileToTestClassDetector() {
-        DefaultTestClassScanner scanner = new DefaultTestClassScanner(files, detector, processor)
+        given:
+        def class1 = stubFileVisitDetails('class1')
+        def class2 = stubFileVisitDetails('class2')
 
         when:
         scanner.run()
@@ -48,17 +53,19 @@ class DefaultTestClassScannerTest extends Specification {
         1 * files.visit(_) >> { args ->
             FileVisitor visitor = args[0]
             assert visitor
-            visitor.visitFile(mockFileVisitDetails('class1'))
-            visitor.visitFile(mockFileVisitDetails('class2'))
+            visitor.visitFile(class1)
+            visitor.visitFile(class2)
         }
+        then:
+        1 * detector.processTestClass({ it.file.is(class1.file) && it.relativePath.is(class1.relativePath) })
+        then:
+        1 * detector.processTestClass({ it.file.is(class2.file) && it.relativePath.is(class2.relativePath) })
 
         0 * _._
     }
 
     @Test
     void skipAnonymousClass() {
-        DefaultTestClassScanner scanner = new DefaultTestClassScanner(files, detector, processor)
-
         when:
         scanner.run()
 
@@ -68,14 +75,14 @@ class DefaultTestClassScannerTest extends Specification {
         1 * files.visit(_) >> { args ->
             FileVisitor visitor = args[0]
             assert visitor
-            visitor.visitFile(mockFileVisitDetails('AnonymousClass$1'))
-            visitor.visitFile(mockFileVisitDetails('AnonymousClass$1$22'))
+            visitor.visitFile(stubFileVisitDetails('AnonymousClass$1'))
+            visitor.visitFile(stubFileVisitDetails('AnonymousClass$1$22'))
         }
 
         0 * _._
     }
 
-    FileVisitDetails mockFileVisitDetails(String className) {
+    FileVisitDetails stubFileVisitDetails(String className) {
         return new DefaultFileVisitDetails(new File("${className}.class"), new RelativePath(false, "${className}.class"), null, null, null)
     }
 }