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

[eyehwan]

TestNG Version

Latest 7.0.0 SNAPSHOT

Expected behavior

If multiple listeners implements the same ITestNGListener, like IExecutionListener, ITestListener, IInvokedMethodListener or IConfigurationListener, 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. If those listeners are independed, that's fine. But if there is a dependency among them, this requirement is neccessary.

Is the issue reproductible on runner?

All

Test case sample

Will submit a pull request.

[eyehwan]

Here I use an example to the expected behavior. Three Listeners defined like this:

Listeners1 implements IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener {}
Listeners2 implements IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener {}
Listeners3 implements IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener {}

Put them in the testng xml file in this order:

  <listener class-name="org.testng.listener.order.self.Listeners1 " />
  <listener class-name="org.testng.listener.order.self.Listeners2 " />
  <listener class-name="org.testng.listener.order.self.Listeners3 " />

Simple test case:

@BeforeMethod
public void beforeMethod() {}
@Test
public void testMethod() {}

Execution order would be fixed as inserted sequence defined in xml file：

Listener1.onExecutionStart
Listener2.onExecutionStart
Listener3.onExecutionStart

    Listener1.onStart(ITestContext context)
    Listener2.onStart(ITestContext context)
    Listener3.onStart(ITestContext context)

        Listener1.beforeConfiguration
        Listener2.beforeConfiguration
        Listener3.beforeConfiguration
        Listener1.beforeInvocation
        Listener2.beforeInvocation
        Listener3.beforeInvocation
            beforeMethod
        Listener3.afterInvocation
        Listener2.afterInvocation
        Listener1.afterInvocation  
        Listener3.onConfigurationSuccess
        Listener2.onConfigurationSuccess
        Listener1.onConfigurationSuccess 

        Listener1.onTestStart
        Listener2.onTestStart
        Listener3.onTestStart   
        Listener1.beforeInvocation
        Listener2.beforeInvocation
        Listener3.beforeInvocation
            testMethod
        Listener3.afterInvocation
        Listener2.afterInvocation
        Listener1.afterInvocation   
        Listener3.onTestSuccess
        Listener2.onTestSuccess
        Listener1.onTestSuccess

    Listener3.onFinish
    Listener2.onFinish
    Listener1.onFinish

Listener3.onExecutionFinish
Listener2.onExecutionFinish
Listener1.onExecutionFinish

[juherr]

Could you explain the why of the feature? Why do you need it?

In fact, listeners are supposed to be independent without any dependency between them. In that case, the order is not really important. As a workaround, if an order is important, you can use a composite listener with the other listeners and manage the order there.

[eyehwan]

In fact, listeners are supposed to be independent without any dependency between them

In fact, in many cases, you cannot control over so called independency. For example, some Listener we used comes from other projects, we want to add some pre/post handling around it in your own listeners. In this case, you have to make sure control over the execution order of listeners. I make a quick search on stackoverflow, similar requirement also popped up by other TestNG users; https://stackoverflow.com/questions/11084157/is-there-a-way-to-define-the-runorder-of-two-different-testng-listeners

As a workaround, if an order is important, you can use a composite listener with the other listeners and manage the order there.

Yes, we as users have been using TestNG for years, and this requirement also exits for long time. there must be many workaround, like yours. But why don't we just make it support in TestNG directly.

[juherr]

The @cbeust 's answer from your StackOverflow link and mine here are the same. TestNG is an opinionated project and I am always feeling nervous when we change the behavior without @cbeust 's point of view. By experience, all users don't have the same requirements and it's always difficult to guarantee a full consensus.

Instead, I prefer keeping the current behavior out of the box when possible and be able to configure it. It is more work but it is the best compromise IMO. Here, we can imagine an IListenerInterceptor which has the listener order responsibility used everywhere instead of listener collections. Do you think something like that is possible?

[eyehwan]

@juherr may I understand your comments as:

We can provide listener order as a new feateure, meanwhile we also have to provide backward compitable guarantee that is execution in disorder as default behavior?

Here I show my opinion, I agree that listeners execution in disorder is current behavior. But I don't think this behavior deserve to keep comitability, because discorder is NOT intended feature provided by TestNG. I prefer calling " Listener order " as a improvement feature，to make up for deficiencies.

But anyway, @juherr, @krmahadevan and @cbeust , I need your comments and making alignments about how to handle this feature, mainly focus on the point: Do we need to keep "Listener execution in disorder" as the default behavior?

Once I got your confirm, we can continue, otherwise, we are blocked here.

[juherr]

I'm just saying that execution in order is just a behavior change and an improvement will be a new SPI. I'm not opposed to a behavior change if we are able to configure it. We already had some troubles in the past because we changed the default behavior (and never thought it will have bad effects).

[krmahadevan]

@juherr - IMO, we don't need to be worried a lot in terms of listener execution order because

1. Their order was never deterministic. So we dont have the risk of causing backward compatibility.
2. This is a new major version which means we are allowed to be non backward compatible.

IMO, being able to have a deterministic listener execution order is useful, especially when I want to daisy chain listeners, but dont want to resort to composition (some listener implementations may not even be composition friendly).

Also since this implementation is merely guaranteeing that the listener wiring in will be exactly as how it was given to TestNG, I think its a good feature.

IMO, we should go ahead with getting this done.

[juherr]

My point is not on the order but on an SPI in order to do it.

[krmahadevan]

@juherr - Can you please elaborate on the SPI part ? I didn't quite understand it. Are you asking for a service loader kind of mechanism that would basically do this ?

[juherr]

@krmahadevan Just a callback somewhere in order to let the user changing listener order. Like we already do for methods with IMethodInterceptor

[eyehwan]

If we use callback to configure listener order, it is an extra action to ask for users to do so, it is not that convenient, especially when we only want to keep listeners insert order.

How about we devide it into two stories,

1. Keep listeners insert order as default behavior
2. Make listener order configurable using callback.

Could we handle these two stories seperately? @krmahadevan @juherr

[juherr]

The stories are good but IMO we can't merge 1. without 2.

[eyehwan]

The stories are good but IMO we can't merge 1. without 2.

@juherr Yes, I can handle it on my pull request branch #2090 with iterative development, hopefully within one month. @krmahadevan What do you think about those two stories?

[krmahadevan]

@eyehwan - Sure I am fine with the user stories... I am fine with just the first user story because my stance is that, TestNG has so far not guaranteed any order for the listeners. So none of our users are going to even notice that there is an order being followed and that is insertion order.

But I guess it doesn't hurt to expose this capability via an ordering mechanism that is user controlled. That way we enable people to order their listeners to their wish (which in your case is insertion order).

So I would suggest that we start from (2) and then work backwards to (1). That would be more logical.

In fact (1) may not even be required, because this customization can very well reside outside of TestNG and TestNG continues to retain its current behavior which is "un-determined order for listeners invocation"

[krmahadevan]

@eyehwan - Just one more thing... If possible start with a skeleton for (2) get feedback and then proceed with the implementation. That would help save you a lot of trouble and large scale rework.

[eyehwan]

@krmahadevan thanks for your suggestions, then I will start from 2.