[jboss-jira] [JBoss JIRA] Commented: (JGRP-850) Fix output leak in testNG test reporting

[Richard Achmatowicz (JIRA)]

    [ https://jira.jboss.org/jira/browse/JGRP-850?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12435809#action_12435809 ] 

Richard Achmatowicz commented on JGRP-850:
------------------------------------------

I think I found the problem, but don't know yet what to do about it. 
Bela's modification to JUnitXMLReporter uses two structures to record test data for test cases:
classes map: class -> List<test result>
outputs map: class -> <StringBuffer for out, StringBuffer for err>
and a overidden version of PrintStream which allows a thread to write output for a test case to the outputs map, using the class name as a key into the map.

It works roughly like this:
(i) the overridden version of PrintStream is installed
(ii) before a test case method is executed by a thread, some thread-local storage is used to store the name of the test case class containing the method ( e.g. if i'm executing test method X.x(),
then class name X gets stored in the thread  local storage). This happens before any test methods for that class are executed.
(iii) when the test case method executes and uses println() or print() to write output, the overridden PrintStream looks up the classname in the thread local storage, uses it to get the string buffers
for that test case class X and the output is written into the buffers associated with that test case.
The buffers are then later used in report generation.

The initialization of the new PrintStream takes place in a test listener method called onStart(), which runs at the start of every <test> element as it is processed.

The initialization of the class name in thyread-local storage is done in a test listener method called onTestStart() which is supposed to be called before any tests are executed.
This is sort of true. However, before the test method is executed, any number of other methods (e.g. @BeforeClass, @BeforeSuite, @BeforeMethod) are getting called too. And in those cases, the thread-local storage will not have yet been initialized, because onTestStart() has not yet been called.  In fact, there is a listener method called onConfigurationSuccess() which gets called after any configuration method (e.g. @BeforeClass, @BeforeSuite, @BeforeMethod) is called. If this is enabled, you can see that it gets called before onTestStart().

So although the usual System.out and System.err are being replaced with the new PrintStream in onStart(),  as each test method is run, the thread-local name of the current test class is not initialised until onTestStart() - and in the interim, configuration methods make use of the overridden PrintStream, which requires the thread local storage to store the output for the test case in the correct StringBuffer. This is how the output is leaking, according to my understanding of things. take any test case in the functional suite, add in some println statements to a @Before message, and you'll see the output appear on the console.

The problem is that the initialization of thread local storage is happening too late. We need a method to be called before any configurators are called for the test case.  Even if we initialised the classes map (class->List<result>)and the outputs map(class->Tuple<StringBuffer,StringBuffer>) in onStart(), this would still not allow the new PrintStream class to get the classname for the test, as it doesn't get set until onTestStart().

I've had a look in the testNG source (in and around TestRunner.run(), which kicks off the execution of a <test> element) and it wasn't clear to me that there was such a hook. It seems that there used to be in earlier versions:

The listener IInvokedMethodListener <http://testng.org/javadocs/org/testng/IInvokedMethodListener.html> allows you to be notified whenever TestNG is about to invoke a test (annotated with @Test) or configuration (annotated with any of the @Before or @After annotation) method. You need to implement the following interface:

public interface IInvokedMethodListener extends ITestNGListener {
 void beforeInvocation(IInvokedMethod method, ITestResult testResult);
 void afterInvocation(IInvokedMethod method, ITestResult testResult);
}

Having a beforeInvocation() callback as described would have allowed initializing the thread-local storage before any configuration methods or test methods were executed.


> Fix output leak in testNG test reporting
> ----------------------------------------
>
>                 Key: JGRP-850
>                 URL: https://jira.jboss.org/jira/browse/JGRP-850
>             Project: JGroups
>          Issue Type: Bug
>            Reporter: Richard Achmatowicz
>            Assignee: Richard Achmatowicz
>            Priority: Minor
>             Fix For: 2.7
>
>
> org.jgroups.util.JUnitXmlReporter is a class which takes output from testNG test cases and prepares xml test case reports in JUnit-compatible format. These xml files are then used to generate JUnit style reports.
> At present, output from test case methods which execute before the main test case (testNG configurator methods, such as @BeforeSuite, @AfterSite, etc) written to System.out and System.err is leaking to the console and not being captured with the rest of the testNG output. 

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira