[testng] 137/355: Fix NPE in Reporter.log()

[Eugene Zhukov]

This is an automated email from the git hooks/post-receive script.

eugene-guest pushed a commit to annotated tag OpenBSD
in repository testng.

commit 38542477be2810926b6187f7f42159929a6e8588
Author: Krishnan Mahadevan <Krishnan.mahadevan1978 at gmail.com>
Date:   Sat Jan 24 11:50:29 2015 +0530

    Fix NPE in Reporter.log()
    
    Root cause :
    Changes to Reporter class to use hasCode() of
    TestResult : https://github.com/cbeust/testng/commit/c9a91722631f3b2f9bcc5a300e7a84c61cd4764f
    
    Fixed this by persisting all the orphaned output
    (output which is not associated with any TestResult) into
    a temporary ThreadLocal ArrayList and then associated it
    back to a TestResult once we found a valid TestResult
    object for the current Thread.
    
    * Added a null check in getOutput() to prevent NPE.
    * Added a unit test.
---
 src/main/java/org/testng/Reporter.java       | 28 +++++++++++++++++++++++++++-
 src/test/java/test/reports/ReporterTest.java | 20 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/testng/Reporter.java b/src/main/java/org/testng/Reporter.java
index b2f30eb..79826a3 100755
--- a/src/main/java/org/testng/Reporter.java
+++ b/src/main/java/org/testng/Reporter.java
@@ -1,11 +1,13 @@
 package org.testng;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Vector;
 
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
+import org.testng.internal.TestResult;
 import org.testng.util.Strings;
 
 /**
@@ -19,7 +21,7 @@ import org.testng.util.Strings;
  * The reporter keeps a combined output of strings (in m_output) and also
  * a record of which method output which line.  In order to do this, callers
  * specify what the current method is with setCurrentTestResult() and the
- * Reporter maintaing a mapping of each test result with a list of integers.
+ * Reporter maintains a mapping of each test result with a list of integers.
  * These integers are indices in the combined output (avoids duplicating
  * the output).
  *
@@ -41,6 +43,9 @@ public class Reporter {
   private static Map<Integer, List<Integer>> m_methodOutputMap = Maps.newHashMap();
 
   private static boolean m_escapeHtml = false;
+  //This variable is responsible for persisting all output that is yet to be associated with any
+  //valid TestResult objects.
+  private static ThreadLocal<List<String>> orphanedOutput = new InheritableThreadLocal<List<String>>();
 
   public static void setCurrentTestResult(ITestResult m) {
     m_currentTestResult.set(m);
@@ -71,13 +76,30 @@ public class Reporter {
       s = Strings.escapeHtml(s);
     }
 
+    if (m == null) {
+      //Persist the output temporarily into a Threadlocal String list.
+      if (orphanedOutput.get() == null) {
+        orphanedOutput.set(new ArrayList<String>());
+      }
+      orphanedOutput.get().add(s);
+      return;
+    }
+
     // synchronization needed to ensure the line number and m_output are updated atomically
     int n = getOutput().size();
+
     List<Integer> lines = m_methodOutputMap.get(m.hashCode());
     if (lines == null) {
       lines = Lists.newArrayList();
       m_methodOutputMap.put(m.hashCode(), lines);
     }
+    //Lets check if there were already some orphaned output for the current Thread.
+    if (orphanedOutput.get() != null) {
+      n = n + orphanedOutput.get().size();
+      getOutput().addAll(orphanedOutput.get());
+      //since we have already added all of the orphaned output to the current TestResult, lets clear it off
+      orphanedOutput.remove();
+    }
     lines.add(n);
     getOutput().add(s);
   }
@@ -145,6 +167,10 @@ public class Reporter {
 
   public static synchronized List<String> getOutput(ITestResult tr) {
     List<String> result = Lists.newArrayList();
+    if (tr == null) {
+      //guard against a possible NPE in scenarios wherein the test result object itself could be a null value.
+      return result;
+    }
     List<Integer> lines = m_methodOutputMap.get(tr.hashCode());
     if (lines != null) {
       for (Integer n : lines) {
diff --git a/src/test/java/test/reports/ReporterTest.java b/src/test/java/test/reports/ReporterTest.java
new file mode 100644
index 0000000..4a58143
--- /dev/null
+++ b/src/test/java/test/reports/ReporterTest.java
@@ -0,0 +1,20 @@
+package test.reports;
+
+import org.testng.*;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+import java.util.Set;
+
+ at Listeners(ReporterTest.class)
+public class ReporterTest extends TestListenerAdapter {
+    @Override public void onStart (ITestContext testContext) {
+        Reporter.log ("foo");
+    }
+    @Test
+    public void testMethod() {
+        Reporter.log ("bar"); // This line is required. Else the log that was triggered from onStart() would never be
+        // persisted at all.
+        Assert.assertTrue (Reporter.getOutput ().size () == 2);
+    }
+}

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-java/testng.git