Make IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener, ISuiteListenerexecute in the order of insertion

[dianny123]

TestNG Version

The latest

Expected behavior

If multiple listeners implements the same ITestNGListener, like IExecutionListener, ITestListener, IInvokedMethodListener IConfigurationListener, or ISuiteListenerexecute, those listeners should be executed as inserted order when **Start etc , meanwhile reverse order when **Finish/Failure etc.

Actual behavior

Listeners implments the same ITestNGListener are executed inorderred.

Is the issue reproductible on runner?

All

Test case sample

I found this requirement has been request before, the history: issue-2089

[dianny123]

How about keep listener order default behavior and this requirement as improvement ?
The draft solution is that provide an option like -listenerinsertorder=true to trigger this requirement.
Its default value is false.

[juherr]

Duplicate of #2089

Maybe you should have a look at https://github.com/sbabcoc/TestNG-Foundation

[dianny123]

@juherr Maybe my statement above is not so clear.
I agree with that TestNG-Foundation can handle listener order.
But we have thousands of suite files and listeners registered in those files. If we handle listener order via ListenerChain, we have to modify those suite files and related classes, that's cost too much effort, and we want to avoid this cost.
Is there a way to handle listener order without changing suite files?
Do you have any ideas?

[krmahadevan]

@dianny123 - You might want to consider leveraging composition for this.

On a high level you can try the following:

1. Build 1 TestNG listener that implements all the interfaces that your other listeners implement and wire in this as a service provider interface loaded listener (SPI)
2. Within this listener, you manually instantiate and maintain the list of each of these listeners.
3. Now for every listener method, you just need to iterate through your (per listener type list) and invoke them in the order you want.

Can you check if that would work for you ?

[dianny123]

@krmahadevan Thank you for your support!
Based on your solution, I'm considering that listeners in suite file will be invoked twice, one is by TestNG, one is by this solution.
For my personal understanding, this solution gets listeners from suite file and then arrange the order, it not affect the TestNG execution process, so TestNG will invoke listeners too.
If there is something wrong, pls correct me, thank you!

[krmahadevan]

@dianny123 - Are your listeners part of your suite file or are they injected via @Listeners annotation ? Would it be possible for you to remove them from there?

If its not possible, then yes TestNG would end up invoking the listeners via these mechanisms once and then TestNG would end up invoking them again via the orchestrating listener which i explained earlier.

You may need to add some additional code which would ensure that the listener doesnt get invoked once again (Which to me sounds like just more complications).

[dianny123]

@krmahadevan
We configure listeners both in suites and with @listeners.
yes we don't want to modify suite files and classes.
Is it possible to intervene TestNG process at runtime from a high level? That I mean I should avoid listeners invoked by TestNG. I really don't know how to do it.

On the other hand, could you explain why TestNG not considered accepting this request? As I said above we can keep listener default behavior and make this as an improvement. I think it is low risk.
Thank you again!

[dianny123]

@krmahadevan @juherr Do you have any updates and comments? I stopped here.

[krmahadevan]

@dianny123 - Given the fact that this can be driven by a configuration, I would like to pursue this. But before that I would like to check out what does https://github.com/sbabcoc/TestNG-Foundation do so that I have a better understanding of everything. But I am right now swamped with deliverables at work and so its going to take sometime before i can get to this.
So request you to please help give me some time.

[dianny123]

Thanks for your reply! I'm appreciated discussing with you when you are free.

[dianny123]

@krmahadevan Hi
I'm just to make sure you didn't miss this topic. 😊

[krmahadevan]

@dianny123 - Thanks for the reminder.

@juherr - I dont think we can do this via a configuration driven approach wherein we give the ability to the users to toggle this feature because I was thinking that for us to implement this, we are better off with using a insertion order aware data structure so that the changes are simple and straightforward.

Now comes the next question of how is the order of insertion defined ?

A listener can be inserted via one of the following approaches

1. via the TestNG suite xml file using the <listeners> tag.
2. via the @Listeners annotation
3. via service loaders
4. via TestNG configuration parameter (usually set via a build tool plugin for e.g., the maven surefire plugin)
5. Programmatic alteration of the suite (Theoretically this is possible, but am yet to vet out whether this actually works).

So now, how do we define the insertion order ? Is it merely sufficient that we just say "TestNG honours the insertion order, but doesnt really care about how the insertion happens (or) should TestNG also start recognising the mode of insertion" ?

@dianny123 - Please feel free to chime in with your expectations as well.

[juherr]

About the listener insertion ways, all are scoped for the suite and they are just different ways to do the same thing. I can't say what should be the order and why.

To be honest, implementing the feature will increase the code complexity.
In terms of priorities, I think it will be better to clean current features and design first and add new things later.

[dianny123]

@krmahadevan @juherr Could we change the status to open?

[krmahadevan]

@juherr

To be honest, implementing the feature will increase the code complexity.

Since TestNG has never guaranteed the order of listeners, we can easily just flip the Map type into LinkedHashMap in

• https://github.com/cbeust/testng/blob/9ea649f6c66ab6165813af7a6c28cbde13c0b685/testng-core/src/main/java/org/testng/TestNG.java#L143-L153
• https://github.com/cbeust/testng/blob/9ea649f6c66ab6165813af7a6c28cbde13c0b685/testng-core/src/main/java/org/testng/SuiteRunner.java#L46-L48

That should solve this issue right ? Do you see any other complexity on this ?

[krmahadevan]

@juherr

In terms of priorities, I think it will be better to clean current features and design first and add new things later.

Have we started with building this list ?

[juherr]

we can easily just flip the Map type into LinkedHashMap
That should solve this issue right ? Do you see any other complexity on this ?

Sure, we can do it if that's all! I'm not sure it will be enough and I think the whole management of listeners should be thought with a fresh eye before modifying it deeper.

Have we started with building this list ?

Nope :) The first step could be to list all TestNG features. Then, we can check features by feature what improvements can be done. TBH, this document part is not really fun and I've never started it.

[dianny123]

@krmahadevan Can I understand it as this request has been accepted, but when and how to implement is on discussion?

[juherr]

@dianny123 The tradeoff is we can make a quick fix where the order is better. But we need more time before specifying a full predictable order.

[juherr]

@krmahadevan What if TestNG provides an @Order(int value) for listener class level?
It will be easy to sort and use.
No order on listener means default value.
No guarantee in the order for listeners with the same order value (maybe the insertion order)

[krmahadevan]

@juherr - Infact, here are some of the orders that I can think of

1. Order based on some order (This will work only for listeners in my test project, but what about those listeners that are loaded via service providers ? )
2. Insertion order
3. Natural ordering or reverse of it (alphabetical and reverse alphabetical)

While we are at this topic, I thought its better if we call out what all we want to support and what we dont want to.

I was thinking that we should abstract out the entire process of listener ordering and let the end user define the way in which it should be ordered (I am thinking that maybe we can consider exposing some sort of ordering mechanism that an end user can plugin to us via a Service Loading mechanism or a configuration parameter. We will use ONLY 1 of it as given to us by the end user and then rely on it to order all of the listeners across the board.

I hope its not sounding a bit too vague.

WDYT ?

[juherr]

It sounds like an SPI where you can plug your own order logic before storing all found listeners.
I think it is a good objective but I don't know if it could be easy to implement. Wanna try?

[krmahadevan]

@juherr Yes... Absolutely. I will try to see what I can come up with.

[dianny123]

What if TestNG provides an @Order(int value) for listener class level?
It will be easy to sort and use.

I think this way is not friendly to one kind of users who has many many suites and classes. Because suites and classes should do some changes based on this way. I think an option such as vm arguments is better.

@dianny123 The tradeoff is we can make a quick fix where the order is better. But we need more time before specifying a full predictable order.

May I know what's the plan if make a quick fix? Does it means the insertion order?

[juherr]

I think an option such as vm arguments is better.

Yes, it could be another way to specify the order value

May I know what's the plan if make a quick fix? Does it means the insertion order?

Quickfix: replace listener maps by LinkedHashMap

[dianny123]

Quickfix: replace listener maps by LinkedHashMap

Sounds good! I will pull the change for review this week.

[dianny123]

@krmahadevan Thank you for your quickFix. Based on your code, I have one additional request that the listener **Finish method work with reverse insertion order.
I create a pull request ( #2627 ), pls have a review and give comments.
Thank you!

[krmahadevan]

@dianny123 - I have added some comments. Please help take a look