Making norun_tests env_types combinations robust #205
Conversation
|
Now, the script parses all these sections in no_run_tests.conf: |
narasimhan-v
force-pushed
the
no_run_tests_robust
branch
from
45d28c9 to
fa49851
Compare
| minor = env_ver | ||
| perm = [] | ||
| env_list = [env_type, dist, major, minor] | ||
| for length in range(1, len(env_list) + 1): |
There was a problem hiding this comment.
Considering we have all combinations covered at length 2, do we need a length of 3 and 4? Just a thought that we can cover what we need at length 1 and 2.
norun_NV
norun_rhel
norun_rhel8
norun_rhel8.1
norun_NV_rhel
norun_NV_rhel8
norun_NV_rhel8.1
norun_rhel_NV
norun_rhel_rhel8
norun_rhel_rhel8.1
norun_rhel8_NV
norun_rhel8_rhel
norun_rhel8_rhel8.1
norun_rhel8.1_NV
norun_rhel8.1_rhel
norun_rhel8.1_rhel8
It also matches what we have in the config now. Config would be better to maintain this way.
Possibly we can extend the length when needed, but I doubt if we would need to.
What are your thoughts here?
There was a problem hiding this comment.
Possibly we can extend the length when needed, but I doubt if we would need to.
You are right in this case alone. Because we have rhel, rhel8, rhel8.1 as different entries, we have this luxury to cover all needed combinations. And since we are not doing too much extra work to generate those extra permutations, I have included all possible ones.
It also matches what we have in the config now. Config would be better to maintain this way.
We are not going to touch the config. Just that the current config limits users in certain ways (like not able to cover certain permutations), which this code the future config can have more entries, but this in no way forces changes or limitations in current config.
There was a problem hiding this comment.
We are not going to touch the config. Just that the current config limits users in certain ways (like not able to cover certain permutations), which this code the future config can have more entries, but this in no way forces changes or limitations in current config.
My only worry was not to make the config clumsy. Because when we have various ways to use the config we may tend to use different section name for same type of use case.
Ex.
norun_rhel8.1_NVnorun_rhel_rhel8_rhel8.2_NV
But yes of course it does not harm anything. So it is your call if we can go with current implementation.
There was a problem hiding this comment.
I get your point. My goal was to make it easier for user to go with any order they want, without knowing what order the code computes (this should ideally be the case as well).
- Does it make the config file clumsy in future - Yes
2a. Does it cover all possible permutations - Yes
2b. Does it make the user's job easier to create whatever permutation they want to - Yes
For me, advantages of 2a and 2b weigh in more than the clumsiness, and the disadvantages of that clumsiness is something I don't mind :)
On a side note, I was even planning to remove the norun_ from the config section names, since that is irrelevant.
There was a problem hiding this comment.
@narasimhan-v one of the concern that I had is already getting discussed here, I agree with @harish-24, we do not need more depth combinations, rather let's try to have layered, but that is not possible with current implementation or topic for current one, let's have another discussion[1].
I would say it is better not to confuse user, and we need to have some rules for user so the documentation, I do NOT agree on what ever user give we should should parse, probably we can add the example for valid patterns that user can provide.
Some tests have the potential of causing system crashes / hangs. Such test in cfg file hinders the run of tests that follow.
So, if such tests are identified and need to be "not run" for a particular environment (Since they can run fine on others),
we have a provision to mention such tests to not run.
Please have a look at "config/wrapper/no_run_tests.conf"
how about make it generic, I guess it is already we use it but not in the order, probably that can be corrected, which would be cleaner addition for user, what say?
Platform:
Platform_Distro:
Platform_Distro_MajorNo:
Platform_Distro_MajorNo_MinorNo:
example for each :
Platform: KVM, NV, PHYP
Distro: rhel, sles, ubuntu
MajorNo: 8, 11, 14
MinorNo: 2, sp4, 04
Additionally, what is the issue with current implementation and why we need this change is not clear from commit message, for me it looks like we are trying to ease user for his wrongly configured environment, which might lead other failures going forward.
[1]: How about having a yaml implementation of norun config, so that we can approach it as
layered, am not big user/fan of yaml, I know you have been using in avocado misc tests, and i feel it would suite this need for long run and we can avoid duplication, this can wait and we can discuss more on such design.
There was a problem hiding this comment.
Additionally, what is the issue with current implementation and why we need this change is not clear from commit message.
To start with, we are not allowing users to provide norun tests for these type of combinations now.
norun_phyp
norun_rhel_phyp
Another issue we will face is, we are not at all looking at processor generation and system type today (P8 vs P9, OpenPower vs IBM), and these are valid use cases where some people do not want a test to run on certain systems where it is harmful.
Forcing users to go with a particular order is not intuitive. I, as a user, will feel norun_P9_NV_... is right, and someone else might feel norun_NV_P9 is right. And we should not be spending time on defining the right order, IMO.
How about having a yaml implementation of norun config
I like this idea for all the reasons above, and will explore more on this.
There was a problem hiding this comment.
Additionally, what is the issue with current implementation and why we need this change is not clear from commit message.
To start with, we are not allowing users to provide norun tests for these type of combinations now.
norun_phyp norun_rhel_phypAnother issue we will face is, we are not at all looking at processor generation and system type today (P8 vs P9, OpenPower vs IBM), and these are valid use cases where some people do not want a test to run on certain systems where it is harmful.
Forcing users to go with a particular order is not intuitive. I, as a user, will feel norun_P9_NV_... is right, and someone else might feel norun_NV_P9 is right. And we should not be spending time on defining the right order, IMO.
Right order helps to avoid confusion, moreover I feel, there is no liking of user needed here, this is to get some job done for user, and rules are always good to keep the coding/processing sane.
More over it will help user identifying if the pattern is already defined, I feel that is more of a usability we need to provide user over any pattern.
Good, we need Processor Generation too, that's where defined pattern would help us.
Small background on KVM test suite: I had a similar need on KVM tests and that resulted in almost 6 months refactoring cpu utils code on avocado but it is cleaner now, the actual change I wanted to make it is still in-review, there we have a Cartesian config which came handy and test implementation there would support such change, similar approach would be good conceptually, I believe yaml will help us.
https://github.com/avocado-framework/avocado-vt/pull/2225
https://github.com/autotest/tp-libvirt/pull/2301
How about having a yaml implementation of norun config
I like this idea for all the reasons above, and will explore more on this.
Good, pls do, this might ease lot of things we discuss already.
My suggestion would be to park this PR for any rework, and explore on yaml way of doing....
Earlier, we were looking for only specific permutations of norun_tests combinations. Now, it is more robust by using permutations. Signed-off-by: Narasimhan V <[email protected]>
narasimhan-v
force-pushed
the
no_run_tests_robust
branch
from
8e08825 to
4edf850
Compare
PraveenPenguin
force-pushed
the
master
branch
from
3e86382 to
11588e0
Compare
Earlier, we were looking for only specific permutations of
norun_tests combinations. Now, it is more robust by using
permutations.
Signed-off-by: Narasimhan V [email protected]