Tests: Add a skip on version function #6844
Conversation
The idea is to have a unified fuction that can skip tests based on valid versions of package. Lists of minimal and maximal versions can be provided along with a package name (default sssd) and requested action on missing package. It does not try to account for differences between distrubutions.
jakub-vavra-cz
force-pushed
the
skip_sssd_ver
branch
from
3b65725 to
07b9adc
Compare
| If multiple min versions are defined it is expected that the fix was introduced | ||
| to different branches at once and the comparison will compare to multiple min versions. | ||
| For version comparison purposes <major>.<minor> is considered one branch. | ||
| For branches where min version is not defined, but they fit between other min versions it is |
There was a problem hiding this comment.
Can you elaborate here about "branches where min version is not defined, but they fit between other min versions"?
Does that mean branch names or tests using this function? A specific example would be helpful I think.
There was a problem hiding this comment.
Let me explain on example:
Lets say that some bug was fixed in sssd 2.9.2 but fix was backported to 2.7 branch as well in 2.7.4-5.
min_versions=["2.7.4-5", "2.9.2"]
Lets say that it finds installed sssd 2.9.1.
It fits sssd-2-9 (<major>.<minor>) so the detected version is not compared to any other min_version and as it is lower that this mine version test will be skipped.
Lets say that test finds 2.8.3 as installed sssd.
We do not have defined any min version for 2.8 branch and 2.8.3 is higher than 2.7.4-5 so it will consider the test valid there and execute. If we wanted to skip 2.8 we would need define min_versions=["2.7.4-5", "2.8.999" , "2.9.2"].
The idea is that we can track minimal (package) version across multiple branches as the same fix could have been backported to different rhel versions. The max versions are similar concept for tests that will no longer by meaningful after some sssd change.
If we add this info to newly introduced tests we can just execute master.
There was a problem hiding this comment.
First of all, let me apologize for the long delayed response here.
Let me explain on example: Lets say that some bug was fixed in sssd 2.9.2 but fix was backported to 2.7 branch as well in 2.7.4-5. min_versions=["2.7.4-5", "2.9.2"] Lets say that it finds installed sssd 2.9.1. It fits sssd-2-9 (<major>.<minor>) so the detected version is not compared to any other min_version and as it is lower that this mine version test will be skipped. Lets say that test finds 2.8.3 as installed sssd. We do not have defined any min version for 2.8 branch and 2.8.3 is higher than 2.7.4-5 so it will consider the test valid there and execute. If we wanted to skip 2.8 we would need define min_versions=["2.7.4-5", "2.8.999" , "2.9.2"].
Thanks for the explanation. It makes more sense to me now but, still seems unintuitive (to me). I suppose you can't rely on adding every maj.min version where it should run because we'd have to add to this every time we release a new maj.min branch.
I'd suggest two things:
- Try to reword:
For branches where min version is not defined, but they fit between other min versions
to something like:
If run for a case where the SSSD <major>.<minor> version is not defined, but is between the listed versions
- Add an example like you listed above in the comments
The idea is that we can track minimal (package) version across multiple branches as the same fix could have been backported to different rhel versions. The max versions are similar concept for tests that will no longer by meaningful after some sssd change.
If we add this info to newly introduced tests we can just execute master.
I thought we agreed to include tests in the relevant branches alongside the features/fixes and where ever those are backported. This sounds like we might not be doing that. Would this be more for identifying tests within a - branch where they could have been added much later than the feature/fix?
| @@ -3448,7 +3448,8 @@ def test_0043_sssd_not_using_given_krb_port( | |||
| 3. Logs contain info about right port being used | |||
| Logs do not contain wrong (default) port being used | |||
| """ | |||
|
|
|||
| if sssdTools.skip_package_version(multihost.client[0], min_versions=["2.6.2-2"]): | |||
| pytest.skip("Incompatible version.") | |||
There was a problem hiding this comment.
Why not just build the pytest.skip() into the skip_package_version method? Then you can drop the if and skip.
There was a problem hiding this comment.
I did try that but running skip outside of the test resulted in missing context and the test was not marked properly as skipped. I would use skipif but I did not figure a clean way of passing session_multihost to the function.
There was a problem hiding this comment.
@spoore1 If the way of selection the valid version works I will probably create a session level fixture that will pull the package versions from client to make them available globally and then I can have skipif helper function that will not need multihost fixture.
There was a problem hiding this comment.
That sounds like it'd be useful if we're going to use this more widely. Not sure if you want to include it with this PR or as a separate effort.
jakub-vavra-cz
force-pushed
the
skip_sssd_ver
branch
from
41e047d to
4407855
Compare
| If multiple min versions are defined it is expected that the fix was introduced | ||
| to different branches at once and the comparison will compare to multiple min versions. | ||
| For version comparison purposes <major>.<minor> is considered one branch. | ||
| For branches where min version is not defined, but they fit between other min versions it is |
There was a problem hiding this comment.
First of all, let me apologize for the long delayed response here.
Let me explain on example: Lets say that some bug was fixed in sssd 2.9.2 but fix was backported to 2.7 branch as well in 2.7.4-5. min_versions=["2.7.4-5", "2.9.2"] Lets say that it finds installed sssd 2.9.1. It fits sssd-2-9 (<major>.<minor>) so the detected version is not compared to any other min_version and as it is lower that this mine version test will be skipped. Lets say that test finds 2.8.3 as installed sssd. We do not have defined any min version for 2.8 branch and 2.8.3 is higher than 2.7.4-5 so it will consider the test valid there and execute. If we wanted to skip 2.8 we would need define min_versions=["2.7.4-5", "2.8.999" , "2.9.2"].
Thanks for the explanation. It makes more sense to me now but, still seems unintuitive (to me). I suppose you can't rely on adding every maj.min version where it should run because we'd have to add to this every time we release a new maj.min branch.
I'd suggest two things:
- Try to reword:
For branches where min version is not defined, but they fit between other min versions
to something like:
If run for a case where the SSSD <major>.<minor> version is not defined, but is between the listed versions
- Add an example like you listed above in the comments
The idea is that we can track minimal (package) version across multiple branches as the same fix could have been backported to different rhel versions. The max versions are similar concept for tests that will no longer by meaningful after some sssd change.
If we add this info to newly introduced tests we can just execute master.
I thought we agreed to include tests in the relevant branches alongside the features/fixes and where ever those are backported. This sounds like we might not be doing that. Would this be more for identifying tests within a - branch where they could have been added much later than the feature/fix?
| @@ -3448,7 +3448,8 @@ def test_0043_sssd_not_using_given_krb_port( | |||
| 3. Logs contain info about right port being used | |||
| Logs do not contain wrong (default) port being used | |||
| """ | |||
|
|
|||
| if sssdTools.skip_package_version(multihost.client[0], min_versions=["2.6.2-2"]): | |||
| pytest.skip("Incompatible version.") | |||
There was a problem hiding this comment.
That sounds like it'd be useful if we're going to use this more widely. Not sure if you want to include it with this PR or as a separate effort.
|
This whole topic will be handled in different way by:
|
The idea is to have a unified fuction that can skip tests based on valid versions of package.
Lists of minimal and maximal versions can be provided along with a package name (default sssd) and requested action on missing package. It does not try to account for differences between distrubutions.