testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.98k stars 1.02k forks source link

Parallel=instances run tests of same instance on different threads #751

Open nmanandhar opened 9 years ago

nmanandhar commented 9 years ago

The documentation says that • parallel="instances": TestNG will run all the methods in the same instance in the same thread, but two methods on two different instances will be running in different threads.

However test methods for the same instances are running on different threads. Please see https://github.com/nmanandhar/TestNGParallelBehaviorTest.git

public class TestClassA {
    private static AtomicInteger instanceCount = new AtomicInteger(0);

    public TestClassA() {
        instanceCount.incrementAndGet();
        log("A new instance of TestClassA created");
    }

    @AfterSuite
    public void printNumberOfInstances() {
        log("Number of instances of TestClassA = " + instanceCount.get());
    }

    @Test
    public void testA1() throws Exception {
        log("testA1 of TestInstance " + this);
    }

    @Test(dependsOnMethods = "testA1")
    public void testB1() {
        log("testB1 of TestInstance " + this);
    }

    @Test(dataProvider = "integerProvider")
    public void testDataProvider(int number) throws Exception {
        log("testDataProvider : number = " + number + " testInstance = " + this);
    }

    @DataProvider
    public Object[][] integerProvider() {
        return new Object[][]{
                {1},
                {2}
        };
    }

    private void log(String message) {
        String threadName = Thread.currentThread().getName();
        synchronized (System.out) {
            System.out.println(String.format("Thread %s : %s ", threadName, message));
        }
    }
}

Suite File

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="InstanceTestSuite" parallel="instances" thread-count="5">
    <test name="InstanceTest">
        <classes>
            <class name="com.nm.TestClassA"/>
        </classes>
    </test>
</suite>

Output of mvn test

Running TestSuite
Thread main : A new instance of TestClassA created
Thread pool-1-thread-1 : testA1 of TestInstance com.nm.TestClassA@58ee2549
Thread pool-1-thread-1 : testDataProvider : number = 1 testInstance = com.nm.TestClassA@58ee2549
Thread pool-1-thread-1 : testDataProvider : number = 2 testInstance = com.nm.TestClassA@58ee2549
Thread pool-1-thread-2 : testB1 of TestInstance com.nm.TestClassA@58ee2549
Thread main : Number of instance = 1
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.835 sec - in TestSuite
nmanandhar commented 9 years ago

Motivation For Need for Running Tests in the Same Thread

TestA started, browser opened and two test ran on this thread
|-------------------|
Thread 1

                           ..A long time later

                                                            |--------------------|
                                                            Thread 2
                                                            Continuing with the remaining tests on TestA instance

Because of this a browser could open run some test , then remain idle for a long time, then resume test after an hour later when some other thread is ready to pick up the remaining tests. Thus even with a thread count of 5, 10 or 20 instance of test were running concurrently, and some browsers remained idle for a long time. With the browser remaining idle for some time ,the selenium hub expires session on the particular browser instance failing the test. Changing the session timeout fixes the issue (but is unsatisfactory) however we can never be sure how many browser instance will be opened. Also with tests being run in multiple threads, we cannot be sure of threading issues that will arise.

veerabhavya commented 5 years ago

Is this happening only when dependsOn methods is used?

krmahadevan commented 5 years ago

The github link shared to recreate this issue, is no longer available. Need a sample that can be used to recreate this issue.

@veerabhavya - Not sure what your question is. If you are experiencing the same issue using 7.0.0 please help provide a sample that can be used to recreate the issue.

veerabhavya commented 5 years ago
public class ToTest {
    String my_name;
    ToTest(String name) {
        my_name = name;
    }

    List<String> str;

    @Test(dataProvider = "dp1", dependsOnMethods = "func2")
    public void func(String b) throws Exception {
        System.out.println("my Test " + Thread.currentThread().getName() + " " + my_name);
        //Thread.sleep(10000);

    }
    @DataProvider(name = "dp1")
    Iterator<Object> sending() throws Exception {
        Iterator it;
        System.out.println("data provider" + Thread.currentThread().getName() + " " + my_name);
        str = new ArrayList<>();
        str.add(null);
        str.add("two");
        //Thread.sleep(5000);
        it = str.iterator();
        return it;
    }
    @Test(dataProvider = "dp2")
    public void func2(String b) throws Exception {
        System.out.println("my Test " + Thread.currentThread().getName() + " " + my_name);
        //Thread.sleep(10000);

    }
    @DataProvider(name = "dp2")
    Iterator<Object> sending2() throws Exception {
        Iterator it;
        System.out.println("data provider" + Thread.currentThread().getName() + " " + my_name);
        str = new ArrayList<>();
        str.add("one");
        str.add("two");
        //Thread.sleep(5000);
        it = str.iterator();
        return it;
    }
}
veerabhavya commented 5 years ago
public class TestFactory3 {
    @Factory
    public Object[] createInstance() throws Exception {
        Object[] result = new Object[4];

        for (int i = 0; i<4; i++)
            result[i] = new ToTest(i + "");

        return result;
    }
}
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
<suite name="MySuite1" verbose="10"  parallel="false" >
  <test name="Mypackage1" parallel="instances" thread-count="4" group-by-instances="true">
    <classes >
      <class name="com.example.dmo4.TestFactory3" />
    </classes>
  </test>
</suite>
veerabhavya commented 5 years ago
Screenshot 2019-09-16 at 11 32 31 AM
veerabhavya commented 5 years ago
Screenshot 2019-09-16 at 11 33 49 AM

Instance 3 is on thread Mypackage1-1 and also on thread Mypackage1-4

krmahadevan commented 5 years ago

@veerabhavya - What version of TestNG are you working with ? The latest released version is 7.0.0

veerabhavya commented 5 years ago

yes 7.0.0 only

mihalyr commented 3 years ago

Hi, this seems to be an older issue, but I just ran into this with version 7.3.0, using -parallel instances with dataprovider and factory constructor my tests are failing because TestNG runs them in different threads. My expectation based on the docs was that all methods in the same instance will be run on the same thread. Is there a way to achieve this with TestNG? Any workarounds?

The code is similar to the examples already provided in this issues and the logging result below shows that the methods of the same instance are being executed on different threads:

2020-09-27 15:19:35.210 [INFO] (TestNG-test=test-3) : ParallelTest - Running #1 testA[instance:v4] on Thread[TestNG-test=test-3,5,main]
2020-09-27 15:19:35.210 [INFO] (TestNG-test=test-1) : ParallelTest - Running #2 testB[instance:v4] on Thread[TestNG-test=test-1,5,main]
2020-09-27 15:19:37.751 [INFO] (TestNG-test=test-5) : ParallelTest - Running #3 testC[instance:v4] on Thread[TestNG-test=test-5,5,main]
2020-09-27 15:19:38.469 [INFO] (TestNG-test=test-4) : ParallelTest - Running #4 testD[instance:v4] on Thread[TestNG-test=test-4,5,main]
krmahadevan commented 3 years ago

@mihalyr - The example shared in https://github.com/cbeust/testng/issues/751#issuecomment-531649370 involves a method dependency. When there is a method dependency, then TestNG will run the independent methods first and then the dependent methods and when dependent methods are executed, there's no guarantee that they will run on the same thread. So can you please share what does your sample look like ?

Here's a modified sample that involves no method dependencies. (along with output using 7.3.0)

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import org.testng.Reporter;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

public class ToTest {

  String my_name;

  ToTest(String name) {
    my_name = name;
  }

  @Test(dataProvider = "dp1")
  public void func(String b) {
    String msg = "Running " + Reporter.getCurrentTestResult().getMethod().getQualifiedName() +
        "#" + b +
        " on Thread [" + Thread.currentThread().getId() + "] for instance [" + my_name + "]";
    System.err.println(msg);
  }

  @DataProvider(name = "dp1")
  Iterator<Object> sending() {
    List<Object> str = new ArrayList<>();
    str.add("sending_1");
    str.add("sending_2");
    return str.iterator();
  }

  @Test(dataProvider = "dp2")
  public void func2(String b) {
    String msg = "Running " + Reporter.getCurrentTestResult().getMethod().getQualifiedName() +
        "#" + b +
        " on Thread [" + Thread.currentThread().getId() + "] for instance [" + my_name + "]";
    System.err.println(msg);
  }

  @DataProvider(name = "dp2")
  Iterator<Object> sending2() {
    List<Object> str = new ArrayList<>();
    str.add("func_1");
    str.add("func_2");
    return str.iterator();
  }
}
import org.testng.annotations.Factory;

public class TestFactory3 {

  @Factory
  public Object[] createInstance() {
    Object[] result = new Object[4];
    for (int i = 0; i < 4; i++) {
      result[i] = new ToTest(i + "");
    }
    return result;
  }
}

Suite xml

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="751_suite" parallel="instances" group-by-instances="true">
  <test name="751_test">
    <classes>
      <class name="com.rationaleemotions.github.issue751.TestFactory3"/>
    </classes>
  </test>
</suite>

Here's the output

Running com.rationaleemotions.github.issue751.ToTest.func#sending_1 on Thread [18] for instance [0]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_1 on Thread [17] for instance [1]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_1 on Thread [16] for instance [2]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_1 on Thread [15] for instance [3]

Running com.rationaleemotions.github.issue751.ToTest.func#sending_2 on Thread [18] for instance [0]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_2 on Thread [17] for instance [1]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_2 on Thread [16] for instance [2]
Running com.rationaleemotions.github.issue751.ToTest.func#sending_2 on Thread [15] for instance [3]

Running com.rationaleemotions.github.issue751.ToTest.func2#func_1 on Thread [18] for instance [0]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_1 on Thread [17] for instance [1]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_1 on Thread [16] for instance [2]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_1 on Thread [15] for instance [3]

Running com.rationaleemotions.github.issue751.ToTest.func2#func_2 on Thread [18] for instance [0]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_2 on Thread [17] for instance [1]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_2 on Thread [16] for instance [2]
Running com.rationaleemotions.github.issue751.ToTest.func2#func_2 on Thread [15] for instance [3]

===============================================
751_suite
Total tests run: 16, Passes: 16, Failures: 0, Skips: 0
===============================================

Process finished with exit code 0
krmahadevan commented 3 years ago

@mihalyr - For scenarios that involve method dependencies and when you are combining factories, you can perhaps try using the JVM argument -Dtestng.thread.affinity=true. There was one issue with this, which now stands fixed as part of fixing https://github.com/cbeust/testng/pull/2368

mihalyr commented 3 years ago

@krmahadevan You are right, I use dependencies, that was one of the reasons I went with TestNG instead of JUnit. I don't think I have seen it mentioned in docs that parallel=instances with dependent methods will not work as expected. The affinity setting improves it, but does not guarantee either and as seen below it does not solve this.

I guess I am left with writing thread safe code then :) Do you think it would make sense to add some warning about this behaviour to the documentation? Because currently it says parallel="instances": TestNG will run all the methods in the same instance in the same thread, ... which is not true in the case below, quite the opposite some some tests e.g. if testB and testC both depend on testA they might be executed in parallel at the same time on the same instance.

Reproducer (ran with -parallel instances ):

import org.testng.annotations.DataProvider;
import org.testng.annotations.Factory;
import org.testng.annotations.Test;

import java.lang.reflect.Method;
import java.util.concurrent.atomic.AtomicInteger;

@Test(suiteName = "testParallel")
public class TestNGParallelDependent {
    private final String prefix;
    private final AtomicInteger counter = new AtomicInteger();

    @Factory(dataProvider = "prefixes")
    TestNGParallelDependent(String prefix) {
        this.prefix = prefix;
    }

    @DataProvider
    static Object[][] prefixes() {
        return new Object[][]{
                new Object[]{"v1"},
                new Object[]{"v2"}
        };
    }

    @Test
    void testA(Method method) {
        log(method.getName());
        work();
    }

    @Test(dependsOnMethods = "testA")
    void testB(Method method) {
        log(method.getName());
        work();
    }

    @Test(dependsOnMethods = "testB")
    void testC(Method method) {
        log(method.getName());
        work();
    }

    private void log(String testName) {
        System.out.println(prefix
                + " " + counter.incrementAndGet()
                + " " + testName
                + " " + Thread.currentThread());
    }

    private static void work() {
        try {
            Thread.sleep(500);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }
    }
}

Output:

v1 1 testA Thread[TestNG-test=prospect-1,5,main]
v2 1 testA Thread[TestNG-test=prospect-2,5,main]
v2 2 testB Thread[TestNG-test=prospect-4,5,main]
v1 2 testB Thread[TestNG-test=prospect-3,5,main]
v2 3 testC Thread[TestNG-test=prospect-3,5,main]
v1 3 testC Thread[TestNG-test=prospect-5,5,main]

Output with -Dtestng.thread.affinity=true

v1 1 testA Thread[TestNG-test=prospect-1,5,main]
v2 1 testA Thread[TestNG-test=prospect-2,5,main]
v2 2 testB Thread[TestNG-test=prospect-4,5,main]
v1 2 testB Thread[TestNG-test=prospect-2,5,main]
v1 3 testC Thread[TestNG-test=prospect-2,5,main]
v2 3 testC Thread[TestNG-test=prospect-3,5,main]
mihalyr commented 3 years ago

@krmahadevan Hm, it seems this might be even worse and even dependent methods might not always execute in the correct order with parallel=instances, below is a reproducer, which quite often fails (not 100% reliable as sometimes does succeed too). Not sure why, but I could not reproduce it without the added delay. Also the number of test methods in the class had a relation to failures too.

Is this a known issue? It seems I cannot use parallel=instances with dependent methods even if my code is thread safe if there are dependencies between test methods.

import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Factory;
import org.testng.annotations.Test;

import java.util.concurrent.atomic.AtomicInteger;

import static org.testng.Assert.assertEquals;

@Test(suiteName = "testParallel")
public class TestNGParallelDependent {
    private final String prefix;
    private final int delay;
    private final AtomicInteger counter = new AtomicInteger();

    private volatile String value;

    @Factory(dataProvider = "dp")
    TestNGParallelDependent(String prefix, int delay) {
        this.prefix = prefix;
        this.delay = delay;
    }

    @DataProvider
    static Object[][] dp() {
        int instances = 3;
        int delay = 1;
        Object[][] params = new Object[instances][2];
        for (int i = 0; i < instances; i++) {
            params[i][0] = "v" + (i + 1);
            params[i][1] = delay * 1000;
            delay <<= 1;
        }
        return params;
    }

    @BeforeClass
    void setup() {
        work(delay);
    }

    @Test
    void testA() {
        assertEquals(1, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testA")
    void testB() {
        assertEquals(2, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testB")
    void testC() {
        assertEquals(3, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testC")
    void testD() {
        assertEquals(4, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testD")
    void testE() {
        assertEquals(5, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testE")
    void testF() {
        assertEquals(6, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testF")
    void testG() {
        assertEquals(7, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testG")
    void testH() {
        assertEquals(8, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testH")
    void testI() {
        assertEquals(9, counter.incrementAndGet());
    }

    @Test(dependsOnMethods = "testI")
    void testJ() {
        assertEquals(10, counter.incrementAndGet());
    }

    private static void work(int delay) {
        try {
            Thread.sleep(delay);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }
    }
}
krmahadevan commented 3 years ago

@mihalyr - I dont remember TestNG ever mentioning anything about thread affinity for depends on methods or depends on groups as a functionality. I believe that is the TestNG internals.

I believe TestNG only says that if you state that you need tests to be run in parallel based on instances (in the case of a factory) then TestNG lets you do that sort of parallel execution as well.

I dont recollect anywhere TestNG stating that it would guarantee thread affinity for sure.

Running independent methods first and then dependent methods later is how TestNG achieves/supports depends on methods and depends on groups as a functionality.

But should it be run on the same thread or a different thread is I believe an implementation detail that needn't be documented.

So I would say that you dont mix the thread affinity with parallel = instances when there are dependencies involved.

Sometime back I introduced a new feature (its more of experimental in nature) which lets you restrict TestNG to honouring thread affinity when there are dependencies involved.

This works only for multi level dependencies (i.e., one upstream method has only 1 downstream method as a dependency and there are no two methods that have the same method as an upstream dependency).

This feature didnt work when it was coupled with instance based parallelism. That was addressed as part of https://github.com/cbeust/testng/pull/2368

So this should work more as expected in the next upcoming release.

mihalyr commented 3 years ago

@krmahadevan I am sorry, but what you just wrote doesn't make any sense. Why are you denying something that is right in the documentation?

I dont remember TestNG ever mentioning anything about thread affinity I believe TestNG only says that if you state that you need tests to be run in parallel based on instances (in the case of a factory) then TestNG lets you do that sort of parallel execution as well. I dont recollect anywhere TestNG stating that it would guarantee thread affinity for sure.

You don't need to remember, believe or recollect anything, open the documentation and see it for yourself: https://testng.org/doc/documentation-main.html#parallel-tests

parallel="instances": TestNG will run all the methods in the same instance in the same thread, but two methods on two different instances will be running in different threads.

It says explicitly that methods from the same instance will be running ON THE SAME THREAD and does not mention any exception for dependencies, right? If you "believe" this is not right, please fix the documentation so it matches whatever TestNG is doing, because I am wasting my time figuring out these things for myself and wondering why does TestNG not do what it says in the documentation.


Regarding the issue with dependencies not running in the correct order that I can reproduce as mentioned here https://github.com/cbeust/testng/issues/751#issuecomment-699694723 I am opening a new issue as this issue is about threading.

krmahadevan commented 3 years ago

@mihalyr

Why are you denying something that is right in the documentation?

All I was saying was that I don't recollect seeing that in the documentation. I am pretty sure its ok not to remember something right ?

You don't need to remember, believe or recollect anything, open the documentation and see it for yourself:

Thanks for pointing me to the place where it says this and but explicitly doesn't call out the behaviour when you couple "instances" with "dependencies".

In the same place if you see there's this also that is mentioned

parallel="methods": TestNG will run all your test methods in separate threads. Dependent methods will also run in separate threads but they will respect the order that you specified.

So this combined with a parallel strategy of "instances" pretty much convey this information no? Yes it doesn't straight forwardly tell this to the user. That can and will be corrected no issues on that.

mihalyr commented 3 years ago

Thanks, correcting the docs to call this out will clarify things and help others not to run into the same issue and spend a lot of time investigating what is going on.

So this combined with a parallel strategy of "instances" pretty much convey this information no?

No. That is for a different setting which has completely different behaviour and does not relate to instances, because with parallel methods one expects that methods (including dependent) will run in different threads, but for instances this is exactly what is not expected.

mihalyr commented 3 years ago

By the way, is correcting the docs the final solution here or are there any plans on actually fixing this and make dependent methods in the same instance run on the same thread when using parallel instances?

krmahadevan commented 3 years ago

@mihalyr

No. That is for a different setting which has completely different behaviour and does not relate to instances, because with parallel methods one expects that methods (including dependent) will run in different threads, but for instances this is exactly what is not expected.

Not true. A parallel strategy of methods means that you let TestNG create the instances and a parallel strategy of instances means TestNG falls back to you to create the instances. Dependency amongst methods is not an attribute of the instance to which it belongs to. So they are pretty much the same thing IMO.

By the way, is correcting the docs the final solution here or are there any plans on actually fixing this and make dependent methods in the same instance run on the same thread when using parallel instances?

Fixing the document will be the fix in this case. If its a test that involves factories and then there are dependencies (no multiple dependencies on upstream method) then the thread affinity feature I called out earlier will work once 7.4.0 is released (it had a bug that got fixed recently)

mihalyr commented 3 years ago

Not true. A parallel strategy of methods means that you let TestNG create the instances and a parallel strategy of instances means TestNG falls back to you to create the instances. Dependency amongst methods is not an attribute of the instance to which it belongs to. So they are pretty much the same thing IMO.

No, this does not make any sense with regards to what we discuss in this issue. Please, refer to the documentation, it's written there clearly:

parallel="methods": TestNG will run all your test methods in separate threads. Dependent methods will also run in separate threads but they will respect the order that you specified. parallel="instances": TestNG will run all the methods in the same instance in the same thread, but two methods on two different instances will be running in different threads.

You can see clearly, parallel methods is about running test methods in separate threads, including dependent methods, it's right there in the docs. While parallel instances is about running instances (NOT METHODS) in parallel, and says that test methods in the same instance run in the same thread. Dependent methods are still part of the test instances, since they are part of the same class, telling that they are not is super confusing and makes it really hard to rely on the test framework.

Fixing the document will be the fix in this case

That is sad to hear, but at least it will be clearly defined and users will know what to expect and who need to run dependent methods on the same thread while running instances in parallel, will know they need to look elsewhere.

krmahadevan commented 3 years ago

@mihalyr - We are going around in circles. I am telling you from what i know of in the codebase.

While parallel instances is about running instances (NOT METHODS) in parallel, and says that test methods in the same instance run in the same thread. Dependent methods are still part of the test instances, since they are part of the same class,

You are confusing instances with dependencies. These are two separate attributes that determine how TestNG runs your tests.

Let me try to summarise what these two are:

These two are two different things. TestNG will always run the independent methods first and then the dependent methods. And this may or may not happen on the same thread. There is no guarantee that TestNG provides on this because the internal implementation is like that.

If a documentation clean up is going to make it clear, it will be done. But beyond that there's really not much that I can offer as explanation (I have tried my level best to explain to you what is the current behaviour as of today). Its upto you to take the explanation or wait for @juherr / @cbeust to see if they can offer a better explanation.

mihalyr commented 3 years ago

I am telling you from what i know of in the codebase.

That's the problem, it seems the codebase does not reflect the expected behaviour declared in the documentation (which is a definition of a bug). I am not really interested in why it is not working in the way it should, I believe you, that there might be lot of reasons for it due to various implementation details. I am just saying that the code does not do what it is saying in the docs, but you keep pushing back and keep explaining to me why it is implemented in a different way, which is not the point here.

Anyway, I got my answer, I cannot run multiple instances in parallel and keep methods of the same instance on the same thread as it is in the docs, that's fine, I will look into implementing it in a different way or framework. What you do with this issue is completely up to you, implement what is in the docs, or change the docs, or not do anything, it's your choice, I just wanted some clarification to not waste more time on this.

krmahadevan commented 3 years ago

@mihalyr

I am just saying that the code does not do what it is saying in the docs, but you keep pushing back and keep explaining to me why it is implemented in a different way, which is not the point here.

I thought we already agreed that the documentation needs to be updated. And while that happens, I was trying to explain to you what is the behavior as of today. I have no interest in pushing you back or anyone else for any reasons. That's beyond the point or motivation behind me (or) anyone else for that matter who wants to contribute to an open source library! If that's not clear, then I thought I should make it explicit.

What you do with this issue is completely up to you, implement what is in the docs, or change the docs, or not do anything, it's your choice, I just wanted some clarification to not waste more time on this.

Sure. Whatever is the correct way that adheres to what the current behavior is, will be documented in the documentation. Now that the behavior has been made explicitly clear to you, I would leave it to you to decide the best way to take this forward!

juherr commented 3 years ago

@mihalyr @krmahadevan Both of you are right: the feature is working as expected except when a dependency feature is involved. It is a known issue and it should be fixed. But the current implementation won't allow it easily and nobody has currently enough free time to dig and fix it. I suppose the documentation should be improved with a link to this issue to warn about the issue.

mihalyr commented 3 years ago

Hi @juherr I ran a few tests here with this config, wanted to try whether @Test(singleThreaded=true) makes a difference, but not, and also created classes instead of factory instances and tested parallel=classes which seems to have the same issue, methods of the same class are not run in the same thread either.

juherr commented 3 years ago

@mihalyr As @krmahadevan explained, the current implementation of each feature is working but there is an issue when parallelization and dependencies are used together. If you want to have a quick look at the issue, you can check https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/internal/thread/graph/GraphThreadPoolExecutor.java and https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/internal/Graph.java You'll see that in case of parallelization, each method is given one by one and can't be executed in the same thread then. We can imagine another implementation where threads are living until the end of the execution and methods are passed via a kind of pipeline. Feel free to try if you have some free time, it will help us :)

mihalyr commented 3 years ago

Thanks, was not sure if it affected both parallel instances and classes. Thanks for the links to the code, I'll try to find some time to look into.