Fix apt unit tests #6029
Fix apt unit tests #6029
Conversation
This fixes an issue with the Debian version information where it was retrieving a number instead of the codename. This also fixes an issue where the version of Ubuntu was incomplete and it was retrieving only the first part of it, rather than the full version. Signed-off-by: David Negreira <[email protected]>
The unit test was failing on recent versions of Debian and Ubuntu because the `login` package is no longer installing the binary on `/bin/login` but rather on `/usr/bin/login`. We have added a function which will retrieve the binary path depending on the distro as well. Resolves: avocado-framework#6028 Signed-off-by: David Negreira <[email protected]>
| @@ -387,7 +387,7 @@ class DebianProbe(Probe): | |||
|
|
|||
| CHECK_FILE = "/etc/debian_version" | |||
| CHECK_FILE_DISTRO_NAME = "debian" | |||
| CHECK_VERSION_REGEX = re.compile(r"(\d+)\.(\d+)") | |||
There was a problem hiding this comment.
I've noticed that, for Debian stable, /etc/debian_version contains a numeric value, such as 12.7. On testing, because there is no numeric version number still determined, it contains trixie/sid.
I'm not sure it's a good idea to default to using code names instead of code names all over. It may be a requirement for testing, but, numeric has advantages such as being able to do comparisons. Or maybe I'm just not aware that Debian mostly disregards numeric versions.
My perception is that the best solution would be one where numeric values are given if available, and if not, code names as used as fallbacks.
There was a problem hiding this comment.
@clebergnu - you are correct here, we can indeed verify if we have /sid in /etc/debian_version or a float value to be able to do the comparison values here.
I need some guidance to achieve this since I don't want to break other things and have a predictable behaviour, or at least you know about it :-)
In your opinion, how do you see this being achieved so that we don't break other things in avocado? Would it be ok to just retrieve the string "sid" OR the full float (example in debian11 11.11 and debian12 12.7) by using an OR on the regex, which would mean we would either return a string or a float. I can verify that on the unit tests we retrieve either of them accordingly, but down the line I am not sure if you want to deal with it like that or other way.
An example of the regex I am thinking on having here
((.+)/(.+)|(\d+.\d+)) - would this approach be ok with you?
My concern is that we are either returning a string or a float here.
There was a problem hiding this comment.
quick update that I have tried the above regexp I have proposed, and the unit tests below get in a stuck state, not able to further debug, not sure why, but seems that the solution of "ORing" in a regexp is not a good one.
unit-644-selftests/unit/utils/software_manager.py:Dpkg.test_is_valid: INTERRUPTED
unit-645-selftests/unit/utils/software_manager.py:Dpkg.test_is_not_valid: INTERRUPTED
| @unittest.skipUnless(os.getuid() == 0, "This test requires root privileges") | ||
| @unittest.skipUnless(apt_supported_distro(), "Unsupported distro") | ||
| class Apt(unittest.TestCase): | ||
| def test_provides(self): | ||
| sm = manager.SoftwareManager() | ||
| self.assertEqual(sm.provides("/bin/login"), "login") | ||
| distro_name = distro.detect().name |
There was a problem hiding this comment.
And optimization here to avoid calling the detection twice:
detected = distro.detect()
login_path = login_binary_path(detected.name, detected.version)Also, it would be OK to change login_binary_path to take a Distro instance IMO.
| if float(distro_version) >= 24.04: | ||
| return "/usr/bin/login" | ||
| if distro_name == "debian": | ||
| if distro_version == "trixie": |
There was a problem hiding this comment.
This ties into my previous point. It's very probably that unstable also has the file at the same place as testing, right? In the future, when those are released, it'd make sense to do a numerical version comparison IMO.
This is some of the work in preparation for the debian packaging on #5912.
I have separated the fixes in two commits, let me know if this is ok.
On a44d5ab I have fixed the Ubuntu/debian version detection as it was not retrieving the correct versions.
On eff2add I have fixed the unit test by adding a function to retrieve the binary based on the distro version.
Happy to hear any feedback or improvements.
Resolves: #6028