action #23546
closed
[sle][functional] Improve sle_version_at_least, e.g. to consider CaasP versions, behave comparable to leap_version_at_least, etc.
Added by riafarov over 6 years ago.
Updated about 6 years ago.
Category:
Enhancement to existing tests
Description
Comment from PR (https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/3451):
@rwx788 like I said..it's important we get CaaSP honoring the sle_version_at_least
CaaSP 1.0 was mostly 12-SP2 with some 12-SP3, so the behaviour of at_least 12-SP2 would likely have worked
CaasP 2.0 will be 12-SP3 - we need it using the sle_version_at_least 12-SP3 behaviour
CaaSP 4.0 will be 15, which is why I think this is_sle approach is not acceptable - it will just break again in 5 months from now.
CaaSP is a sle distri, so we need to treat it as such
Problem (or confusing part is) sle_version_at_least returns true if VERSION value is anything which doesn't match SLE version naming convention. This is used in many places in TW, so we should approach it and make it more clear. E.g. leap_version_at_least returns false in case of non leap distri. This is inconsistent.
- Target version set to Milestone 15
- Subject changed from [sle][functional] Improve sle_version_at_least to consider CaasP versions to [sle][functional] Improve sle_version_at_least, e.g. to consider CaasP versions, behave comparable to leap_version_at_least, etc.
I don't think you need to consider CaaSP here, we use separate is_caasp sub.
@mkravec That's the problem because when we call sle_version_at_least we have to additionally call is_sle or is_caasp, which is confusing as method returns true for other distris. And that caused couple of regressions as well, as if you read function name, you don't expect that it will return true for openSUSE, for example.
I totally agree here - it's confusing and I also don't like it. Just wanted to say that you don't need to fix that for CaaSP.
sle_version_at_least is used ~200 times, so it will be donkey work to change that :)
@rwx788 - do you think we can close this? Now that we have following version_utils functionality I don't see what else can we do here.
is_sle '>=15'
is_caasp '>=4.0'
I guess what we should do is get rid of all occurences of sle_version_at_least and leap_version_at_least using the new, nicer version functions you provided.
- Status changed from New to Resolved
@okurz, I agree with your statement, but I guess it's fine to handle it in another subticket. As stated above, we cannot simply remove all occurrences. So I would create a new ticket for that with more specific details and resolve this one, as we have more or less an idea only. Feel free to reopen and convert it to epic.
@mkravec, as stated above, let's now do proper review to avoid new usages and request changes to old code when related part is touched.
Also available in: Atom
PDF