Listener's onAfterClass is called before @AfterClass configuration methods are executed.

[pshevche]

TestNG Version

7.6.1 and older

Expected behavior

I would expect that listener's onAfterClass method is executed after onConfiguration* methods for all methods annotated with @AfterClass are called. Similarly to how onBeforeClass is called before the first method annotated with @BeforeClass is called.

Or at least that the ordering is consistent for both types of configuration methods.

Actual behavior

• onBeforeClass is called before configuration methods annotated w/ @BeforeClass are executed.
• onAfterClass is called before configuration methods annotated w/ @AfterClass are executed.

Is the issue reproducible on runner?

Issue is reproducible both on Gradle and Maven runners, did not check anything else. Here is a small Gradle project with listener and test case implementations that illustrates the problem: https://github.com/pshevche/testng-after-class-order-reproducer

Test case sample

• Test case: https://github.com/pshevche/testng-after-class-order-reproducer/blob/main/lib/src/test/java/dev/pshevche/AfterClassTest.java
• Listener implementation: https://github.com/pshevche/testng-after-class-order-reproducer/blob/main/lib/src/main/java/dev/pshevche/LoggingListener.java

Actual output:

onBeforeClass: dev.pshevche.AfterClassTest
onConfigurationSuccess: setupClass
onTestStart: test1
onTestSuccess: test1
onAfterClass: dev.pshevche.AfterClassTest
onConfigurationSuccess: cleanupClass

[RSXEnthusiast]

I've also noticed and been frustrated by this.

[juherr]

Hi,

Could you explain why you think it is better?

If you see configuration methods like a easy way to write listeners, you expect that the order stay.
Even if it is often not possible, listeners are not supposed the have dependency between them.

Do you mean that you expect after methods are run in the reverse order than before methods? (I don't remember the current behavior)
Do you think this behavior will the same for all configuration methods ?

[cbeust]

I share @juherr's concern.

I understand where this change is coming from and I think an argument can be made that "after" listeners should be run after their After method, but this change is very likely to break existing users, so it worries me.

[pshevche]

Hi,

Could you explain why you think it is better?

My main issues with the current state are that class-level listeners are not symmetric, do not fit any of the patterns set by other listeners (i.e. sets wrong user expectations) and do not consistently identify test class boundaries.
On one side, onBeforeClass identifies the test class start and onConfiguration* for @BeforeClass methods make it seem like methods are executed within class boundaries and contribute to its duration, outcome, whatever.
On the other hand, onAfterClass does not necessarily identify class finish and@AfterClass are reported in a way that they seem to belong rather to the test level.
Let me know if I misunderstand the initial design and that current way better reflects the framework's internals.

Do you mean that you expect after methods are run in the reverse order than before methods?

I don't have any expectations in this area. It is not guaranteed that we have the same amount of after- and before-methods or whether they have dependencies between them. But I would still expect some sort of symmetric behavior of listeners.

Do you think this behavior will the same for all configuration methods ?

I also don't have any preference here or a suggestion for the correct way. Currently, there are two general approaches, and class-level listeners do not fit any of those, and that's what I mainly find confusing.

E.g. for suites and tests you would get:
onStart(ISuite)/onStart(ITestContext)
onConfiguration* for all @BeforeSuite/@BeforeTest methods
onConfiguration* for all @AfterSuite/@AfterTest methods
onSuiteFinish(ISuite)/onTestFinish(ITestContext)

For methods, it looks different, but also understandable:
onConfiguration* for all @BeforeMethod methods
onTestStart
onTest*
onConfiguration* for all @AfterMethod methods

And for classes it is neither of those. As I said, if this better reflects the framework's way of working, that is perfectly fine. If not, then I think it would be beneficial for users to have them symmetric and align with one of the existing structures.

[RSXEnthusiast]

Hi,

I don't think I could've said it better than @pshevche. As far as initial design, looking at the initial issue, that user seems to have just wanted a way to print out something before each test class runs. I think it would make sense for a user to want something to print after the entire test class has run, rather than after all the tests have run, but before the class has finished.

That said, I do share the worry that it could break existing users. I'd say it's worth the risk to make functionality more predictable, but I'm also not the one that might have broken tests due to the change.

[leonard84]

@cbeust if you are really worried that a fix would break existing users, would you consider introducing a new listener interface, that has the "correct" behavior, or introduce a new default method onAfterClassAfterMethods that gets called at the correct time?

[juherr]

@leonard84 We already added some engine configurations for behavior changes in the past. The current one could use the same mechanism.