r11698@thor:  alex | 2007-02-18  E. Europe Standard Time
 2 improvements + 1 bugfix (TESTNG-142)

diff --git a/CHANGES.txt b/CHANGES.txt
index 087dd3a..b5a0ed7 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -5,6 +5,11 @@
 Added: ISuite now gives access to the current XmlSuite
 Fixed: TESTNG-139 dependsOnMethods gets confused when dependency is "protected"
 Fixed: TESTNG-141 junit attribute set to false in testng-failed.xml when it should be true
+Fixed: TESTNG-142 Exceptions in DataProvider are not reported as failed test
+Added: Improved behavior for @Before/@AfterClass when using @Factory 
+(http://forums.opensymphony.com/thread.jspa?threadID=6594&messageID=122294#122294)
+Added: Support for concurrent execution for invocationCount=1 threadPoolSize>1 and @DataProvider
+(http://forums.opensymphony.com/thread.jspa?threadID=64738&tstart=0)
 
 ===========================================================================
 5.5
diff --git a/src/main/org/testng/ClassMethodMap.java b/src/main/org/testng/ClassMethodMap.java
index b09166b..6d73eed 100644
--- a/src/main/org/testng/ClassMethodMap.java
+++ b/src/main/org/testng/ClassMethodMap.java
@@ -4,15 +4,24 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * This class maintains a map of <CODE><Class, List<ITestNGMethod>></CODE>.
  * It is used by TestWorkers to determine if the method they just ran
  * is the last of its class, in which case it's time to invoke all the
  * afterClass methods.
+ * 
+ * @author <a href='mailto:the[dot]mindstorm[at]gmail[dot]com'>Alex Popescu</a>
  */
 public class ClassMethodMap {
   private Map<Class, List<ITestNGMethod>> m_classMap = new HashMap<Class, List<ITestNGMethod>>();
+  // These two variables are used throughout the workers to keep track
+  // of what beforeClass/afterClass methods have been invoked
+  private Map<ITestClass, Set<Object>> m_beforeClassMethods = new HashMap<ITestClass, Set<Object>>();
+  private Map<ITestClass, Set<Object>> m_afterClassMethods = new HashMap<ITestClass, Set<Object>>();
+  
+
   
   public ClassMethodMap(ITestNGMethod[] methods) {
     for (ITestNGMethod m : methods) {
@@ -40,4 +49,25 @@
   private Class getMethodClass(ITestNGMethod m) {
     return m.getTestClass().getRealClass();
   }
+  
+  public Map<ITestClass, Set<Object>> getInvokedBeforeClassMethods() {
+    return m_beforeClassMethods;
+  }
+  
+  public Map<ITestClass, Set<Object>> getInvokedAfterClassMethods() {
+    return m_afterClassMethods;
+  }
+  
+  public void clear() {
+    for(Set<Object> instances: m_beforeClassMethods.values()) {
+      instances.clear();
+      instances= null;
+    }
+    for(Set<Object> instances: m_afterClassMethods.values()) {
+      instances.clear();
+      instances= null;
+    }
+    m_beforeClassMethods.clear();
+    m_afterClassMethods.clear();
+  }
 }
diff --git a/src/main/org/testng/TestRunner.java b/src/main/org/testng/TestRunner.java
index 9a6cc62..f045523 100644
--- a/src/main/org/testng/TestRunner.java
+++ b/src/main/org/testng/TestRunner.java
@@ -8,6 +8,7 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -529,7 +530,11 @@
       public long getMaxTimeOut() {
         return 0;
       }
-      
+            
+      public List<ITestResult> getTestResults() {
+        return null;
+      }
+
       /**
        * @see java.lang.Runnable#run()
        */
@@ -567,33 +572,20 @@
     log(3, "Found " + (sequentialList.size() + parallelList.size()) + " applicable methods");
     
     //
-    // Find out all the group methods
-    //
-//    m_groupMethods = findGroupMethods(m_classMap.values());
-
-    //
     // Create the workers
     //
     List<TestMethodWorker> workers = new ArrayList<TestMethodWorker>();
     
-    // These two variables are used throughout the workers to keep track
-    // of what beforeClass/afterClass methods have been invoked
-    Map<ITestClass, ITestClass> beforeClassMethods = new HashMap<ITestClass, ITestClass>();
-    Map<ITestClass, ITestClass> afterClassMethods = new HashMap<ITestClass, ITestClass>();
-    
     ClassMethodMap cmm = new ClassMethodMap(m_allTestMethods);
     
     // All the sequential tests are place in one worker, guaranteeing they
     // will be invoked sequentially
     if (sequentialList.size() > 0) {
-      for (List<ITestNGMethod> sl : sequentialList) {
-        
+      for (List<ITestNGMethod> sl : sequentialList) {        
         workers.add(new TestMethodWorker(m_invoker,
                                          methodsToMethodInstances(sl),
                                          m_xmlTest.getSuite(),
                                          params,
-                                         beforeClassMethods, 
-                                         afterClassMethods,
                                          m_allTestMethods,
                                          m_groupMethods,
                                          cmm,
@@ -605,15 +597,12 @@
     // invoked in parallel
     if (parallelList.size() > 0) {
       for (ITestNGMethod tm : parallelList) {
-//        MethodInstance mi = new MethodInstance(tm, tm.getTestClass().getInstances(true));
-        MethodInstance[] methodInstances = methodsToMultipleMethodInstances(Arrays.asList(new ITestNGMethod[] {tm}));
+        List<MethodInstance> methodInstances = methodsToMultipleMethodInstances(Arrays.asList(new ITestNGMethod[] {tm}));
         for (MethodInstance mi : methodInstances) {
           workers.add(new TestMethodWorker(m_invoker,
                                            new MethodInstance[] { mi },
                                            m_xmlTest.getSuite(),
                                            params,
-                                           beforeClassMethods, 
-                                           afterClassMethods,
                                            m_allTestMethods,
                                            m_groupMethods,
                                            cmm,
@@ -622,10 +611,15 @@
       }
     }
 
-    runWorkers(workers, xmlTest.getParallel());
+    try {
+      runWorkers(workers, xmlTest.getParallel());
+    }
+    finally {
+      cmm.clear();
+    }
   }
 
-  private MethodInstance[] methodsToMultipleMethodInstances(List<ITestNGMethod> sl) {
+  private List<MethodInstance> methodsToMultipleMethodInstances(List<ITestNGMethod> sl) {
     List<MethodInstance> vResult = new ArrayList<MethodInstance>();
     for (ITestNGMethod m : sl) {
       Object[] instances = m.getTestClass().getInstances(true);
@@ -634,8 +628,7 @@
       }
     }
     
-    MethodInstance[] result = vResult.toArray(new MethodInstance[vResult.size()]);
-    return result;
+    return vResult;
   }
 
   private MethodInstance[] methodsToMethodInstances(List<ITestNGMethod> sl) {
@@ -652,9 +645,7 @@
   //
   private void runWorkers(List<? extends IMethodWorker> workers, String parallelMode) {
     if (XmlSuite.PARALLEL_METHODS.equals(parallelMode) 
-        || "true".equalsIgnoreCase(parallelMode)
-//        || XmlSuite.PARALLEL_TESTS.equals(parallelMode)
-        ) 
+        || "true".equalsIgnoreCase(parallelMode) ) 
     {
       //
       // Parallel run
diff --git a/src/main/org/testng/internal/IMethodWorker.java b/src/main/org/testng/internal/IMethodWorker.java
index a7822d6..6795bcb 100644
--- a/src/main/org/testng/internal/IMethodWorker.java
+++ b/src/main/org/testng/internal/IMethodWorker.java
@@ -1,9 +1,15 @@
 package org.testng.internal;

 

+import java.util.List;

+

+import org.testng.ITestResult;

+

 

 /**

- * This class/interface 

+ * Requirements for a method runnable.

  */

 public interface IMethodWorker extends Runnable {

   long getMaxTimeOut();

+  

+  List<ITestResult> getTestResults();

 }

diff --git a/src/main/org/testng/internal/Invoker.java b/src/main/org/testng/internal/Invoker.java
index 99f3373..aa8c371 100644
--- a/src/main/org/testng/internal/Invoker.java
+++ b/src/main/org/testng/internal/Invoker.java
@@ -5,6 +5,7 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
@@ -641,6 +642,11 @@
    * <p/>
    * Note that this method also takes care of invoking the beforeTestMethod
    * and afterTestMethod, if any.
+   * 
+   * Note (alex): this method can be refactored to use a SingleTestMethodWorker that 
+   * directly invokes 
+   * {@link #invokeTestMethod(Object[], ITestNGMethod, Object[], XmlSuite, Map, ITestClass, ITestNGMethod[], ITestNGMethod[], ConfigurationGroupMethods)}
+   * and this would simplify the implementation (see how DataTestMethodWorker is used)
    */
   public List<ITestResult> invokeTestMethods(ITestNGMethod testMethod,
                                              ITestNGMethod[] allTestMethods,
@@ -673,7 +679,7 @@
     int failureCount = 0;
 
     Class[] expectedExceptionClasses = 
-      MethodHelper.findExpectedExceptions(m_annotationFinder, testMethod.getMethod());
+        MethodHelper.findExpectedExceptions(m_annotationFinder, testMethod.getMethod());
     while(invocationCount-- > 0) {
       boolean okToProceed = checkDependencies(testMethod, testClass, allTestMethods);
 
@@ -685,7 +691,7 @@
             //
             // HINT: If threadPoolSize specified, run this method in its own pool thread.
             //
-            if (testMethod.getThreadPoolSize() > 1) {
+            if (testMethod.getThreadPoolSize() > 1 && testMethod.getInvocationCount() > 1) {
               try {
                 result = invokePooledTestMethods(testMethod, allTestMethods, suite, 
                     parameters, groupMethods, testContext);
@@ -704,29 +710,54 @@
 
 
               Map<String, String> allParameterNames = new HashMap<String, String>();
-//              boolean isStatic = (dataProvider.getModifiers() & Modifier.STATIC) != 0;
               Object instance = testClass.getInstances(true)[0];
-              Iterator<Object[]> allParameterValues =
-                Parameters.handleParameters(testMethod, allParameterNames, 
-                    instance, parameters, suite, m_annotationFinder, testContext);
+              ParameterBag bag= handleParameters(testMethod, 
+                  instance, allParameterNames, parameters, suite, testContext);
 
-              while (allParameterValues.hasNext()) {
-                Object[] parameterValues= allParameterValues.next();
-//                Object[] instances = testClass.getInstances(true);
-
+              if(bag.hasErrors()) {
+                failureCount = handleInvocationResults(testMethod, 
+                    bag.errorResults, failureCount, expectedExceptionClasses, true);
+                // there is nothing we can do more
+                continue;
+              }
+              
+              Iterator<Object[]> allParameterValues= bag.parameterValues;
+              
+              if(testMethod.getThreadPoolSize() > 1) {
                 try {
-                    result = invokeTestMethod(instances,
-                                              testMethod,
-                                              parameterValues,
-                                              suite,
-                                              allParameterNames,
-                                              testClass,
-                                              beforeMethods,
-                                              afterMethods,
-                                              groupMethods);
+                  result = invokePooledTestMethods(instances, 
+                      testMethod, 
+                      allTestMethods, 
+                      beforeMethods, 
+                      afterMethods, 
+                      groupMethods, 
+                      suite, 
+                      parameters, 
+                      allParameterNames, 
+                      allParameterValues);
                 }
                 finally {
                   failureCount = handleInvocationResults(testMethod, result, failureCount, expectedExceptionClasses, true);
+                }                
+              }
+              else {
+                while (allParameterValues.hasNext()) {
+                  Object[] parameterValues= allParameterValues.next();
+  
+                  try {
+                      result = invokeTestMethod(instances,
+                                                testMethod,
+                                                parameterValues,
+                                                suite,
+                                                allParameterNames,
+                                                testClass,
+                                                beforeMethods,
+                                                afterMethods,
+                                                groupMethods);
+                  }
+                  finally {
+                    failureCount = handleInvocationResults(testMethod, result, failureCount, expectedExceptionClasses, true);
+                  }
                 }
               } // for parameters
             }
@@ -760,10 +791,65 @@
     
   } // invokeTestMethod
 
+  private ParameterBag handleParameters(ITestNGMethod testMethod,
+      Object instance,
+      Map<String, String> allParameterNames,
+      Map<String, String> parameters,
+      XmlSuite suite,
+      ITestContext testContext)
+  {
+    try {
+      return new ParameterBag(Parameters.handleParameters(testMethod,
+          allParameterNames, instance, parameters, suite, m_annotationFinder, testContext), null);
+    }
+    catch(Throwable cause) {
+      return new ParameterBag(null, 
+          new TestResult(
+              testMethod.getTestClass(), 
+              instance, 
+              testMethod, 
+              cause, 
+              System.currentTimeMillis(),
+              System.currentTimeMillis()));
+    }
+    
+  }
+  
+  private List<ITestResult> invokePooledTestMethods(Object[] instances,
+      ITestNGMethod testMethod, 
+      ITestNGMethod[] allTestMethods, 
+      ITestNGMethod[] beforeMethods,
+      ITestNGMethod[] afterMethods,
+      ConfigurationGroupMethods groupMethods,
+      XmlSuite suite, 
+      Map<String, String> parameters, 
+      Map<String, String> allParameterNames,
+      Iterator<Object[]> allParameterValues) 
+  {
+    List<ITestResult> result= new ArrayList<ITestResult>();
+    //
+    // Create the workers
+    //
+    List<IMethodWorker> workers= new ArrayList<IMethodWorker>();    
+    
+    while (allParameterValues.hasNext()) {
+      Object[] parameterValues= allParameterValues.next();
+
+      workers.add(new DataTestMethodWorker(instances, 
+          testMethod,
+          parameterValues,
+          beforeMethods,
+          afterMethods,
+          groupMethods,
+          suite,
+          allParameterNames));
+    }
+    
+    return runWorkers(testMethod, workers, testMethod.getThreadPoolSize(), groupMethods, suite, parameters);
+  }
+  
   /**
    * Invokes a method that has a specified threadPoolSize. 
-   * To reduce thread contention and also to correctly handle thread-confinement
-   * this method invokes the @BeforeGroups and @AfterGroups corresponding to the current @Test method.
    */
   private List<ITestResult> invokePooledTestMethods(ITestNGMethod testMethod, 
                                                     ITestNGMethod[] allTestMethods, 
@@ -772,19 +858,11 @@
                                                     ConfigurationGroupMethods groupMethods,
                                                     ITestContext testContext) 
   {
-    // HINT: invoke @BeforeGroups on the original method (reduce thread contention, and also solve thread confinement)
-    ITestClass testClass= testMethod.getTestClass();
-    Object[] instances = testClass.getInstances(true);
-    for(Object instance: instances) {
-      invokeBeforeGroupsConfigurations(testClass, testMethod, groupMethods, suite, parameters, instance);
-    }
-    
-    
     List<ITestResult> result= new ArrayList<ITestResult>();
     //
     // Create the workers
     //
-    List<TestMethodWorker> workers= new ArrayList<TestMethodWorker>();    
+    List<IMethodWorker> workers= new ArrayList<IMethodWorker>();    
     List<ITestNGMethod> clones= new ArrayList<ITestNGMethod>(testMethod.getInvocationCount());
     
     for (int i = 0; i < testMethod.getInvocationCount(); i++) {
@@ -802,7 +880,7 @@
     }
 
     try {
-      result = runWorkers(testMethod, workers, testMethod.getThreadPoolSize());
+      result = runWorkers(testMethod, workers, testMethod.getThreadPoolSize(), groupMethods, suite, parameters);
     }
     finally {
       for(ITestNGMethod clone: clones) {
@@ -810,10 +888,6 @@
       }
       clones.clear();
       clones= null;
-
-      for(Object instance: instances) {
-        invokeAfterGroupsConfigurations(testClass, testMethod, groupMethods, suite, parameters, instance);
-      }
     }
     
     return result;
@@ -888,11 +962,28 @@
     return failureCount;
   }
   
-  private List<ITestResult> runWorkers(ITestNGMethod testMethod, List<TestMethodWorker> workers, int threadPoolSize)
+  /**
+   * To reduce thread contention and also to correctly handle thread-confinement
+   * this method invokes the @BeforeGroups and @AfterGroups corresponding to the current @Test method.
+   */
+  private List<ITestResult> runWorkers(ITestNGMethod testMethod, 
+      List<IMethodWorker> workers, 
+      int threadPoolSize, 
+      ConfigurationGroupMethods groupMethods, 
+      XmlSuite suite, 
+      Map<String, String> parameters)
   {
+    // HINT: invoke @BeforeGroups on the original method (reduce thread contention, and also solve thread confinement)
+    ITestClass testClass= testMethod.getTestClass();
+    Object[] instances = testClass.getInstances(true);
+    for(Object instance: instances) {
+      invokeBeforeGroupsConfigurations(testClass, testMethod, groupMethods, suite, parameters, instance);
+    }
+    
+    
     long maxTimeOut= -1; // 10 seconds
 
-    for(TestMethodWorker tmw : workers) {
+    for(IMethodWorker tmw : workers) {
       long mt= tmw.getMaxTimeOut();
       if(mt > maxTimeOut) {
         maxTimeOut= mt;
@@ -905,10 +996,14 @@
     // Collect all the TestResults
     //
     List<ITestResult> result = new ArrayList<ITestResult>();
-    for (TestMethodWorker tmw : workers) {
+    for (IMethodWorker tmw : workers) {
       result.addAll(tmw.getTestResults());
     }
     
+    for(Object instance: instances) {
+      invokeAfterGroupsConfigurations(testClass, testMethod, groupMethods, suite, parameters, instance);
+    }
+    
     return result;
   }
 
@@ -1164,4 +1259,71 @@
   private void log(int level, String s) {
     Utils.log("Invoker " + Thread.currentThread().hashCode(), level, s);
   }
+  
+  private class DataTestMethodWorker implements IMethodWorker {
+    final Object[] m_instances;
+    final ITestNGMethod m_testMethod;
+    final ITestNGMethod[] m_beforeMethods;
+    final ITestNGMethod[] m_afterMethods;
+    final ConfigurationGroupMethods m_groupMethods;
+    final Object[] m_parameters;
+    final XmlSuite m_suite;
+    final Map<String, String> m_allParameterNames;
+    
+    List<ITestResult> m_results;
+    
+    public DataTestMethodWorker(Object[] instances, 
+        ITestNGMethod testMethod, 
+        Object[] params,
+        ITestNGMethod[] befores, 
+        ITestNGMethod[] afters, 
+        ConfigurationGroupMethods groupMethods, 
+        XmlSuite suite, 
+        Map<String, String> paramNames) {
+      m_instances= instances;
+      m_testMethod= testMethod;
+      m_parameters= params;
+      m_beforeMethods= befores;
+      m_afterMethods= afters;
+      m_groupMethods= groupMethods;
+      m_suite= suite;
+      m_allParameterNames= paramNames;
+    }
+    
+    public long getMaxTimeOut() {
+      return 0;
+    }
+
+    public void run() {
+      m_results= invokeTestMethod(m_instances,
+          m_testMethod,
+          m_parameters,
+          m_suite,
+          m_allParameterNames,
+          m_testMethod.getTestClass(),
+          m_beforeMethods,
+          m_afterMethods,
+          m_groupMethods);
+    }
+
+    public List<ITestResult> getTestResults() {
+      return m_results;
+    }
+  }
+  
+  private static class ParameterBag {
+    final Iterator<Object[]> parameterValues;
+    final List<ITestResult> errorResults= new ArrayList<ITestResult>();
+    
+    public ParameterBag(Iterator<Object[]> params, TestResult tr) {
+      parameterValues= params;
+      if(tr != null) {
+        errorResults.add(tr);
+      }
+    }
+    
+    public boolean hasErrors() {
+      return !errorResults.isEmpty();
+    }
+  }
 }
diff --git a/src/main/org/testng/internal/TestMethodWorker.java b/src/main/org/testng/internal/TestMethodWorker.java
index f8db8e3..d9f832c 100644
--- a/src/main/org/testng/internal/TestMethodWorker.java
+++ b/src/main/org/testng/internal/TestMethodWorker.java
@@ -2,8 +2,10 @@
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.testng.ClassMethodMap;
 import org.testng.ITestClass;
@@ -32,8 +34,8 @@
   protected IInvoker m_invoker = null;
   protected Map<String, String> m_parameters = null;
   protected XmlSuite m_suite = null;
-  protected Map<ITestClass, ITestClass> m_invokedBeforeClassMethods = null;
-  protected Map<ITestClass, ITestClass> m_invokedAfterClassMethods = null;
+//  protected Map<ITestClass, Set<Object>> m_invokedBeforeClassMethods = null;
+//  protected Map<ITestClass, Set<Object>> m_invokedAfterClassMethods = null;
   protected ITestNGMethod[] m_allTestMethods;
   protected List<ITestResult> m_testResults = new ArrayList<ITestResult>();
   protected ConfigurationGroupMethods m_groupMethods = null;
@@ -44,8 +46,6 @@
                           MethodInstance[] testMethods,
                           XmlSuite suite,
                           Map<String, String> parameters,
-                          Map<ITestClass, ITestClass> invokedBeforeClassMethods,
-                          Map<ITestClass, ITestClass> invokedAfterClassMethods,
                           ITestNGMethod[] allTestMethods,
                           ConfigurationGroupMethods groupMethods,
                           ClassMethodMap classMethodMap,
@@ -55,8 +55,8 @@
     m_testMethods = testMethods;
     m_suite = suite;
     m_parameters = parameters;
-    m_invokedBeforeClassMethods = invokedBeforeClassMethods;
-    m_invokedAfterClassMethods = invokedAfterClassMethods;
+//    m_invokedBeforeClassMethods = invokedBeforeClassMethods;
+//    m_invokedAfterClassMethods = invokedAfterClassMethods;
     m_allTestMethods = allTestMethods;
     m_groupMethods = groupMethods;
     m_classMethodMap = classMethodMap;
@@ -100,7 +100,7 @@
   
       ITestClass testClass = tm.getTestClass();
 
-      invokeBeforeClassMethods(testClass);
+      invokeBeforeClassMethods(testClass, m_testMethods[indexMethod]);
       
       //
       // Invoke test method
@@ -109,7 +109,7 @@
         invokeTestMethods(tm, m_testMethods[indexMethod].getInstances(), m_testContext);
       }
       finally {
-        invokeAfterClassMethods(testClass, tm);
+        invokeAfterClassMethods(testClass, m_testMethods[indexMethod]);
       }
     }
   }
@@ -139,10 +139,10 @@
   //
   // Invoke the before class methods if not done already
   //
-  protected void invokeBeforeClassMethods(ITestClass testClass) {
+  protected void invokeBeforeClassMethods(ITestClass testClass, MethodInstance mi) {
     // if no BeforeClass than return immediately
     // used for parallel case when BeforeClass were already invoked
-    if(null == m_invokedBeforeClassMethods) {
+    if( (null == m_classMethodMap) || (null == m_classMethodMap.getInvokedBeforeClassMethods())) {
       return;
     }
     ITestNGMethod[] classMethods= testClass.getBeforeClassMethods();
@@ -152,22 +152,30 @@
     
     // the whole invocation must be synchronized as other threads must
     // get a full initialized test object (not the same for @After)
-    synchronized(m_invokedBeforeClassMethods) {
-      if (! m_invokedBeforeClassMethods.containsKey(testClass)) {  
-        m_invokedBeforeClassMethods.put(testClass, testClass);
-        m_invoker.invokeConfigurations(testClass,
-                                       testClass.getBeforeClassMethods(),
-                                       m_suite,
-                                       m_parameters,
-                                       null /* instance */);
+    Map<ITestClass, Set<Object>> invokedBeforeClassMethods= m_classMethodMap.getInvokedBeforeClassMethods();
+    synchronized(invokedBeforeClassMethods) {
+      Set<Object> instances= invokedBeforeClassMethods.get(testClass);
+      if(null == instances) {
+        instances= new HashSet<Object>();
+        invokedBeforeClassMethods.put(testClass, instances);
+      }
+      for(Object instance: mi.getInstances()) {
+        if (! instances.contains(instance)) {  
+          instances.add(instance);
+          m_invoker.invokeConfigurations(testClass,
+                                         testClass.getBeforeClassMethods(),
+                                         m_suite,
+                                         m_parameters,
+                                         instance);
+        }
       }
     }
   }
   
-  protected void invokeAfterClassMethods(ITestClass testClass, ITestNGMethod tm) {
+  protected void invokeAfterClassMethods(ITestClass testClass, MethodInstance mi) {
     // if no BeforeClass than return immediately
     // used for parallel case when BeforeClass were already invoked
-    if( (null == m_invokedAfterClassMethods) || (null == m_classMethodMap)) {
+    if( (null == m_classMethodMap) || (null == m_classMethodMap.getInvokedAfterClassMethods()) ) {
       return;
     }
     ITestNGMethod[] afterClassMethods= testClass.getAfterClassMethods();
@@ -179,21 +187,29 @@
     //
     // Invoke after class methods if this test method is the last one
     //
+    List<Object> invokeInstances= new ArrayList<Object>();
+    ITestNGMethod tm= mi.getMethod();
     if (m_classMethodMap.removeAndCheckIfLast(tm)) {
-      boolean invokeAfter= false;
-      synchronized(m_invokedAfterClassMethods) {
-        if (! m_invokedAfterClassMethods.containsKey(testClass)) {
-          m_invokedAfterClassMethods.put(testClass, testClass);
-          invokeAfter= true;
+      Map<ITestClass, Set<Object>> invokedAfterClassMethods= m_classMethodMap.getInvokedAfterClassMethods();
+      synchronized(invokedAfterClassMethods) {
+        Set<Object> instances = invokedAfterClassMethods.get(testClass);
+        if(null == instances) {
+          instances= new HashSet<Object>();
+          invokedAfterClassMethods.put(testClass, instances);
+        }
+        for(Object inst: mi.getInstances()) {
+          if(! instances.contains(inst)) {
+            invokeInstances.add(inst);
+          }
         }
       }
       
-      if(invokeAfter) {
+      for(Object inst: invokeInstances) {
         m_invoker.invokeConfigurations(testClass,
                                        afterClassMethods,
                                        m_suite,
                                        m_parameters,
-                                       null /* instance */);
+                                       inst);
       }
     }
   }
@@ -210,24 +226,6 @@
     return m_testResults;
   }
   
-  private boolean isLastTestMethodForClass(ITestNGMethod tm, ITestNGMethod[] testMethods)
-  {
-    for (int i = testMethods.length - 1; i >= 0; i--) {
-      ITestNGMethod thisMethod = testMethods[i];
-      ITestClass testClass = tm.getTestClass();
-      if (thisMethod.getTestClass().equals(testClass)) {
-        if (thisMethod.equals(tm)) {
-          return true;
-        }
-        else {
-          return false;
-        }
-      }
-    }
-    
-    return false;
-  }
-
   private void ppp(String s) {
     Utils.log("TestMethodWorker", 2, ThreadUtil.currentThreadInfo() + ":" + s);
   }
@@ -239,7 +237,8 @@
 
 class SingleTestMethodWorker extends TestMethodWorker {
   private static final ConfigurationGroupMethods EMPTY_GROUP_METHODS=
-    new ConfigurationGroupMethods(new ITestNGMethod[0], new HashMap<String, List<ITestNGMethod>>(), new HashMap<String, List<ITestNGMethod>>());
+    new ConfigurationGroupMethods(new ITestNGMethod[0], 
+        new HashMap<String, List<ITestNGMethod>>(), new HashMap<String, List<ITestNGMethod>>());
   
   public SingleTestMethodWorker(IInvoker invoker, 
                                 MethodInstance testMethod,
@@ -252,8 +251,6 @@
           new MethodInstance[] {testMethod},
           suite,
           parameters,
-          null,
-          null,
           allTestMethods,
           EMPTY_GROUP_METHODS,
           null,
diff --git a/test/src/test/dataprovider/FailingDataProvider.java b/test/src/test/dataprovider/FailingDataProvider.java
new file mode 100644
index 0000000..e325d61
--- /dev/null
+++ b/test/src/test/dataprovider/FailingDataProvider.java
@@ -0,0 +1,21 @@
+package test.dataprovider;

+

+import org.testng.Assert;

+import org.testng.annotations.DataProvider;

+import org.testng.annotations.Test;

+

+

+/**

+ * This class/interface 

+ */

+public class FailingDataProvider {

+  @DataProvider

+  public Object[][] throwsExpectedException() {

+    throw new RuntimeException("expected exception from @DP");

+  }

+  

+  @Test(dataProvider="throwsExpectedException")

+  public void dpThrowingException() {

+    Assert.fail("Method should never get invoked");

+  }

+}

diff --git a/test/src/test/dataprovider/FailingDataProviderTest.java b/test/src/test/dataprovider/FailingDataProviderTest.java
new file mode 100644
index 0000000..53bca1d
--- /dev/null
+++ b/test/src/test/dataprovider/FailingDataProviderTest.java
@@ -0,0 +1,23 @@
+package test.dataprovider;

+

+import org.testng.Assert;

+import org.testng.TestListenerAdapter;

+import org.testng.TestNG;

+import org.testng.annotations.Test;

+

+

+/**

+ * TESTNG-142:

+ * Exceptions in DataProvider are not reported as failed test

+ */

+public class FailingDataProviderTest {

+  @Test

+  public void failingDataProvider() {

+    TestNG testng= new TestNG();

+    testng.setTestClasses(new Class[] {FailingDataProvider.class});

+    TestListenerAdapter tla = new TestListenerAdapter();

+    testng.addListener(tla);

+    testng.run();

+    Assert.assertEquals(tla.getFailedTests().size(), 1, "Test method should be marked as failed");

+  }

+}

diff --git a/test/src/test/factory/classconf/XClassOrderWithFactory.java b/test/src/test/factory/classconf/XClassOrderWithFactory.java
new file mode 100644
index 0000000..ebb46f7
--- /dev/null
+++ b/test/src/test/factory/classconf/XClassOrderWithFactory.java
@@ -0,0 +1,37 @@
+package test.factory.classconf;

+

+import org.testng.annotations.AfterClass;

+import org.testng.annotations.BeforeClass;

+import org.testng.annotations.Factory;

+import org.testng.annotations.Test;

+

+

+/**

+ * This class/interface 

+ */

+public class XClassOrderWithFactory {

+  public static final String EXPECTED_LOG= "BTABTABTA";

+  public static final StringBuffer LOG= new StringBuffer();

+  

+  @Factory

+  public Object[] createInstances() throws Exception {

+      return new Object[] {

+              new XClassOrderTest(), new XClassOrderTest(), new XClassOrderTest()

+          };

+  }

+  

+  public static class XClassOrderTest {

+    @BeforeClass

+    public void beforeClass() {

+      LOG.append("B");

+    }

+  

+    public @Test void test() {

+      LOG.append("T");

+    }

+  

+    public @AfterClass void afterClass() {

+      LOG.append("A");

+    }

+  }

+}

diff --git a/test/src/test/factory/classconf/XClassOrderWithFactoryTest.java b/test/src/test/factory/classconf/XClassOrderWithFactoryTest.java
new file mode 100644
index 0000000..ed6d68b
--- /dev/null
+++ b/test/src/test/factory/classconf/XClassOrderWithFactoryTest.java
@@ -0,0 +1,23 @@
+package test.factory.classconf;

+

+import org.testng.Assert;

+import org.testng.TestListenerAdapter;

+import org.testng.TestNG;

+import org.testng.annotations.BeforeClass;

+import org.testng.annotations.Test;

+

+

+/**

+ * This class/interface 

+ */

+public class XClassOrderWithFactoryTest {

+  @Test

+  public void testBeforeAfterClassInvocationsWithFactory() {

+    TestNG testng= new TestNG();

+    testng.setTestClasses(new Class[] {XClassOrderWithFactory.class});

+    TestListenerAdapter tla = new TestListenerAdapter();

+    testng.addListener(tla);

+    testng.run();

+    Assert.assertEquals(XClassOrderWithFactory.LOG.toString(), XClassOrderWithFactory.EXPECTED_LOG);

+  }

+}

diff --git a/test/testng.xml b/test/testng.xml
index 06ae657..07d30f6 100644
--- a/test/testng.xml
+++ b/test/testng.xml
@@ -80,6 +80,12 @@
       <class name="test.configuration.BaseGroupsTest" />
     </classes>
   </test>
+	
+	<test name="Factory tests">
+		<classes>
+			<class name="test.factory.classconf.XClassOrderWithFactoryTest" />
+		</classes>
+	</test>
   
   <test name="Basic" >
     <classes>
@@ -348,6 +354,7 @@
       <class name="test.dataprovider.StaticDataProviderSampleTest" />
       <class name="test.dataprovider.UnnamedDataProviderTest" />
       <class name="test.dataprovider.TestContextTest" />
+			<class name="test.dataprovider.FailingDataProviderTest" />
     </classes>
   </test>