search

testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.99k stars 1.02k forks source link

Add support for Priority in Configuration Methods #2663

Open martinaldrin opened 3 years ago

martinaldrin commented 3 years ago

Hi,

This is a feature that we have been asked by many of our users for years now. And I thought that it might be easier to create a pull request to start the discussion. The code is not ready and lacking allot of test. I just wanted to push a draft that shows that it is possible without to much effort.

Priority on configuration methods is something our users have requested from us for long time, today many of our user control the order by the naming of the methods, but I think it would be much nicer to to reuse the support that already exists in TestNg for Test Methods and I can not see any problem of adding such functionality. Many of our users can have over 50 before/after methods and adding dependency between them make it very complicated to maintain. much easier if the users can group the methods with different priorities.

What is your opinion about adding Priority support for Configuration Methods, if you are ok I will continue with this PR. I expect it to work in similar way as for Test Annotation.

I tried to bring this up in the forum many years ago, but did not get much response. So I will try here instead. https://groups.google.com/g/testng-dev/c/qSZFOYRWHkY/m/JzcswgrnAAAJ

juherr commented 3 years ago

I'm not sure mixing configuration methods and priority will be a good idea. The main reason is that priority is a global ordering feature with the suite scope. It means you will create a dependency between all @BeforeMethod of the suite. As the default priority is 0, all @BeforeMethod will be run together by default.

Is it the expected behavior?

martinaldrin commented 3 years ago

Hi, I'm not sure if I understand your concern. In my test I don't see that behavior that you mention. Are you sure the priority is on suite level, because if that was the case I guess my test would have a totally different behavior.

The only problem I have is to get my unit test working when I add the test to "testng-core/src/test/resources/testng.xml" Then I get a different behavior when I run the test with "gradle build" then it seems that priority is not working at all.

/Martin

juherr commented 3 years ago

In fact, I've really no idea what will be the behavior of the test engine if you add a dependency feature on configuration methods because they don't share the same logic as test methods. Edit: I didn't remember we already have dependsOnMethods on configuration methods.

From the functional point of view, I don't know what will be the expected behavior because priority is suite scoped, not class scoped. It means all configuration methods will be considered when edges will be created.

I think the feature won't be easy to add and need a lot of tests.

@krmahadevan thought?

martinaldrin commented 3 years ago

The priority is not a global feature, e.g. it will sort the method within the same annotation type. E.g. It allows you to sort which BeforeSuite or BeforeMethod should be executed first, today many users solves this by name the so they are sorted in correct order. I don't think this will conflict with any other features either.

krmahadevan commented 3 years ago

The priority is not a global feature, e.g. it will sort the method within the same annotation type. E.g. It allows you to sort which BeforeSuite or BeforeMethod should be executed first, today many users solves this by name the so they are sorted in correct order. I don't think this will conflict with any other features either.

@martinaldrin To be honest, I dont think there are a lot of users who establish dependency amongst configuration methods. In the close to a decade of being part of TestNG and responding to different queries on different forums (Github, LinkedIn, Stackoverflow, Google forums) I have seen people ask about dependencies between tests, but dependency between configurations is something that i have not heard of. Also this solution/alternative of users resorting alphabetising their configurations in order to get the order right is not documented anywhere to the best of my knowledge and I have not seen too many users who dig into the code to that level to find out this information.

I second what @juherr said that this feature of adding a priority to configuration methods is not something that is going to be easy to solve.

Today TestNG has a notion of method interceptors which are basically invoked by TestNG wherein it allows the testng user to order the tests to whatever order they want. Maybe we can consider building a method interceptor for configurations as well, wherein we just let the testng user tell us what is the order in which they would like to run the test. That way, it needn't be just priorities or alphabetical ordering by names. It could be anything. That functionality would ensure that the customisation or configuration method ordering is externalised with minimal changes. The test method interceptor AFAIK only extracts out independent methods and gives it to the user for ordering. So the same behaviour would be expected for configurations as well.

But before all that, we would need to first define what this capability actually means.

Are we trying to order the configurations of the same type based on some order? What should the scope be for that. Should it be confined to within a <test> or should it be confined to within a <suite>.

What should the behaviour be when this ordering is coupled with hard dependencies (dependsOnGroups and dependsOnMethods)

martinaldrin commented 3 years ago

Hi @krmahadevan , thanks for the reply

@martinaldrin To be honest, I dont think there are a lot of users who establish dependency amongst configuration methods. In the close to a decade of being part of TestNG and responding to different queries on different forums (Github, LinkedIn, Stackoverflow, Google forums) I have seen people ask about dependencies between tests, but dependency between configurations is something that i have not heard of. Also this solution/alternative of users resorting alphabetising their configurations in order to get the order right is not documented anywhere to the best of my knowledge and I have not seen too many users who dig into the code to that level to find out this information.

We use configuration methods allot, so for us it is important. No we have not digging in to the code to find that out the sorting mechanism, that was just something we figured out. I cloned TestNg for the first time 3 weeks ago, before I have just been a regular user.

I second what @juherr said that this feature of adding a priority to configuration methods is not something that is going to be easy to solve.

Maybe I don't have the full picture, but I got exactly the behavior I expected by adding priority for the configuration methods. Ordering and everything is already handled by TestNg, this was just a feature that I enabled. So no code needs to be changed due to that.

Today TestNG has a notion of method interceptors which are basically invoked by TestNG wherein it allows the testng user to order the tests to whatever order they want. Maybe we can consider building a method interceptor for configurations as well, wherein we just let the testng user tell us what is the order in which they would like to run the test. That way, it needn't be just priorities or alphabetical ordering by names. It could be anything. That functionality would ensure that the customisation or configuration method ordering is externalised with minimal changes. The test method interceptor AFAIK only extracts out independent methods and gives it to the user for ordering. So the same behaviour would be expected for configurations as well.

Yes, I know there is other ways to solve this, like intercept the methods in the order we want to run them. But that is much more complicated way of doing it, And most of our users just prefer the standard way using TestNg. We have also already investigated if it would be possible to implement this in our Framework in a easy way by adding a Priority annotation, but our conclusion was that it got to complicated to do it in our Framework and the best would be to get the support in TestNg.

But before all that, we would need to first define what this capability actually means.

Are we trying to order the configurations of the same type based on some order? What should the scope be for that. Should it be confined to within a <test> or should it be confined to within a <suite>.

Before Suite is on level, and the other methods are on level, no changes should be made

What should the behaviour be when this ordering is coupled with hard dependencies (dependsOnGroups and dependsOnMethods)

In my view, everything should work as it does with the dependsOnGroup/Methods, should not be any difference. The only thing that priority will do is to sort the method in a different order compared to the default alphabetic order. Which enable users to control the order in a very easy way with standard api:s. The concept of dependsOnGroup/Method already exist and works perfect together with priority on Test Methods, And I showed that also in my PR #2664

I'm not sure I fully understand your concerns her, since this is just a feature that allows to sort the methods in a different way. Support already exist in TestNg, It will not interfere with dependsOnGroups/Methods, since dependsOn have higher priority than a user defined priority.

But I think we first need to focus on restoring #2664 since is a blocker for us for uplifting to TestNg and it is a blocker for this PR as well.

krmahadevan commented 3 years ago

@martinaldrin

We use configuration methods allot, so for us it is important. No we have not digging in to the code to find that out the sorting mechanism, that was just something we figured out. I cloned TestNg for the first time 3 weeks ago, before I have just been a regular user.

Sure. Not discounting the fact that this feature is important to you folks. This was more of a general observation based on what I have seen as user questions/issues etc., over the period of time.

Maybe I don't have the full picture,

What me and @juherr are trying to state here is that, we dont have a historic understanding of what this functionality is supposed to do. So we would need to ensure that while we enable this feature, we also don't break any of the existing tests and to be honest I dont know what all other functionalities/deviations exist with this feature that can break and can surface just like it surfaced with you folks.

Yes, I know there is other ways to solve this, like intercept the methods in the order we want to run them. But that is much more complicated way of doing it, And most of our users just prefer the standard way using TestNg. We have also already investigated if it would be possible to implement this in our Framework in a easy way by adding a Priority annotation, but our conclusion was that it got to complicated to do it in our Framework and the best would be to get the support in TestNg.

To be honest there's no complications. Once we have an interception mechanism for configuration methods, we can basically expose call backs at the following levels (which testng users will consume via listeners)

Now within the listener, you would just need to find out how you would want to order the methods. It could be via a custom configuration such as @Priority which in your test project you can define and based on that, sort the methods and give back the result back to TestNG.

I personally feel that this is a lesser invasive change into TestNG wherein we aren't adding anything extra, but just exposing the capability of defining the order of configuration methods.

The one pitfall with this approach would that a user would be able to order ONLY independent methods and not the ones that have the below attributes:

I'm not sure I fully understand your concerns here.

Since this is a lesser documented feature when compared to all the other functionalities of TestNG, I am just concerned that we may perhaps not have enough tests that will vet out all of the expected behaviours so that at-least now we would have a benchmark of what the functionality works like. I am saying this more from a maintainer's perspective.

If you could help add as many tests as possible which can cover all the possibilities of this functionality (with and without priorities), that would be a great help here.

I hope that adds clarity to where I am coming from.

martinaldrin commented 3 years ago

I personally feel that this is a lesser invasive change into TestNG wherein we aren't adding anything extra, but just exposing the capability of defining the order of configuration methods.

Now this PR also contains changes that is required for #2664, This was needed so I was able to write unit tests in expected order. Lets focus on #2664 first and then we can continue with this discussion after that.