-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener, ISuiteListenerexecute in the order of insertion #2558
Comments
How about keep listener order default behavior and this requirement as improvement ? |
Duplicate of #2089 Maybe you should have a look at https://github.com/sbabcoc/TestNG-Foundation |
@juherr Maybe my statement above is not so clear. |
@dianny123 - You might want to consider leveraging composition for this. On a high level you can try the following:
Can you check if that would work for you ? |
@krmahadevan Thank you for your support! |
@dianny123 - Are your listeners part of your suite file or are they injected via 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). |
@krmahadevan 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. |
@krmahadevan @juherr Do you have any updates and comments? I stopped here. |
@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. |
Thanks for your reply! I'm appreciated discussing with you when you are free. |
@krmahadevan Hi |
@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
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. |
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. |
@krmahadevan @juherr Could we change the status to open? |
Since TestNG has never guaranteed the order of listeners, we can easily just flip the
That should solve this issue right ? Do you see any other complexity on this ? |
Have we started with building this list ? |
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.
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. |
@krmahadevan Can I understand it as this request has been accepted, but when and how to implement is on discussion? |
@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. |
@krmahadevan What if TestNG provides an |
@juherr - Infact, here are some of the orders that I can think of
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 ? |
It sounds like an SPI where you can plug your own order logic before storing all found listeners. |
@juherr Yes... Absolutely. I will try to see what I can come up with. |
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.
May I know what's the plan if make a quick fix? Does it means the insertion order? |
Yes, it could be another way to specify the order value
Quickfix: replace listener maps by LinkedHashMap |
Sounds good! I will pull the change for review this week. |
@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. |
@dianny123 - I have added some comments. Please help take a look |
TestNG Version
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
The text was updated successfully, but these errors were encountered: