From 58983936927fbf1c99d655e31e81e569b218ed9c Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Mon, 22 Jun 2026 12:37:32 -0400 Subject: [PATCH 1/6] Address Pending Comments --- src/core/src/bootstrap/EnvLayer.py | 4 +- .../package_managers/Dnf5PackageManager.py | 4 +- src/core/tests/Test_EnvLayer.py | 47 +++++++++++++++---- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index 4c01ea12..0c842b45 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -99,8 +99,8 @@ def get_package_manager(self): # Check for Azure Linux 4 or Above( uses dnf5) if self.is_distro_azure_linux_4(str(os_name)): - code, out = self.run_command_output('which dnf', False, False) - if code == 0: + code, out = self.run_command_output('dnf --version', False, False) + if code == 0 and out and str(out).strip().startswith('5'): return Constants.DNF5 else: print("Error: Expected package manager dnf5 not found on this Azure Linux4 VM.") diff --git a/src/core/src/package_managers/Dnf5PackageManager.py b/src/core/src/package_managers/Dnf5PackageManager.py index d2942a1c..84d97d27 100644 --- a/src/core/src/package_managers/Dnf5PackageManager.py +++ b/src/core/src/package_managers/Dnf5PackageManager.py @@ -263,9 +263,9 @@ def get_dependent_list(self, packages): code, output = self.env_layer.run_command_output(cmd, False, False) self.composite_logger.log_verbose("[DNF5] Dependency simulation. [Command={0}][Code={1}]".format(cmd, str(code))) if code not in self.dnf5_simulation_valid_exit_codes: - self.composite_logger.log_error("[DNF5] Unexpected failure. [Command={0}][Code={1}][Output={2}]".format(cmd, str(code), output)) + self.composite_logger.log_error("[DNF5] Unexpected failure during dependency simulation. [Command={0}][Code={1}][Output={2}]".format(cmd, str(code), output)) error_msg = "DNF5 dependency simulation failed. Investigate and resolve unexpected return code({0}) from package manager on command: {1} ".format(str(code), cmd) - self.status_handler.add_error_to_status(error_msg, Constants.PatchOperationErrorCodes.DEFAULT_ERROR) + self.status_handler.add_error_to_status(error_msg, Constants.PatchOperationErrorCodes.PACKAGE_MANAGER_FAILURE) raise Exception(error_msg, "[{0}]".format(Constants.ERROR_ADDED_TO_STATUS)) dependencies = self.extract_dependencies(output, packages) diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index cecc0ec8..69b36e87 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -45,6 +45,9 @@ def mock_platform_system_windows(self): def mock_linux_distribution(self): return ['test', 'test', 'test'] + def mock_linux_distribution_to_return_azure_linux_4(self): + return ['Microsoft Azure Linux', '4.0', ''] + def mock_linux_distribution_to_return_azure_linux_3(self): return ['Microsoft Azure Linux', '3.0', ''] @@ -71,6 +74,14 @@ def mock_run_command_for_tdnf(self, cmd, no_output=False, chk_err=False): return 0, '' return -1, '' + def mock_run_command_for_dnf5(self, cmd, no_output=False, chk_err=False): + if cmd.find("dnf --version") > -1: + return 0, '5.2.0' + return -1, '' + + def mock_distro_os_release_attr_return_azure_linux_4(self, attribute): + return '4.0.0' + def mock_distro_os_release_attr_return_azure_linux_3(self, attribute): return '3.0.0' @@ -96,18 +107,20 @@ def test_get_package_manager(self): self.backup_distro_os_release_attr = distro.os_release_attr test_input_output_table = [ - [self.mock_run_command_for_apt, self.mock_linux_distribution, Constants.APT], - [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_3, Constants.TDNF], - [self.mock_run_command_for_yum, self.mock_linux_distribution_to_return_azure_linux_3, str()], # check for Azure Linux machine which does not have tdnf - [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_2, Constants.TDNF], - [self.mock_run_command_for_yum, self.mock_linux_distribution, Constants.YUM], - [self.mock_run_command_for_zypper, self.mock_linux_distribution, Constants.ZYPPER], - [lambda cmd, no_output, chk_err: (-1, ''), self.mock_linux_distribution, str()], # no matches for any package manager + [self.mock_run_command_for_apt, self.mock_linux_distribution, Constants.APT, self.mock_distro_os_release_attr_return_none], + [self.mock_run_command_for_dnf5, self.mock_linux_distribution_to_return_azure_linux_4, Constants.DNF5, self.mock_distro_os_release_attr_return_azure_linux_4], + [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_3, Constants.TDNF, self.mock_distro_os_release_attr_return_azure_linux_3], + [self.mock_run_command_for_yum, self.mock_linux_distribution_to_return_azure_linux_3, str(), self.mock_distro_os_release_attr_return_none], # check for Azure Linux machine which does not have tdnf + [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_2, Constants.TDNF, self.mock_distro_os_release_attr_return_azure_linux_3], + [self.mock_run_command_for_yum, self.mock_linux_distribution, Constants.YUM, self.mock_distro_os_release_attr_return_none], + [self.mock_run_command_for_zypper, self.mock_linux_distribution, Constants.ZYPPER, self.mock_distro_os_release_attr_return_none], + [lambda cmd, no_output, chk_err: (-1, ''), self.mock_linux_distribution, str(), self.mock_distro_os_release_attr_return_none], # no matches for any package manager ] for row in test_input_output_table: self.envlayer.run_command_output = row[0] self.envlayer.platform.linux_distribution = row[1] + distro.os_release_attr = row[3] package_manager = self.envlayer.get_package_manager() self.assertTrue(package_manager is row[2]) @@ -118,6 +131,7 @@ def test_get_package_manager(self): # restore original methods self.envlayer.run_command_output = self.backup_run_command_output self.envlayer.platform.linux_distribution = self.backup_linux_distribution + distro.os_release_attr = self.backup_distro_os_release_attr platform.system = self.backup_platform_system def test_is_distro_azure_linux_3(self): @@ -138,6 +152,23 @@ def test_is_distro_azure_linux_3(self): # restore original methods distro.os_release_attr = self.backup_envlayer_distro_os_release_attr + def test_is_distro_azure_linux_4(self): + self.backup_envlayer_distro_os_release_attr = distro.os_release_attr + + test_input_output_table = [ + [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_azure_linux_4, True], + [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_none, False] + ] + + for row in test_input_output_table: + distro_name = row[0]()[0] # Extract distro name from tuple (first element) + distro.os_release_attr = row[1] + result = self.envlayer.is_distro_azure_linux_4(distro_name) + self.assertEqual(result, row[2]) + + # restore original methods + distro.os_release_attr = self.backup_envlayer_distro_os_release_attr + def test_filesystem(self): # only validates if these invocable without exceptions backup_retry_count = Constants.MAX_FILE_OPERATION_RETRY_COUNT @@ -152,7 +183,7 @@ def test_platform(self): self.envlayer.platform.cpu_arch() self.envlayer.platform.vm_name() - def test_get_package_manager_azure_linux_4_and_rhel10_not_supported(self): + def test_get_package_manager_rhel10_not_supported(self): """Test for RHEL 10 log unsupported message""" self.backup_platform_system = platform.system self.backup_linux_distribution = self.envlayer.platform.linux_distribution From 735c1c969e8b957b30d449d9a80c38baa3f809db Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Mon, 22 Jun 2026 13:06:40 -0400 Subject: [PATCH 2/6] Address Copilot review --- src/core/tests/Test_EnvLayer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index 69b36e87..9417d96a 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -111,7 +111,7 @@ def test_get_package_manager(self): [self.mock_run_command_for_dnf5, self.mock_linux_distribution_to_return_azure_linux_4, Constants.DNF5, self.mock_distro_os_release_attr_return_azure_linux_4], [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_3, Constants.TDNF, self.mock_distro_os_release_attr_return_azure_linux_3], [self.mock_run_command_for_yum, self.mock_linux_distribution_to_return_azure_linux_3, str(), self.mock_distro_os_release_attr_return_none], # check for Azure Linux machine which does not have tdnf - [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_2, Constants.TDNF, self.mock_distro_os_release_attr_return_azure_linux_3], + [self.mock_run_command_for_tdnf, self.mock_linux_distribution_to_return_azure_linux_2, Constants.TDNF, self.mock_distro_os_release_attr_return_azure_linux_2], [self.mock_run_command_for_yum, self.mock_linux_distribution, Constants.YUM, self.mock_distro_os_release_attr_return_none], [self.mock_run_command_for_zypper, self.mock_linux_distribution, Constants.ZYPPER, self.mock_distro_os_release_attr_return_none], [lambda cmd, no_output, chk_err: (-1, ''), self.mock_linux_distribution, str(), self.mock_distro_os_release_attr_return_none], # no matches for any package manager @@ -122,7 +122,7 @@ def test_get_package_manager(self): self.envlayer.platform.linux_distribution = row[1] distro.os_release_attr = row[3] package_manager = self.envlayer.get_package_manager() - self.assertTrue(package_manager is row[2]) + self.assertEqual(package_manager, row[2]) # test for Windows platform.system = self.mock_platform_system_windows From 8adaa58388606af9ec9952f9dbec3d45289ed6f6 Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Mon, 22 Jun 2026 15:15:41 -0400 Subject: [PATCH 3/6] Address Code Review comments #1 --- src/core/src/bootstrap/EnvLayer.py | 14 +++++++++++--- src/core/tests/Test_EnvLayer.py | 20 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index 0c842b45..5699ceaa 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -99,11 +99,19 @@ def get_package_manager(self): # Check for Azure Linux 4 or Above( uses dnf5) if self.is_distro_azure_linux_4(str(os_name)): + code, out = self.run_command_output('which dnf', False, False) + if code != 0: + print("Error: Expected package manager dnf not found on this Azure Linux4 VM.") + return str() + code, out = self.run_command_output('dnf --version', False, False) - if code == 0 and out and str(out).strip().startswith('5'): - return Constants.DNF5 + if code == 0 and out: + #Output : dnf5 version 5.2.18.0 + version = str(out).split()[-1] + if version.startswith('5'): + return Constants.DNF5 else: - print("Error: Expected package manager dnf5 not found on this Azure Linux4 VM.") + print("Error: Expected dnf version 5 not found on this Azure Linux4 VM.") return str() # Check for Azure Linux (3 and below use TDNF) diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index 9417d96a..fd159c62 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -75,8 +75,10 @@ def mock_run_command_for_tdnf(self, cmd, no_output=False, chk_err=False): return -1, '' def mock_run_command_for_dnf5(self, cmd, no_output=False, chk_err=False): - if cmd.find("dnf --version") > -1: - return 0, '5.2.0' + if "which dnf" in cmd: + return 0, '/usr/bin/dnf' + if "dnf --version" in cmd: + return 0, 'dnf5 version 5.2.18.0' return -1, '' def mock_distro_os_release_attr_return_azure_linux_4(self, attribute): @@ -208,6 +210,20 @@ def test_get_package_manager_rhel10_not_supported(self): # restore self.__restore_mocks() + def test_mock_command_fallback_paths(self): + """Test that mock commands return -1 for unexpected commands""" + # Cover line 60: apt mock with unexpected command + code, out = self.mock_run_command_for_apt('which yum') + self.assertEqual(code, -1) + + # Cover line 75: dnf5 mock with unexpected command + code, out = self.mock_run_command_for_dnf5('which not-dnf') + self.assertEqual(code, -1) + + # Cover line 75: dnf5 mock with unexpected command + code, out = self.mock_run_command_for_tdnf('which apt') + self.assertEqual(code, -1) + def __restore_mocks(self): """Restore backed up mocks to their original state""" distro.os_release_attr = self.backup_distro_os_release_attr From 1c9c1fffb859cf7156f7318a14b6497ad4b2c9dd Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Mon, 22 Jun 2026 16:07:00 -0400 Subject: [PATCH 4/6] Address Code Review comments #1 --- src/core/src/bootstrap/EnvLayer.py | 5 ++- src/core/tests/Test_EnvLayer.py | 49 ++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index 5699ceaa..7caa91fc 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -110,9 +110,8 @@ def get_package_manager(self): version = str(out).split()[-1] if version.startswith('5'): return Constants.DNF5 - else: - print("Error: Expected dnf version 5 not found on this Azure Linux4 VM.") - return str() + print("Error: Expected dnf version 5 not found on this Azure Linux4 VM.") + return str() # Check for Azure Linux (3 and below use TDNF) if self.is_distro_azure_linux(str(os_name)): diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index fd159c62..4b9620f6 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -81,6 +81,16 @@ def mock_run_command_for_dnf5(self, cmd, no_output=False, chk_err=False): return 0, 'dnf5 version 5.2.18.0' return -1, '' + def mock_run_command_for_dnf_not_found(self, cmd, no_output=False, chk_err=False): + return -1, '' + + def mock_run_command_for_dnf_wrong_version(self, cmd, no_output=False, chk_err=False): + if "which dnf" in cmd: + return 0, '/usr/bin/dnf' + if "dnf --version" in cmd: + return 0, 'dnf version 4.14.0' + return -1, '' + def mock_distro_os_release_attr_return_azure_linux_4(self, attribute): return '4.0.0' @@ -159,7 +169,7 @@ def test_is_distro_azure_linux_4(self): test_input_output_table = [ [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_azure_linux_4, True], - [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_none, False] + [self.mock_linux_distribution_to_return_azure_linux_4, self.mock_distro_os_release_attr_return_none, False], ] for row in test_input_output_table: @@ -171,6 +181,33 @@ def test_is_distro_azure_linux_4(self): # restore original methods distro.os_release_attr = self.backup_envlayer_distro_os_release_attr + def test_get_package_manager_dnf5_error_cases(self): + """Test dnf5 error cases in get_package_manager""" + self.backup_platform_system = platform.system + self.backup_linux_distribution = self.envlayer.platform.linux_distribution + self.backup_run_command_output = self.envlayer.run_command_output + self.backup_distro_os_release_attr = distro.os_release_attr + + platform.system = self.mock_platform_system + self.envlayer.platform.linux_distribution = self.mock_linux_distribution_to_return_azure_linux_4 + distro.os_release_attr = self.mock_distro_os_release_attr_return_azure_linux_4 + + test_input_output_table = [ + [self.mock_run_command_for_dnf_not_found, str()], + [self.mock_run_command_for_dnf_wrong_version, str()], + ] + + for row in test_input_output_table: + self.envlayer.run_command_output = row[0] + result = self.envlayer.get_package_manager() + self.assertEqual(result, row[1]) + + # restore original methods + self.envlayer.run_command_output = self.backup_run_command_output + self.envlayer.platform.linux_distribution = self.backup_linux_distribution + distro.os_release_attr = self.backup_distro_os_release_attr + platform.system = self.backup_platform_system + def test_filesystem(self): # only validates if these invocable without exceptions backup_retry_count = Constants.MAX_FILE_OPERATION_RETRY_COUNT @@ -212,16 +249,16 @@ def test_get_package_manager_rhel10_not_supported(self): def test_mock_command_fallback_paths(self): """Test that mock commands return -1 for unexpected commands""" - # Cover line 60: apt mock with unexpected command - code, out = self.mock_run_command_for_apt('which yum') + code, out = self.mock_run_command_for_apt('which apt') self.assertEqual(code, -1) - # Cover line 75: dnf5 mock with unexpected command code, out = self.mock_run_command_for_dnf5('which not-dnf') self.assertEqual(code, -1) - # Cover line 75: dnf5 mock with unexpected command - code, out = self.mock_run_command_for_tdnf('which apt') + code, out = self.mock_run_command_for_dnf_wrong_version('dnf --v') + self.assertEqual(code, -1) + + code, out = self.mock_run_command_for_tdnf('which not-tdnf') self.assertEqual(code, -1) def __restore_mocks(self): From e8dd90b91d7622578c36f5119f3f238e9522f78e Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Tue, 23 Jun 2026 10:13:00 -0400 Subject: [PATCH 5/6] Address Code Review comment #2 --- src/core/src/bootstrap/EnvLayer.py | 4 +++- src/core/tests/Test_EnvLayer.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index 7caa91fc..e0c5e77b 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -110,7 +110,9 @@ def get_package_manager(self): version = str(out).split()[-1] if version.startswith('5'): return Constants.DNF5 - print("Error: Expected dnf version 5 not found on this Azure Linux4 VM.") + print("Error: Expected dnf version 5 on this Azure Linux4 VM. Found: {0}".format(version)) + return str() + print("Error: Unable to determine dnf version. Code={0}, Output={1}".format(code, out)) return str() # Check for Azure Linux (3 and below use TDNF) diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index 4b9620f6..b130a95d 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -17,6 +17,7 @@ import platform import sys import unittest + # Conditional import for StringIO try: from StringIO import StringIO # Python 2 @@ -91,6 +92,13 @@ def mock_run_command_for_dnf_wrong_version(self, cmd, no_output=False, chk_err=F return 0, 'dnf version 4.14.0' return -1, '' + def mock_run_command_for_dnf_version_command_failure(self, cmd, no_output=False, chk_err=False): + if "which dnf" in cmd: + return 0, '/usr/bin/dnf' + if "dnf --version" in cmd: + return -1, 'dnf version command failure' + return -1, '' + def mock_distro_os_release_attr_return_azure_linux_4(self, attribute): return '4.0.0' @@ -195,6 +203,7 @@ def test_get_package_manager_dnf5_error_cases(self): test_input_output_table = [ [self.mock_run_command_for_dnf_not_found, str()], [self.mock_run_command_for_dnf_wrong_version, str()], + [self.mock_run_command_for_dnf_version_command_failure, str()] ] for row in test_input_output_table: @@ -258,6 +267,9 @@ def test_mock_command_fallback_paths(self): code, out = self.mock_run_command_for_dnf_wrong_version('dnf --v') self.assertEqual(code, -1) + code, out = self.mock_run_command_for_dnf_version_command_failure('dnf --v') + self.assertEqual(code, -1) + code, out = self.mock_run_command_for_tdnf('which not-tdnf') self.assertEqual(code, -1) From 3f553637a2443a49057e5eaffebafd8f45dfbd5a Mon Sep 17 00:00:00 2001 From: Yashna Parikh Date: Tue, 23 Jun 2026 10:37:10 -0400 Subject: [PATCH 6/6] Separate out methods --- src/core/src/bootstrap/EnvLayer.py | 21 +++++++++++++++------ src/core/tests/Test_EnvLayer.py | 1 - 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index e0c5e77b..e63f1994 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -81,6 +81,18 @@ def is_distro_rhel_10(self, distro_name): """ Checks if the current distro is RHEL 10 """ return self.__is_matching_distro_and_version(distro_name, Constants.RED_HAT, version_to_match=10) + def __is_dnf_available(self): + code, _ = self.run_command_output('which dnf', False, False) + return code == 0 + + def __get_dnf_version(self): + code, out = self.run_command_output('dnf --version', False, False) + # Output : dnf5 version 5.2.18.0 + if code != 0 or not out: + return code, out, None + version = str(out).split()[-1] + return code, out, version + def get_package_manager(self): # type: () -> str """ Detects package manager type """ @@ -99,15 +111,12 @@ def get_package_manager(self): # Check for Azure Linux 4 or Above( uses dnf5) if self.is_distro_azure_linux_4(str(os_name)): - code, out = self.run_command_output('which dnf', False, False) - if code != 0: + if not self.__is_dnf_available(): print("Error: Expected package manager dnf not found on this Azure Linux4 VM.") return str() - code, out = self.run_command_output('dnf --version', False, False) - if code == 0 and out: - #Output : dnf5 version 5.2.18.0 - version = str(out).split()[-1] + code, out, version = self.__get_dnf_version() + if version: if version.startswith('5'): return Constants.DNF5 print("Error: Expected dnf version 5 on this Azure Linux4 VM. Found: {0}".format(version)) diff --git a/src/core/tests/Test_EnvLayer.py b/src/core/tests/Test_EnvLayer.py index b130a95d..36140c95 100644 --- a/src/core/tests/Test_EnvLayer.py +++ b/src/core/tests/Test_EnvLayer.py @@ -17,7 +17,6 @@ import platform import sys import unittest - # Conditional import for StringIO try: from StringIO import StringIO # Python 2