Defend against NPE when running disabled tests. (#3570)

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

@@ -18,6 +18,7 @@ package org.gradle.testing.testng
 
 import org.gradle.testing.AbstractTestFrameworkIntegrationTest
 import org.gradle.testing.fixture.TestNGCoverage
+import spock.lang.Issue
 
 class TestNGTestFrameworkIntegrationTest extends AbstractTestFrameworkIntegrationTest {
     def setup() {
@@ -74,4 +75,19 @@ class TestNGTestFrameworkIntegrationTest extends AbstractTestFrameworkIntegratio
     String getFailingTestCaseName() {
         return "fail"
     }
+
+    @Issue("https://github.com/gradle/gradle/issues/3545")
+    def "disabled tests do not throw NullPointerException"() {
+        given:
+        file("src/test/java/DisabledTest.java") << """
+            @org.testng.annotations.Test(enabled = false)
+            public class DisabledTest {
+                public void testOne() {}
+                public void testTwo() {}
+            }
+        """
+
+        expect:
+        succeeds "check"
+    }
 }

subprojects/testing-jvm/src/main/java/org/gradle/api/internal/tasks/testing/testng/TestNGTestResultProcessorAdapter.java

@@ -111,7 +111,11 @@ public void onAfterClass(ITestClass testClass) {
         synchronized (lock) {
             id = testClassId.remove(testClass);
         }
-        resultProcessor.completed(id, new TestCompleteEvent(clock.getCurrentTime()));
+        // Guard against TestNG calling this hook more than once with the same testClass.
+        // See https://github.com/cbeust/testng/issues/1618 for details.
+        if (id != null) {
+            resultProcessor.completed(id, new TestCompleteEvent(clock.getCurrentTime()));
+        }
     }
 
     @Override

subprojects/testing-jvm/src/test/groovy/org/gradle/api/internal/tasks/testing/testng/TestNGTestResultProcessorAdapterTest.groovy

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.gradle.api.internal.tasks.testing.testng
+
+import org.gradle.api.internal.tasks.testing.TestCompleteEvent
+import org.gradle.api.internal.tasks.testing.TestResultProcessor
+import org.gradle.internal.id.IdGenerator
+import org.gradle.internal.time.Clock;
+import org.testng.ITestClass
+import spock.lang.Issue;
+import spock.lang.Specification
+import spock.lang.Subject
+
+class TestNGTestResultProcessorAdapterTest extends Specification {
+
+    private TestResultProcessor resultProcessor = Mock()
+
+    private IdGenerator<Long> idGenerator = Mock {
+        generateId() >> 1L
+    }
+
+    @Subject
+    private TestNGTestResultProcessorAdapter resultProcessorAdapter = new TestNGTestResultProcessorAdapter(
+        resultProcessor, idGenerator, Mock(Clock))
+
+    @Issue("https://github.com/gradle/gradle/issues/3545")
+    def "runs onAfterClass hook only once per test class"() {
+        given:
+        def testClass = Mock(ITestClass)
+        resultProcessorAdapter.onBeforeClass(testClass)
+
+        when:
+        resultProcessorAdapter.onAfterClass(testClass)
+
+        then:
+        1 * resultProcessor.completed(!null, _ as TestCompleteEvent)
+
+        when:
+        resultProcessorAdapter.onAfterClass(testClass)
+
+        then:
+        0 * resultProcessor.completed(_, _ as TestCompleteEvent)
+    }
+}