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

-Dtestng.thread.affinity=true do not work when running multiple instance of test in parallel #2321

Closed
2 of 7 tasks
ajmera25 opened this issue Jun 11, 2020 · 7 comments · Fixed by #2368
Closed
2 of 7 tasks
Milestone

Comments

@ajmera25
Copy link

ajmera25 commented Jun 11, 2020

TestNG Version

7.1.0

Expected behavior

two instance of test class should get different thread. All the test method within the test class should run in same thread.

Actual behavior

Test method of same instance are getting different thread.

Is the issue reproductible on runner?

  • Shell
  • Maven
  • Gradle
  • Ant
  • Eclipse
  • IntelliJ
  • NetBeans

Test case sample

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

public class MultipleInstance {
    private int part;

    private long threadId;

    @Factory(dataProvider = "dp")
    public MultipleInstance(int part) {
        this.part = part;
    }

    @Test
    public void independent() {
        threadId = Thread.currentThread().getId();
        System.err.println(getClass().getSimpleName() + " part - " + part + " independent() running on " + threadId);
    }

    @Test(dependsOnMethods = "independent")
    public void dependent() {
        long currentThreadId = Thread.currentThread().getId();
        System.err.println(getClass().getSimpleName() + " part - " + part + " dependent() running on " + currentThreadId);
        Assert.assertEquals(currentThreadId, threadId, "Thread Ids didn't match");
    }


    @DataProvider(name = "dp")
    public static Object[][] getData() {
        return new Object[][]{
                {1},
                {2}
        };
    }
}
import java.util.Collections;

public class IssueTest {
    public static void main(String[] args) {
        XmlSuite xmlSuite = new XmlSuite();
        xmlSuite.setName("sample_suite");
        xmlSuite.setParallel(XmlSuite.ParallelMode.INSTANCES);
        xmlSuite.setVerbose(2);
        XmlTest xmlTest = new XmlTest(xmlSuite);
        xmlTest.setName("sample_test");
        xmlTest.setParallel(XmlSuite.ParallelMode.INSTANCES);
        xmlTest.setVerbose(2);
        xmlTest.setXmlClasses(Collections.singletonList(new XmlClass(MultipleInstance.class)));
        System.err.println(xmlSuite.toXml());
        System.setProperty("testng.thread.affinity", "true");
        TestNG testng = new TestNG();
        testng.setXmlSuites(Collections.singletonList(xmlSuite));
        testng.run();
    }
}
@kobebryant2007
Copy link
Contributor

Hi,
Base on the test code above, the 'testng.thread.affinity = true' did some effect on test results but maybe in an unexpected way. Maybe this property doesn't work in 'instances' mode and it need deeper investigation.

@krmahadevan
Copy link
Member

@kobebryant2007 - Yep.. I am aware of some issues with this feature. This is another such issue. I am guessing that this feature cannot be applied on a wide scale and perhaps may end up being confined to very specific use cases alone.

This was an experimental feature to begin with. Let me see what can be done on this when I get some time.

@kobebryant2007
Copy link
Contributor

Yes, I think things would be complicated when facing multi-thread. I can do some investigation, but not sure could find the root cause.

@krmahadevan
Copy link
Member

@kobebryant2007 - Sure. Please do post back the findings.

@kobebryant2007
Copy link
Contributor

kobebryant2007 commented Sep 16, 2020

After several investigation, I found that the root cause is related to the rules of how to maintain the dependency relationship. For now, TestNG using the test method name to identify a node in graph, but this seem not work when using parameter test with dependsOnMethods.
I did several change in DependecyMap class to make sure that dependent method and independent method would use the same parameter. Then all test will be passed.
Just add baseClassMethod.getFactoryMethodParamsInfo().getParameters()[0] == derivedClassMethod.getFactoryMethodParamsInfo().getParameters()[0] in the return line in these two methods.

private static boolean isSameInstance(
      ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
    Object baseInstance = baseClassMethod.getInstance();
    System.out.println("begin");
    Arrays.stream(baseClassMethod.getFactoryMethodParamsInfo().getParameters()).forEach(System.out::print);
    System.out.println("end");
    Object derivedInstance = derivedClassMethod.getInstance();
    return derivedInstance != null
        && baseInstance != null
        && baseInstance.getClass().isAssignableFrom(derivedInstance.getClass())
        && baseClassMethod.getFactoryMethodParamsInfo().getParameters()[0] == derivedClassMethod.getFactoryMethodParamsInfo().getParameters()[0];
  }
private static boolean hasInstance(
      ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
    Object baseInstance = baseClassMethod.getInstance();
    Object derivedInstance = derivedClassMethod.getInstance();
    return (derivedInstance != null || baseInstance != null) 
            && baseClassMethod.getFactoryMethodParamsInfo().getParameters()[0] == derivedClassMethod.getFactoryMethodParamsInfo().getParameters()[0];
  }

and another change in GraphThreadPoolExecutor

private void mapNodeToWorker(List<IWorker<T>> runnables, List<T> freeNodes) {
    if (!RuntimeBehavior.enforceThreadAffinity()) {
      return;
    }
    for (IWorker<T> runnable : runnables) {
      for (T freeNode : freeNodes) {
        IWorker<T> w = mapping.get(freeNode);
        if (w != null) {
          long current = w.getThreadIdToRunOn();
          runnable.setThreadIdToRunOn(current);
        }
        if (runnable.toString().contains(freeNode.toString())) {
          mapping.put(freeNode, runnable);
        }
      }
    }
  }

It's a draft that only to explain the root cause, you can check it first.

BR

@krmahadevan
Copy link
Member

@kobebryant2007 - Great. Why not raise this as a PR itself (since you have done all the hard work of figuring out all of this stuff) ?

@kobebryant2007
Copy link
Contributor

Sure, I'll do it later.

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