Configuration methods have the same test class instance when IInvokedMethodListener is being used with @Factory

[RiJo]

TestNG Version

7.4.0

7.5.0-SNAPHOT (testng-7.5.0-20210517.112015-26.jar)

7.1.1

Expected behavior

For each configuration method (@​BeforeClass, @​BeforeMethod, etc.) corresponding test class instance created with @​Factory is set in parameters of IInvokedMethodListener calls. E.g. org.testng.ITestNGMethod#getInstance and org.testng.ITestResult#getInstance.

Actual behavior

For most configuration methods (@​BeforeClass, @​BeforeMethod, etc.), the test class instance created with @​Factory is not properly set in parameters of IInvokedMethodListener calls. E.g. org.testng.ITestNGMethod#getInstance and org.testng.ITestResult#getInstance. The behavior is not consistent.

This is a continuation of #2428, where a similar issue was solved but for configuration annotations. This issue is about utlizing the org.testng.IInvokedMethodListener instead.

It is probably causing problems with the solution for #2426, where one now get the factory parameters, but for the wrong test class instance in some cases when @​Factory is used.

Is the issue reproductible on runner?

• [ ] Shell
• [X] Maven
• [ ] Gradle
• [ ] Ant
• [ ] Eclipse
• [X] IntelliJ
• [ ] NetBeans

Test case sample 1

@Listeners(FactoryTest.class)
public class FactoryTest implements IInvokedMethodListener {

    @Override
    public void beforeInvocation(final IInvokedMethod method, final ITestResult testResult) {
        System.out.println(method.getTestMethod().getConstructorOrMethod().getName() +
                " - " + method.getTestMethod().getInstance() +
                " - " + testResult.getInstance());
    }

    @Factory
    public static Object[] factory() {
        return new Object[] {
                new FactoryTest(),
                new FactoryTest(),
                new FactoryTest()
        };
    }

    @BeforeClass
    public void beforeClass() {
    }

    @BeforeMethod
    public void beforeMethod() {
    }

    @Test
    public void test() {
    }

    @AfterMethod
    public void afterMethod() {
    }

    @AfterClass
    public void afterClass() {
    }
}

Console output

beforeClass - com.example.FactoryTest@13a5fe33 - com.example.FactoryTest@13a5fe33
beforeMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
test - com.example.FactoryTest@13a5fe33 - com.example.FactoryTest@13a5fe33
afterMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
afterClass - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
beforeClass - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
beforeMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
test - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
afterMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
afterClass - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
beforeClass - com.example.FactoryTest@527740a2 - com.example.FactoryTest@527740a2
beforeMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
test - com.example.FactoryTest@527740a2 - com.example.FactoryTest@527740a2
afterMethod - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc
afterClass - com.example.FactoryTest@3108bc - com.example.FactoryTest@3108bc

===============================================
Default Suite
Total tests run: 3, Passes: 3, Failures: 0, Skips: 0
===============================================

As one can see in the console output, "3108bc" is used in most cases, even though other instances should be used. No more configuration annotations have been tested other than the ones in my example above.

Test case sample 2

Using @​Factory with data provider for the construtor is broken as well:

@Listeners(FactoryTest.Listener.class)
public class FactoryTest {

    public static class Listener implements IInvokedMethodListener {

        @Override
        public void beforeInvocation(final IInvokedMethod method, final ITestResult testResult) {
            System.out.println(method.getTestMethod().getConstructorOrMethod().getName() +
                    " - " + method.getTestMethod().getInstance() +
                    " - " + testResult.getInstance());
            System.out.println("");
        }
    }

    @Factory(dataProvider = "constructorArguments")
    public FactoryTest(final int parameter) {

    }

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

    @BeforeClass
    public void beforeClass() {
    }

    @BeforeMethod
    public void beforeMethod() {
    }

    @Test
    public void test() {
    }

    @AfterMethod
    public void afterMethod() {
    }

    @AfterClass
    public void afterClass() {
    }
}

Console output

beforeClass - com.example.FactoryTest@2b2948e2 - com.example.FactoryTest@2b2948e2
beforeMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
test - com.example.FactoryTest@2b2948e2 - com.example.FactoryTest@2b2948e2
afterMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
afterClass - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
beforeClass - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
beforeMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
test - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
afterMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
afterClass - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
beforeClass - com.example.FactoryTest@eec5a4a - com.example.FactoryTest@eec5a4a
beforeMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
test - com.example.FactoryTest@eec5a4a - com.example.FactoryTest@eec5a4a
afterMethod - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0
afterClass - com.example.FactoryTest@6ddf90b0 - com.example.FactoryTest@6ddf90b0

===============================================
Default Suite
Total tests run: 3, Passes: 3, Failures: 0, Skips: 0
===============================================

Debugging

A conditional break point, arguments.getInstance() != tm.getInstance(), can be put here: https://github.com/cbeust/testng/blob/4170ef68fb8203265cba4f6bfdbfb9c4a0b63f05/src/main/java/org/testng/internal/ConfigInvoker.java#L315

[juherr]

Except when the parallel feature is used, the test instance should be the same everywhere.

In your sample, your test class is implementing a listener which should not be allowed. For example, this use-case will fail if you use a custom constructor because TestNG won't be able to instantiate the listener.

Can you post the output of your samples where the listener is a dedicated class?

[RiJo]

I don't expect the test instance to be the same, even with parallel feature disabled, since the factory produces three independent instances.

I wanted to give a dense example. "Test case sample 2" in the description has a separated listener implementation. Below you can find test case sample where the classes are separated:

public class MyFactory {

    @Factory
    public static Object[] factory() {
        return new Object[] {
                new FactoryTest(),
                new FactoryTest(),
                new FactoryTest()
        };
    }
}

public class MyListener implements IInvokedMethodListener {

    @Override
    public void beforeInvocation(final IInvokedMethod method, final ITestResult testResult) {
        System.out.println(method.getTestMethod().getConstructorOrMethod().getName() +
                " - " + method.getTestMethod().getInstance() +
                " - " + testResult.getInstance());
    }
}

@Listeners(MyListener.class)
public class FactoryTest {

    @BeforeClass
    public void beforeClass() {
    }

    @BeforeMethod
    public void beforeMethod() {
    }

    @Test
    public void test() {
    }

    @AfterMethod
    public void afterMethod() {
    }

    @AfterClass
    public void afterClass() {
    }
}

Console output:

beforeClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
beforeMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
test - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9

beforeClass - com.example.FactoryTest@47ef968d - com.example.FactoryTest@47ef968d
beforeMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
test - com.example.FactoryTest@47ef968d - com.example.FactoryTest@47ef968d
afterMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9

beforeClass - com.example.FactoryTest@5bc79255 - com.example.FactoryTest@5bc79255
beforeMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
test - com.example.FactoryTest@5bc79255 - com.example.FactoryTest@5bc79255
afterMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9

===============================================
Default Suite
Total tests run: 3, Passes: 3, Failures: 0, Skips: 0
===============================================

This is the expected output grouped by instance for readability (i.e. a chunk per test class instance):

beforeClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
beforeMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
test - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterMethod - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9
afterClass - com.example.FactoryTest@23e028a9 - com.example.FactoryTest@23e028a9

beforeClass - com.example.FactoryTest@47ef968d- com.example.FactoryTest@47ef968d
beforeMethod - com.example.FactoryTest@47ef968d- com.example.FactoryTest@47ef968d
test - com.example.FactoryTest@47ef968d- com.example.FactoryTest@47ef968d
afterMethod - com.example.FactoryTest@47ef968d- com.example.FactoryTest@47ef968d
afterClass - com.example.FactoryTest@47ef968d- com.example.FactoryTest@47ef968d

beforeClass - com.example.FactoryTest@5bc79255- com.example.FactoryTest@5bc79255
beforeMethod - com.example.FactoryTest@5bc79255- com.example.FactoryTest@5bc79255
test - com.example.FactoryTest@5bc79255- com.example.FactoryTest@5bc79255
afterMethod - com.example.FactoryTest@5bc79255- com.example.FactoryTest@5bc79255
afterClass - com.example.FactoryTest@5bc79255- com.example.FactoryTest@5bc79255

[juherr]

Agree with the expected output. The current behavior is really strange.

Do you want to dig a bit and try to find the issue?

[RiJo]

Great! I will push a commit with failing unit tests to start with. Since I'm not familiar with the code yet, it would be good if someone with knowledge can take a look at it. I can do some more debugging as well, to see if I can locate the cause. Thanks!

[juherr]

A PR with a failing unit test is already a good step! 👍

[RiJo]

My last commit contains failing unit tests (the previous was amended to fix some mistakes). Should I create a PR for it as well?

The tests currently fails locally as expected:

====================================================================================================
    ::::::Failed Tests for Suite ::: [factory test] ::: Test name [factory test]::::::
    ====================================================================================================
    test.factory.github2560.Github2560Test.staticFactory()
    Exception:
    java.lang.AssertionError: Maps do not match for key:0 actual:[beforeClass, test] expected:[beforeClass, beforeMethod, test, afterMethod, afterClass]
        at test.factory.github2560.Github2560Test.staticFactory(Github2560Test.java:21)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
    ... Removed 15 stack frames

    test.factory.github2560.Github2560Test.constructorFactory()
    Exception:
    java.lang.AssertionError: Maps do not match for key:0 actual:[beforeClass, test] expected:[beforeClass, beforeMethod, test, afterMethod, afterClass]
        at test.factory.github2560.Github2560Test.constructorFactory(Github2560Test.java:37)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
    ... Removed 15 stack frames

    ====================================================================================================

[juherr]

Whatever you prefer: you can keep the commit or make a draft PR.

[RiJo]

I've done some additional testing in older TestNG versions, and here are my findings:

• 7.1.1 has the same issue, but with a different character: only @​Test method is correct, opposed to 7.4.0 where also @​BeforeClass is correct as it seems.
• Maybe the fixes for #2428 actually solved the @​BeforeClass, but nothing more? To me it looks like those fixes only cared about the @​BeforeClass, and that similar work should also be done for the remainder of configuration methods.

[RiJo]

I played around a bit more at the master branch, and the changes from #2428 partially solved this issue (specifically the @​BeforeClass case). I pushed my local test code, where I also fixed the @​AfterClass annotation, to the PR: 888fc515823cfd8adcd36eaeb137d4383dbfc0ca (note: it is only for testing purposes and not intended to submit to master branch as it is!)

The problem I face now is the limitations of ITestClassConfigInfo interface which only supports the @​BeforeClass case. Potentially a new interface is required, or if an existing one should be extended (as in my example commit). I think someone else in the TestNG community, more familiar with this code, should have a look at it instead to solve it properly. Thanks!

[juherr]

Thanks! :+1:

[RiJo]

Any updates on this issue? Anything I can do to help? Thanks!

[juherr]

@RiJo Is #2562 ready for review? Could you check? I will be happy to review and merge it then.

[RiJo]

Yes, someone needs to have a look if the current implementation makes sense before it can be finalized. As I said earlier, there're some limitations of the ITestClassConfigInfo interface which must be dealt with first. Thanks