commit 648bfc7bd309bcd3ab6d123b48797b71b557127b
author Julien Herr <julien@herr.fr> Sat Nov 21 14:07:47 2015 +0100
committer Julien Herr <julien@herr.fr> Sat Nov 21 22:39:22 2015 +0100
tree af1a34f4a168ce9ae1768c8d1e0dc7280a27e65e
parent 36dfb40f1200b21ab195392ef15660739859646e

Fix #876: NPE when a factory method is not static

src/main/java/org/testng/internal/FactoryMethod.java

diff --git a/src/main/java/org/testng/internal/FactoryMethod.java b/src/main/java/org/testng/internal/FactoryMethod.java
index d599b25..fb80287 100644
--- a/src/main/java/org/testng/internal/FactoryMethod.java
+++ b/src/main/java/org/testng/internal/FactoryMethod.java
@@ -1,5 +1,6 @@
 package org.testng.internal;
 
+import java.lang.reflect.Modifier;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -33,12 +34,22 @@
                        ITestContext testContext)
   {
     super(com.getName(), com, annotationFinder, instance);
-//    Utils.checkInstanceOrStatic(instance, method);
+    Utils.checkInstanceOrStatic(instance, com.getMethod());
     Class<?> declaringClass = com.getDeclaringClass();
     if (instance != null && ! declaringClass.isAssignableFrom(instance.getClass())) {
       throw new TestNGException("Mismatch between instance/method classes:"
           + instance.getClass() + " " + declaringClass);
     }
+    if (instance == null && com.getMethod() != null && !Modifier.isStatic(com.getMethod().getModifiers())) {
+      throw new TestNGException("An inner factory method MUST be static. But '" + com.getMethod().getName() + "' from '" + declaringClass.getName() + "' is not.");
+    }
+    if (com.getMethod() != null && !Modifier.isPublic(com.getMethod().getModifiers())) {
+      try {
+        com.getMethod().setAccessible(true);
+      } catch (SecurityException e) {
+        throw new TestNGException(e);
+      }
+    }
 
     m_instance = instance;
     m_xmlTest = xmlTest;

src/main/java/org/testng/internal/Utils.java

diff --git a/src/main/java/org/testng/internal/Utils.java b/src/main/java/org/testng/internal/Utils.java
index 741a812..aa399b0 100644
--- a/src/main/java/org/testng/internal/Utils.java
+++ b/src/main/java/org/testng/internal/Utils.java
@@ -796,7 +796,7 @@
    * Make sure that either we have an instance or if not, that the method is static
    */
   public static void checkInstanceOrStatic(Object instance, Method method) {
-    if (instance == null && ! Modifier.isStatic(method.getModifiers())) {
+    if (instance == null && method != null && ! Modifier.isStatic(method.getModifiers())) {
       throw new TestNGException("Can't invoke " + method + ": either make it static or add "
           + "a no-args constructor to your class");
     }

src/test/java/test/factory/FactoryIntegrationTest.java

diff --git a/src/test/java/test/factory/FactoryIntegrationTest.java b/src/test/java/test/factory/FactoryIntegrationTest.java
index 94645dd..169bdf0 100644
--- a/src/test/java/test/factory/FactoryIntegrationTest.java
+++ b/src/test/java/test/factory/FactoryIntegrationTest.java
@@ -2,6 +2,7 @@
 
 import org.testng.TestListenerAdapter;
 import org.testng.TestNG;
+import org.testng.TestNGException;
 import org.testng.annotations.Test;
 import test.SimpleBaseTest;
 
@@ -18,9 +19,9 @@
 
         try {
             tng.run();
-            failBecauseExceptionWasNotThrown(IllegalArgumentException.class);
-        } catch (IllegalArgumentException e) {
-            assertThat(e).hasMessage("A factory method MUST be static. But 'createInstances' from 'test.factory.GitHub876Sample' is not.");
+            failBecauseExceptionWasNotThrown(TestNGException.class);
+        } catch (TestNGException e) {
+            assertThat(e).hasMessage("\nCan't invoke public java.lang.Object[] test.factory.GitHub876Sample.createInstances(): either make it static or add a no-args constructor to your class");
         }
     }
 }