Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DependsOnMethods made parallel class use several threads #1185

Closed
thibaut-sticky opened this issue Oct 6, 2016 · 38 comments
Closed

DependsOnMethods made parallel class use several threads #1185

thibaut-sticky opened this issue Oct 6, 2016 · 38 comments

Comments

@thibaut-sticky
Copy link

thibaut-sticky commented Oct 6, 2016

TestNG Version

6.9.12 and 6.9.13.8

Expected behavior

When setting TestNG to run tests in parallel by class, all the tests in a single class should be run in the same thread.

Actual behavior

If one of the test method depends on another, a new thread is created, which is really annoying for Selenium case for instance (BeforeClass open the browser, AfterClass kill it)

Test case sample

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
<suite name="main" parallel="classes" thread-count="2" verbose="0" group-by-instances="false">
    <test name="all">
        <classes>
            <class name="TestFoo"/>
        </classes>
    </test>
</suite>
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

public class TestFoo {


  @BeforeClass
  public void navigateTo() {
    System.out.print("BeforeClass - ");
    printThread();
  }

  private void printThread() {
    System.out.println(Thread.currentThread().getName());
  }

  @AfterClass
  public void die() {
    System.out.print("AfterClass - ");
    printThread();
  }

  @Test
  public void t1() {
    System.out.print("T1 - ");
    printThread();
  }

  @Test(dependsOnMethods = "t1")
  public void t2() {
    System.out.print("T2 - ");
    printThread();
  }

}
BeforeClass - pool-1-thread-1
T1 - pool-1-thread-1
T2 - pool-1-thread-2
AfterClass - pool-1-thread-2

What I've tried:

  • singleThreaded = true
  • group-by-instance="true"
@juherr
Copy link
Member

juherr commented Oct 6, 2016

Is it working as expected without the dependsOnMethods?

@ryanlevell
Copy link

@juherr yes, only 1 thread was used when I removed dependsOnMethods from the example above:

BeforeClass - pool-1-thread-1
T1 - pool-1-thread-1
T2 - pool-1-thread-1
AfterClass - pool-1-thread-1

@thibaut-sticky
Copy link
Author

It's working fine also if I set thread-count to 1.

@juherr
Copy link
Member

juherr commented Oct 11, 2016

Yes, the problem is located in GraphThreadPoolExecutor/DynamicGraph. But I don't know how to fix it for the moment.
Don't expect a quick fix, sorry.

@thibaut-sticky
Copy link
Author

@juherr At least it's already good to know where the issue come from. Except remove the dependsOnMethods and refactor a bit my class, do you see a workaround?

@juherr
Copy link
Member

juherr commented Oct 11, 2016

In fact, dependsOn with parallel class can not work together. If it worked before, it was a side effect of another issue :/

@thibaut-sticky
Copy link
Author

I don't understand why you said dependsOn with parallel class cannot work together. Indeed, in this paralell mode, all the test methods inside a class has to be run in the same thread, so it does not affect the dependsOn, this is the theory.
Perhaps, you said it cannot work because the code does not allow it?

@juherr
Copy link
Member

juherr commented Oct 12, 2016

Perhaps, you said it cannot work because the code does not allow it?

In the current state, yes. But it is supposed to work ;)

@mathias21
Copy link

Hi @juherr

Any update on this? I'm facing the same problem due to parallel execution with dependsOnMethod. I had a look to graph/GraphThreadPoolExecutor.java but it looks like my basic knowledge won't be enough here.

Thank you!

@juherr
Copy link
Member

juherr commented Jan 10, 2017

@mathias21 Nope, nothing new for the moment. Sorry.

@mathias21
Copy link

mathias21 commented Jan 10, 2017

Hi @thibaut-sticky

I think I've found a workaround to solve this problem. It takes 2 dependencies: with the max amount of devices you are going to use at the same time, and also forcing you to release the session during @afterclass method, but apparently it works. If you specify in your suite XML file "thread-count='2'", then you can define your suite as following:

<suite name="SmokeTest" configfailurepolicy="continue" parallel="classes" thread-count="2">
<test name="Test cases" >
        <classes>
            <class name="packageA.TestClass1"/>
            <class name="packageA.TestClass2"/>
        </classes>
    </test>

    <test name="Test cases2" >
        <classes>
            <class name="packageB.TestClass1"/>
            <class name="packageB.TestClass2"/>
        </classes>
    </test>
</suite>

It is not the best solution, but apparently runs your tests with proper sorting.

@thibaut-sticky
Copy link
Author

@mathias21 Thx you for your workaround, but sadly in my case I cannot apply it. But at least I can see I'm not the only one using parallel and dependsOnMethod features 👍

@toutoudnf
Copy link

i have the same problem with parallel=classes & using dependsOnMethod in one classe, and i do some prepare work using localThread var :(. it takes me a long time to find out the thread change. hope this bug could be fixed soon :)

@dr29bart
Copy link
Contributor

The similar problem happens when using priority. Described in #1066

@juherr juherr self-assigned this Jan 22, 2017
@sylvainbouxin
Copy link

@juherr
I am quite impacted by this issue as well, and saw you self-assigned the ticket.
Does that mean you're actively working on a resolution? What kind of priority is this for you?
This rather high for us...
Thanks

@mathias21
Copy link

Hi all

Do we know if this is not happening in a concrete version? I'm also using 6.9.12. I believe this is a structural issue, not a punctual bug or typo, am I right?

@juherr
Copy link
Member

juherr commented Jan 23, 2017

@sylvainbouxin "actively" is not the good word but it is on the top of the list and I planned to work on it unless someone has enough motivation.
And I'm open to any help! 😉

@mathias21 Exactly! There are some similar issues and the root cause is often DynamicGraph which manages the order of methods and the group of methods when parallel is used. So, just one of the most important part of TestNG which generate test regression each time I have to modify it! 😨

@sylvainbouxin
Copy link

@juherr nice challenge, indeed...
I'm currently looking into this: I really need this fixed or write my own framework to manage the order of methods and classes with parallel execution.

@juherr
Copy link
Member

juherr commented Jan 23, 2017

@sylvainbouxin Nice! Feel free to send me emails if you need to exchange some extra things.

@cbeust
Copy link
Collaborator

cbeust commented Feb 14, 2017

@juherr, you said:

Yes, the problem is located in GraphThreadPoolExecutor/DynamicGraph.

Can you elaborate on what the problem is exactly?

@juherr
Copy link
Member

juherr commented Feb 15, 2017

@cbeust due to the edge between methods, dependsOn* methods are no more together in the free nodes list. Now, they are free one by one.

@vinothkumartc
Copy link

@juherr
Hello Any update on when the issue will get fixed.

@ghost
Copy link

ghost commented Oct 25, 2017

Can you please prioritize this issue-fix @juherr

@krmahadevan
Copy link
Member

krmahadevan commented Oct 26, 2017

@nevilp88 - Its not that this issue is not a priority. Its just that, we haven't gotten around to fixing this, mainly due to lack of sufficient time. Trust me, this is one of the biggest issues that still is in our plate for fixing. But since this is also a problem in the core of TestNG, its a bit tricky to change behavior and ensure that we don't break anything.
If you would like to take a go at it, and help provide a PR, we would be more than happy to help review it and get it merged.

There's just a handful bunch of us (to be honest, currently just me and @juherr) who are juggling with all issues and trying our best to get them fixed. Thanks for your understanding

@piotrbo
Copy link

piotrbo commented Nov 23, 2017

Hi, I'm just one more guy facing this issue

damencho added a commit to jitsi/jitsi-meet-torture that referenced this issue Jan 4, 2018
Workarounds a problem where methods and after class run in different threads, which results filling the whole grid at some point.
testng-team/testng#1185
Also changes the test report to be prettier.
@apanashchenko
Copy link

Hi! Any updates?

@ryudice
Copy link

ryudice commented Apr 26, 2018

Any plans to fix this?

krmahadevan added a commit to krmahadevan/testng that referenced this issue May 14, 2018
Closes testng-team#89, testng-team#1050, testng-team#1066, testng-team#1173, testng-team#1185

This PR aims at assuring that methods that fall
under the below two use cases, all run on the 
same thread when classes are being run in parallel:

* @test methods in a class are ordered by priority
* @test methods have a single dependency on 
another method using “dependsOnMethods” attribute.

The thread affinity feature is supposed to be 
“experimental” and it can be turned on via the
JVM argument : -Dtestng.thread.affinity=true

This feature is turned off by default just to 
ensure that we don’t have any users experiencing
un-usual behavior.
krmahadevan added a commit to krmahadevan/testng that referenced this issue May 14, 2018
Closes testng-team#89, testng-team#1050, testng-team#1066, testng-team#1173, testng-team#1185

This PR aims at assuring that methods that fall
under the below two use cases, all run on the 
same thread when classes are being run in parallel:

* @test methods in a class are ordered by priority
* @test methods have a single dependency on 
another method using “dependsOnMethods” attribute.

The thread affinity feature is supposed to be 
“experimental” and it can be turned on via the
JVM argument : -Dtestng.thread.affinity=true

This feature is turned off by default just to 
ensure that we don’t have any users experiencing
un-usual behavior.
krmahadevan added a commit to krmahadevan/testng that referenced this issue May 15, 2018
Closes testng-team#89, testng-team#1050, testng-team#1066, testng-team#1173, testng-team#1185

This PR aims at assuring that methods that fall
under the below two use cases, all run on the 
same thread when classes are being run in parallel:

* @test methods in a class are ordered by priority
* @test methods have a single dependency on 
another method using “dependsOnMethods” attribute.

The thread affinity feature is supposed to be 
“experimental” and it can be turned on via the
JVM argument : -Dtestng.thread.affinity=true

This feature is turned off by default just to 
ensure that we don’t have any users experiencing
un-usual behavior.
@krmahadevan
Copy link
Member

Fixed by #1783

@thibaut-sticky
Copy link
Author

Thx for the fix. The current changelog of the future release is pretty big. Can we expect quickly a new version?

@juherr
Copy link
Member

juherr commented Jun 1, 2018

@thibaut-sticky I'm not sure we will release quickly because it will be a major release and we want to take enough time to change/break everything we want/need without waiting for the next major release.

dlatsuga added a commit to dlatsuga/project-olymp that referenced this issue Jul 14, 2018
@andrei-kotau-epam
Copy link

andrei-kotau-epam commented Nov 9, 2018

@krmahadevan If I am not mistaken issue was fixed. But I am still able to reproduce it in 7.0.0-beta1.

@ryudice
Copy link

ryudice commented Nov 9, 2018

I dont think this was ever fixed, however 7.0.0 reverts whichever fix you attempted to do.

We moved away from testng as our testsuite was growing and we couldnt afford not being able to use parallel exeuction.

I did test our old codebase that was still using testng with 7.0.0 and the issues keeps happening.

@krmahadevan
Copy link
Member

@andrei-kotau-epam @ryudice

When using TestNG 7.0.0-beta1 am assuming that you folks are enabling this feature by passing the JVM argument -Dtestng.thread.affinity=true. Only when this JVM argument is passed would thread affinity be guaranteed. Please retry and let me know how it goes.

If even after doing that this issue is still occurring, please help share a sample that can be used to reproduce this issue.

@andrei-kotau-epam
Copy link

@krmahadevan works for me with -Dtestng.thread.affinity=true (7.0.0-beta1) . Thank you!
cc: @ryudice

@mstancl
Copy link

mstancl commented Jul 11, 2019

I have come across an issue.
When i tried this (on both beta7 and beta1) with the VM argument -Dtestng.thread.affinity=true :

import org.testng.annotations.Test;

public class TestClass2 {
    @Test()
    public void test0(){
        System.out.println("TestClass2 - test0. Thread " + Thread.currentThread().getId());
    }

    @Test
    public void test1() {
        System.out.println("TestClass2 - test1. Thread " + Thread.currentThread().getId());
    }

    @Test(dependsOnMethods = "test1")
    public void test2() {
        System.out.println("TestClass2 - test2. Thread " + Thread.currentThread().getId());
    }

    @Test(dependsOnMethods = "test2")
    public void test3() {
        System.out.println("TestClass2 - test3. Thread " + Thread.currentThread().getId());
    }

    @Test()
    public void test4(){
        System.out.println("TestClass2 - test4. Thread " + Thread.currentThread().getId());
    }
}

and then tried to run through xml like this :

<suite name="Default Suite">
    <test  name="testNG tests" parallel="classes" thread-count="50" >
        <classes>
            <class name="ie.kbc.qa.seleniumT24.debugs.TestClass2"/>
        </classes>
    </test>
</suite>

I got an exception :

Exception in thread "TestNG-test=testNG tests-1" java.lang.NullPointerException
TestClass2 - test0. Thread 9
	at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
	at org.testng.internal.thread.graph.GraphThreadPoolExecutor.handleThreadAffinity(GraphThreadPoolExecutor.java:156)
	at org.testng.internal.thread.graph.GraphThreadPoolExecutor.afterExecute(GraphThreadPoolExecutor.java:102)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1157)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
TestClass2 - test1. Thread 9
TestClass2 - test4. Thread 9

Process finished with exit code 1

This exception is thrown when there is more then one test with no dependOnMethods

@krmahadevan
Copy link
Member

@mstancl - Thanks for sharing that sample. Since this is a problem that has arose out of the thread affinity feature that I introduced into TestNG, I have created a new bug for this #2110

Please track this issue with #2110

@FredVaugeois
Copy link

FredVaugeois commented Jun 4, 2020

Hi! I have another issue that is related to this topic. Maybe it's just a misuse of TestNG from my part, but I feel it could be a problem on TestNG's side.

I am using TestNG 7.1.0.

So basically the problem is that the thread affinity feature only works when you test a single class (from what I've tested). Here I have two classes with the same exact methods:

image

And here I have my xml file with a only one of them, which gives me the following result:

image

image

Which is the correct behaviour! Although, if I run my two classes in parallel, this is where it gets funky:

image

image

Again, it may be me not understanding how to write a proper xml file...

Also, here's what happens when I don't use -Dtestng.thread.affinity=true (just to prove that I am using it):
image

Thank you!

Edit - This syntax will work and have the two classes on a separated threads:
image

ALTHOUGH: The two classes will not run in parallel, even though they run on different threads.

@krmahadevan
Copy link
Member

@FredVaugeois - I would suggest that you please open up a new issue and include a sample that can be used to reproduce the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests