Race condition with maven-surefire

This is a duplicate of 
"https://code.google.com/p/junitparams/issues/detail?can=2&start=0&num=100&q=&co
lspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary&groupby=&sort=
&id=60". I am re-posting it here since it is marked as invalid and I am not 
sure, it will be monitored any more.

This is the summary from Issue 60.

"
Reported by ngr...@inco5.com, Apr 9, 2014
See stack trace at the bottom... My builds randomly fail, at least when using 
"parametersForTest"-style parameters. This never occurs within my IDE, so it 
may be that Surefire is doing something wrong that's causing this downstream 
effect. But it does happen for many different parameterized tests, and it's not 
caused by null test values.

What steps will reproduce the problem?

1. Run any parameterized test using Maven Surefire 2.16 or 2.17 under JDK 7 or 
JDK 8
2. Keep running the same tests (`mvn test`) until you get a NullPointerException

What is the expected output? What do you see instead?

Expected output is that the test executes every time.

What version of the product are you using? On what operating system?

I'm using version 1.0.2 on both OS X and Linux. I've tested with JDK 7 and JDK 
8. The problem seems to be much more frequent using JDK 8.

Please provide any additional information below.

[INFO] parallel='none', perCoreThreadCount=true, threadCount=0, 
useUnlimitedThreads=false, threadCountSuites=0, threadCountClasses=0, 
threadCountMethods=0

[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-surefire-plugin:2.16:test (default-test) on 
project test: Execution default-test of goal 
org.apache.maven.plugins:maven-surefire-plugin:2.16:test failed: There was an 
error in the forked process
[ERROR] org.apache.maven.surefire.testset.TestSetFailedException: 
java.lang.NullPointerException; nested exception is 
java.lang.NullPointerException: null
[ERROR] java.lang.NullPointerException
[ERROR] at 
org.apache.maven.surefire.common.junit4.JUnit4RunListener.extractClassName(JUnit
4RunListener.java:178)
[ERROR] at 
org.apache.maven.surefire.common.junit4.JUnit4RunListener.getClassName(JUnit4Run
Listener.java:154)
[ERROR] at 
org.apache.maven.surefire.common.junit4.JUnit4RunListener.createReportEntry(JUni
t4RunListener.java:149)
[ERROR] at 
org.apache.maven.surefire.common.junit4.JUnit4RunListener.testStarted(JUnit4RunL
istener.java:87)
[ERROR] at 
org.junit.runner.notification.RunNotifier$3.notifyListener(RunNotifier.java:115)
[ERROR] at 
org.junit.runner.notification.RunNotifier$SafeNotifier.run(RunNotifier.java:61)
[ERROR] at 
org.junit.runner.notification.RunNotifier.fireTestStarted(RunNotifier.java:112)
[ERROR] at 
junitparams.internal.ParameterisedTestMethodRunner.runTestMethod(ParameterisedTe
stMethodRunner.java:41)
[ERROR] at 
junitparams.internal.ParameterisedTestClassRunner.runParameterisedTest(Parameter
isedTestClassRunner.java:143)
[ERROR] at junitparams.JUnitParamsRunner.runChild(JUnitParamsRunner.java:405)
[ERROR] at junitparams.JUnitParamsRunner.runChild(JUnitParamsRunner.java:383)
[ERROR] at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
[ERROR] at 
org.apache.maven.surefire.junitcore.pc.Scheduler$1.run(Scheduler.java:258)
[ERROR] at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
[ERROR] at java.util.concurrent.FutureTask.run(FutureTask.java:262)
[ERROR] at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
[ERROR] at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[ERROR] at java.lang.Thread.run(Thread.java:744)
[ERROR] -> [Help 1]
"

I can reproduce it on Windows and I spent some time to find the problem in the 
code. Here is the explanation, again written in Issue 60 as well:

"
There is a bug in the code for parallel execution.

JUnitParamsRunner extends BlockJUnit4ClassRunner

    protected Statement methodInvoker(FrameworkMethod method, Object test) {
        Statement methodInvoker = this.parameterisedRunner.parameterisedMethodInvoker(method, test);
        if(methodInvoker == null) {
            methodInvoker = super.methodInvoker(method, test);
        }

        return methodInvoker;
    }

methodInvoker though can be executed in parallel for multiple tests and this is 
not defended by JUnitParams. This method calls code in 
ParameterisedTestClassRunner later in its execution. More concretely this is:

return new InvokeParameterisedMethod(method, testClass, 
parameterisedMethod.currentParamsFromAnnotation(), parameterisedMethod.count());

in

private Statement buildMethodInvoker(FrameworkMethod method, Object testClass, 
TestMethod testMethod)

Note "parameterisedMethod.currentParamsFromAnnotation()", its implementation is:

return this.method.parametersSets()[this.nextCount()];

where

public int nextCount() {
        return this.count++;
    }

And this is the problem. Since this can be executed in parallel, the generated 
"count" may be wrong sometimes. And because of that it will take a wrong 
parameterSet. 

When the test executes, it reached the following method:

"
private Description findChildForParams(Statement methodInvoker, Description 
methodDescription)
"

The following code can be found there:

"
            do {
                if(!var3.hasNext()) {
                    return null;
                }

                child = (Description)var3.next();
                parameterisedInvoker = this.findParameterisedMethodInvokerInChain(methodInvoker);
            } while(!child.getMethodName().startsWith(parameterisedInvoker.getParamsAsString()));
"

Because of the wrong parametersSet, this loop will end up with:

"
                if(!var3.hasNext()) {
                    return null;
                }
"

and will return null.

And now, take a look at the following method:

"
    void runTestMethod(Statement methodInvoker, RunNotifier notifier) {
        Description methodDescription = this.method.describe();
        Description methodWithParams = this.findChildForParams(methodInvoker, methodDescription);
        this.runMethodInvoker(notifier, methodDescription, methodInvoker, methodWithParams);
    }
"

Apparently methodWithParams is null and this is the root cause to end up with 
NullPointerException at at 
org.apache.maven.surefire.common.junit4.JUnit4RunListener.extractClassName.

Ideally this should be reworked but the following override of JUnitParamsRunner 
seems to be working. It just synchronizes the methodinvoker:

"
import junitparams.JUnitParamsRunner;
import org.junit.runners.model.FrameworkMethod;
import org.junit.runners.model.InitializationError;
import org.junit.runners.model.Statement;

/**
 * Custom {@link JUnitParamsRunner} which fixes a synchronization issue in the original
 * JUnitParamsRunner (at least in version 1.0.3)
 */
public class CustomJUnitParamsRunner extends JUnitParamsRunner {

    /**
     * Creates an instance of {@link CustomJUnitParamsRunner} for a given test class.
     *
     * @param testClass the test class to be run using this junit runner
     * @throws InitializationError if the test class is malformed
     */
    public CustomJUnitParamsRunner(Class<?> testClass) throws InitializationError {
        super(testClass);
    }

    @Override
    protected synchronized Statement methodInvoker(FrameworkMethod method, Object test) {
        return super.methodInvoker(method, test);
    }
}

"
"
Original issue reported on code.google.com by tarantul...@abv.bg on 13 Jan 2015 at 5:41
shivadorepally / junitparams

Race condition with maven-surefire #64
