testng parallel methods mode failures

[adrian-herscu]

Recently, I discovered that when running with TestNG in parallel=methods mode I am getting ConcurrentModificationException on ScenarioExecutor#stages field.

I am trying to isolate the issue and have a clean test that reproduces.

Yet another related issue is that ExpectedScenarioState fields are injected with instances from other threads causing test methods to fail due to uninitialized instances of these fields.

Is it safe to assume the JGiven was designed with thread safety in mind?

[adrian-herscu]

Here is the isolated issue:

import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;

import org.hamcrest.*;
import org.testng.annotations.*;

import com.tngtech.jgiven.*;
import com.tngtech.jgiven.annotation.*;
import com.tngtech.jgiven.testng.*;

import lombok.*;
import lombok.extern.slf4j.*;

/**
 * Opened https://github.com/TNG/JGiven/issues/491
 */
@Slf4j
public class ParallelMethodsTest extends
    ScenarioTest<ParallelMethodsTest.Fixtures, ParallelMethodsTest.Actions, ParallelMethodsTest.Verifications> {
    static class Actions extends Stage<Actions> {
        @ExpectedScenarioState
        private ThreadLocal<String> state;

        Actions setting_state(final String string) {
            log.debug("setting {}", string);
            state.set(string);
            return self();
        }
    }

    static class ActionsEx extends Stage<ActionsEx> {
        @ExpectedScenarioState
        private ThreadLocal<String> state1;

        ActionsEx setting_state(final String string) {
            log.debug("setting {}", string);
            state1.set(string);
            return self();
        }
    }

    static class FixturesEx extends Stage<FixturesEx> {
        @ProvidedScenarioState
        private final ThreadLocal<String> state1 = new ThreadLocal<>();

        FixturesEx nothing() {
            return self();
        }
    }

    static class VerificationsEx extends Stage<VerificationsEx> {
        @ExpectedScenarioState
        private ThreadLocal<String> state1;

        VerificationsEx the_state(final Matcher<String> stateMatcher) {
            log.debug("asserting on {}", state1.get());
            assertThat(state1.get(), stateMatcher);
            return self();
        }
    }

    static class Fixtures extends Stage<Fixtures> {
        @ProvidedScenarioState
        private final ThreadLocal<String> state = new ThreadLocal<>();

        Fixtures nothing() {
            return self();
        }
    }

    static class Verifications extends Stage<Verifications> {
        @ExpectedScenarioState
        private ThreadLocal<String> state;

        Verifications the_state(final Matcher<String> stateMatcher) {
            log.debug("asserting on {}", state.get());
            assertThat(state.get(), stateMatcher);
            return self();
        }
    }

    public static final int   REPETITIONS = 100;

    @ScenarioStage
    protected FixturesEx      additionalFixtures;

    @ScenarioStage
    protected ActionsEx       additionalActions;

    @ScenarioStage
    protected VerificationsEx additionalVerifications;

    @Test
    public void test1() {
        for (var i = 0; i < REPETITIONS; i++) {
            given().nothing();
            additionalFixtures
                .given().nothing();
            when().setting_state("kuku" + i);
            additionalActions
                .when().setting_state("muku" + i);
            then().the_state(is("kuku" + i));
            additionalVerifications
                .then().the_state(is("muku" + i));
        }
    }

    @Test
    public void test2() {
        for (var i = 0; i < REPETITIONS; i++) {
            given().nothing();
            additionalFixtures
                .given().nothing();
            when().setting_state("kuku" + i);
            additionalActions
                .when().setting_state("muku" + i);
            then().the_state(is("kuku" + i));
            additionalVerifications
                .then().the_state(is("muku" + i));
        }
    }
}

and the testng.xml:

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
<suite name="JGiven Commons Tests Suite" verbose="1">
  <test name="defects" parallel="methods" thread-count="10">
    <classes>
      <class name="ParallelMethodsTest" />
    </classes>
  </test>
</suite>

If changing the above to parallel="none" then it works. Also if decreasing REPETITIONS then it is more likely to succeed.

I am getting mostly NullPointerExeception, but saw also EmptyStackException, and ConcurrentModificationException .

...and another weird issue is that the fields in additional stages collide with these in the direct stages unless I changed their names, e.g. from state to state1.

[l-1squared]

Hi, @adrian-herscu
sorry for the rather late reply, I have been on vacation.

Unfortunately, you question about thread-safe can at the moment only be answered by @janschaefer, as he was solely responsible for the design.

In any case, I want to thank you for isolating the issue and providing a test against which to test. However I am right now lacking the time to look into the issue immedeately, so I have to ask you to be patient.

Best,
Kristof

[l-1squared]

I want to tackle the Issue alongside #823, so I'm closing this PR. I will take care of it though in #829.

Regarding the weird behavior of the scenario states when you call all of them "state": If you do that, then the fields have the same name and type. We have no chance to distingish fields there, because in that precise case we want to consider them as related. Spring does this in much the same way.