Private @Before* methods aren't called when defined in a base class

[zhogov]

TestNG Version

6.14.3 and 7.0.0-beta3, Java 8 and 11

Actual behavior

Private @Before* methods aren't called when defined in a base class, but work when defined in the same class.

Expected behavior

Private @Before* methods should be called, whether they were inherited or not

Is the issue reproductible on runner?

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

Test case sample

Following test passes - calls @BeforeMethod

public class NoInheritanceTest {
    boolean called = false;

    @BeforeMethod
    private void init() {
        called = true;
    }

    @Test
    public void testWorks() {
        assertTrue(called, "Should have been set in @Before");
    }
}

Following test fails - @BeforeMethod is not called:

public class ChildTest extends BaseTest {
    @Test
    public void testFails() {
        assertTrue(called, "Should have been set in @Before");
    }
}

public class BaseTest {
    boolean called = false;

    @BeforeMethod
    private void init() {
        called = true;
    }
}

Test case project: https://github.com/zhogov/testng-issue-2097

[juherr]

Right, it is the expected behavior:

Behaviour of annotations in superclass of a TestNG class

The annotations above will also be honored (inherited) when placed on a superclass of a TestNG class. This is useful for example to centralize test setup for multiple test classes in a common superclass.

In that case, TestNG guarantees that the "@before" methods are executed in inheritance order (highest superclass first, then going down the inheritance chain), and the "@after" methods in reverse order (going up the inheritance chain).

https://testng.org/doc/documentation-main.html#annotations

@krmahadevan Could you confirm the issue? I'm surprised it is not working like that.

[Proryanator]

@juherr @zhogov we noticed this behavior too at my organization. I think the reason that inheritance only works with public annotated methods, is probably due to how Java does inheritance in general.

Leaving annotations aside, private methods in a superclass are not inherited by child classes. And since annotations are tied to methods, it would make sense that those annotated private methods are not accessible to child classes, and thus do not run.

When TestNG decides to run all annotated methods like @BeforeMethod/@AfterMethod in a class, it would look at those defined in the class itself, which will not include any private ones defined in the parent class :)

@juherr I would think this can be good for design, as it would allow you to define parent-only annotated methods that you do not wish your child classes to run. I'm glad @zhogov you brought it up here though!

Hope this might shed some light on this behavior.

[zhogov]

@Proryanator
As I understand this behavior doesn't follow the documentation.

I would expect this to work consistently across all method visibilities.

As for

I would think this can be good for design, as it would allow you to define parent-only annotated methods that you do not wish your child classes to run.

Maybe, we just need some clarification/decision from framework authors since it is a gray area now

[juherr]

@Proryanator Good catch, I didn't notice the private before.

The behavior with private test methods is not documented and works with setAccessible(true) which is not a good practice since Java9 (https://stackoverflow.com/a/41265267/4234729).

I think we should drop the support of private test methods in the next major version but we can just add a warning message.

@krmahadevan WDYT?

[krmahadevan]

@juherr - Sure... We can add warning messages whenever TestNG is being used to invoke private methods... And then strip support for them later.. Kind of doesn't make sense to be honest to have private methods annotated using TestNG annotations and then having them be invoked.

[Proryanator]

Do you think that could be like one of the intellisense errors you get if you tagged a private method with @BeforeMethod? Similar to that of deprecated but in a different sense.

I would assume this would apply for any TestNG annotation applied to a private method too, so that message ought to say those too.

Or one you'd see as a console warning? I'm not sure how these sorts of things work when warning downstream users :)

[Proryanator]

Peeking through old open issues, looks like there's still an open one here for the same issue.
#473

In general how does closing of old old issues work? A few members with the rights to close old open/duplicate issues? Just curious.

[krmahadevan]

@Proryanator

A few members with the rights to close old open/duplicate issues?

Yes. That is correct.

[juherr]

#473 has a similar title but looks a bit different.

@Proryanator At our side, only console warnings are possible but IDEs can manage a better display if they want to warn against this bad usage (ping @missedone @akozlova )

[akozlova]

IDEA could warn and actually warn about junit malformed methods but not about testNG. We plan to prettify the inspections for both test frameworks and this issue also should be addressed. Sorry for the inconvenience

[jiong-fu]

@juherr - Sure... We can add warning messages whenever TestNG is being used to invoke private methods... And then strip support for them later.. Kind of doesn't make sense to be honest to have private methods annotated using TestNG annotations and then having them be invoked.

@krmahadevan these private test methods from parent class are excluded at the beginning by the org.testng.internal.ClassHelper#canInclude method. I can imagine that warning messages can be printed out there, but it is expensive to check if a private method is annotated with one of the TestNG annotations with reflection.

[krmahadevan]

@jiong-fu - Sure it is. But then again it would be a one time exercise that gets done at the beginning of the execution right?

Also I am not sure, how much of an additional cost are we adding here which warrants a concern. Would you happen to have any info around that ?

[jiong-fu]

@jiong-fu - Sure it is. But then again it would be a one time exercise that gets done at the beginning of the execution right?

Yes, the collection of the test methods to be executed gets done only once by initialization method.

Also I am not sure, how much of an additional cost are we adding here which warrants a concern. Would you happen to have any info around that ?

I assume that you referred to the additional cost in performance? I am not sure if the performance was somehow measured / monitored before and if it is a big concern.

[Proryanator]

It shouldn't be any more cost that it's already doing, canInclude() already looks at whether it's public or private, so we're already doing reflection there 🤔

Right in extractMethods, that unused part of the if, you could log the warning there.

Of course, if any other rules change inside of canInclude, you can't necessarilly say "Skipping @BeforeMethod because it's private" just from canInclude().

[jiong-fu]

@Proryanator

It shouldn't be any more cost that it's already doing, canInclude() already looks at whether it's public or private, so we're already doing reflection there 🤔

In addition to visibility check, a method should be checked whether it is annotated as TestNG configuration method. Otherwise for example an unannotated private method in base class is excluded only because it is not visible to the child.

Right in extractMethods, that unused part of the if, you could log the warning there.

With the current implementation, it can only be concluded that a method should not be included, but the reason may vary.

[Proryanator]

Ah I see, those private methods I mentioned are just for methods in general.

Yeah you're right, somewhere outside of there you'd need to also check if they're TestNG annotated methods before showing the warning, good catch!