Move setup_convert2rhel call inside install_or_update function (#68)

Signed-off-by: Andrea Waltlova <awaltlov@redhat.com>

scripts/c2r_script.py

@@ -450,7 +450,7 @@ def _check_if_package_installed(pkg_name):
     return return_code == 0
 
 
-def install_convert2rhel():
+def install_or_update_convert2rhel(required_files):
     """
     Install the convert2rhel tool to the system.
     Returns True and transaction ID if the c2r pkg was installed, otherwise False, None.
@@ -461,6 +461,7 @@ def install_convert2rhel():
     c2r_installed = _check_if_package_installed(c2r_pkg_name)
 
     if not c2r_installed:
+        setup_convert2rhel(required_files)
         output, returncode = run_subprocess(
             ["/usr/bin/yum", "install", c2r_pkg_name, "-y"],
         )
@@ -690,9 +691,10 @@ def main():
             archive_analysis_report(C2R_REPORT_TXT_FILE)
 
         # Setup Convert2RHEL to be executed.
-        setup_convert2rhel(required_files)
         do_cleanup = True
-        convert2rhel_installed, transaction_id = install_convert2rhel()
+        convert2rhel_installed, transaction_id = install_or_update_convert2rhel(
+            required_files
+        )
         if convert2rhel_installed:
             YUM_TRANSACTIONS_TO_UNDO.add(transaction_id)
 
@@ -781,7 +783,7 @@ def main():
                 gpg_key_file.keep = True
 
                 # NOTE: When c2r statistics on insights are not reliant on rpm being installed
-                # remove below line (=decide only based on install_convert2rhel() result)
+                # remove below line (=decide only based on install_or_update_convert2rhel() result)
                 if convert2rhel_installed:
                     YUM_TRANSACTIONS_TO_UNDO.remove(transaction_id)
 

tests/test_install.py

@@ -2,7 +2,7 @@
 from mock import patch, call
 
 from scripts.c2r_script import (
-    install_convert2rhel,
+    install_or_update_convert2rhel,
     ProcessError,
 )
 
@@ -14,7 +14,7 @@
         ((b"output", 0), True, False),
     ),
 )
-def test_install_convert2rhel(
+def test_install_or_update_convert2rhel(
     subprocess_mock, pkg_installed_mock, should_undo_transaction
 ):
     with patch(
@@ -26,33 +26,41 @@ def test_install_convert2rhel(
             return_value=pkg_installed_mock,
         ) as mock_run_pkg_check:
             with patch(
-                "scripts.c2r_script._get_last_yum_transaction_id", return_value=1
-            ) as mock_transaction_get:
-                should_undo, _ = install_convert2rhel()
+                "scripts.c2r_script.setup_convert2rhel",
+                return_value=pkg_installed_mock,
+            ) as mock_download_files:
+                with patch(
+                    "scripts.c2r_script._get_last_yum_transaction_id", return_value=1
+                ) as mock_transaction_get:
+                    should_undo, _ = install_or_update_convert2rhel([])
 
     assert should_undo is should_undo_transaction
     mock_run_pkg_check.assert_called_once()
     mock_run_subprocess.assert_called_once()
     assert mock_transaction_get.call_count == (0 if pkg_installed_mock else 1)
 
     if pkg_installed_mock:
+        mock_download_files.assert_not_called()
         expected_calls = [
             ["/usr/bin/yum", "update", "convert2rhel", "-y"],
         ]
     else:
+        mock_download_files.assert_called_once()
         expected_calls = [["/usr/bin/yum", "install", "convert2rhel", "-y"]]
 
     assert mock_run_subprocess.call_args_list == [call(args) for args in expected_calls]
 
 
 @patch("scripts.c2r_script._check_if_package_installed", return_value=False)
 @patch("scripts.c2r_script.run_subprocess", return_value=(b"failed", 1))
-def test_install_convert2rhel_raise_exception(mock_run_subprocess, mock_pkg_check):
+def test_install_or_update_convert2rhel_raise_exception(
+    mock_run_subprocess, mock_pkg_check
+):
     with pytest.raises(
         ProcessError,
         match="Installing convert2rhel with yum exited with code '1' and output:\nfailed",
     ):
-        install_convert2rhel()
+        install_or_update_convert2rhel([])
 
     expected_calls = [["/usr/bin/yum", "install", "convert2rhel", "-y"]]
 
@@ -67,7 +75,7 @@ def test_update_convert2rhel_raise_exception(mock_run_subprocess, mock_pkg_check
         ProcessError,
         match="Updating convert2rhel with yum exited with code '1' and output:\nfailed",
     ):
-        install_convert2rhel()
+        install_or_update_convert2rhel([])
 
     expected_calls = [
         ["/usr/bin/yum", "update", "convert2rhel", "-y"],

tests/test_main_analysis.py

@@ -10,8 +10,7 @@
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("scripts.c2r_script.gather_json_report", side_effect=[{"actions": []}])
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(False, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(False, 1))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.run_convert2rhel", return_value=("", 0))
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -37,8 +36,7 @@ def test_main_success_c2r_installed(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     mock_gather_json_report,
     capsys,  # to check for rollback info in stdout
 ):
@@ -49,10 +47,8 @@ def test_main_success_c2r_installed(
     assert "Convert2RHEL Analysis script finished successfully!" in output
     assert '"alert": false' in output
     assert mock_rollback_inhibitor_check.call_count == 1
-
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_json_report.call_count == 1
@@ -70,8 +66,7 @@ def test_main_success_c2r_installed(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("scripts.c2r_script.gather_json_report", side_effect=[{"actions": []}])
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(False, None))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(False, None))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.run_convert2rhel", return_value=("", 0))
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -97,8 +92,7 @@ def test_main_success_c2r_updated(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     mock_gather_json_report,
     capsys,  # to check for rollback info in stdout
 ):
@@ -111,8 +105,7 @@ def test_main_success_c2r_updated(
     assert mock_rollback_inhibitor_check.call_count == 1
 
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_json_report.call_count == 1
@@ -131,8 +124,7 @@ def test_main_success_c2r_updated(
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("__builtin__.open", new_callable=mock_open())
 @patch("scripts.c2r_script.gather_json_report", return_value={})
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(True, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(True, 1))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.run_convert2rhel", side_effect=ProcessError("test", "Process error"))
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -153,8 +145,7 @@ def test_main_process_error(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     mock_gather_json_report,
     mock_open_func,
     capsys,
@@ -167,8 +158,7 @@ def test_main_process_error(
     assert '"alert": true' in output
 
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_json_report.call_count == 1
@@ -185,8 +175,7 @@ def test_main_process_error(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("__builtin__.open", mock_open(read_data="not json serializable"))
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(True, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(True, 1))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.run_convert2rhel", side_effect=Mock())
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -207,8 +196,7 @@ def test_main_general_exception(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     capsys,
 ):
     main()
@@ -219,8 +207,7 @@ def test_main_general_exception(
     assert '"alert": true' in output
 
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_textual_report.call_count == 0
@@ -235,8 +222,7 @@ def test_main_general_exception(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("__builtin__.open", mock_open(read_data="not json serializable"))
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(True, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(True, 1))
 @patch("os.path.exists", return_value=False)
 @patch("scripts.c2r_script._check_ini_file_modified", return_value=True)
 @patch("scripts.c2r_script.run_convert2rhel", side_effect=Mock())
@@ -259,8 +245,7 @@ def test_main_inhibited_ini_modified(
     mock_run_convert2rhel,
     mock_custom_ini,
     mock_ini_modified,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     capsys,
 ):
     main()
@@ -273,10 +258,9 @@ def test_main_inhibited_ini_modified(
     assert mock_archive_analysis_report.call_count == 0
     assert mock_get_system_distro_version.call_count == 1
     assert mock_is_eligible_releases.call_count == 1
-    assert mock_setup_convert2rhel.call_count == 1
     assert mock_custom_ini.call_count == 1
     assert mock_ini_modified.call_count == 4
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_run_convert2rhel.call_count == 0
     assert mock_gather_textual_report.call_count == 0
     assert mock_generate_report_message.call_count == 0
@@ -288,8 +272,7 @@ def test_main_inhibited_ini_modified(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("__builtin__.open", mock_open(read_data="not json serializable"))
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(True, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(True, 1))
 @patch("os.path.exists", return_value=True)
 @patch("scripts.c2r_script.run_convert2rhel", side_effect=Mock())
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -310,8 +293,7 @@ def test_main_inhibited_custom_ini(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     capsys,
 ):
     main()
@@ -325,8 +307,7 @@ def test_main_inhibited_custom_ini(
     assert mock_get_system_distro_version.call_count == 1
     assert mock_is_eligible_releases.call_count == 1
     assert mock_inhibitor_check.call_count == 4
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_run_convert2rhel.call_count == 0
     assert mock_gather_textual_report.call_count == 0
     assert mock_generate_report_message.call_count == 0
@@ -338,8 +319,7 @@ def test_main_inhibited_custom_ini(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("scripts.c2r_script.gather_json_report", side_effect=[{"actions": [], "status": "ERROR"}])
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(False, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(False, 1))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.run_convert2rhel", return_value=("", 1))
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
@@ -363,8 +343,7 @@ def test_main_inhibited_c2r_installed_no_rollback_err(
     mock_gather_textual_report,
     mock_run_convert2rhel,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     mock_gather_json_report,
     capsys,
 ):
@@ -376,8 +355,7 @@ def test_main_inhibited_c2r_installed_no_rollback_err(
     assert '"alert": true' in output
 
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_json_report.call_count == 1
@@ -400,8 +378,7 @@ def test_main_inhibited_c2r_installed_no_rollback_err(
 @patch("scripts.c2r_script.IS_ANALYSIS", True)
 @patch("scripts.c2r_script.SCRIPT_TYPE", "ANALYSIS")
 @patch("scripts.c2r_script.gather_json_report", side_effect=[{"actions": []}])
-@patch("scripts.c2r_script.setup_convert2rhel", side_effect=Mock())
-@patch("scripts.c2r_script.install_convert2rhel", return_value=(False, 1))
+@patch("scripts.c2r_script.install_or_update_convert2rhel", return_value=(False, 1))
 @patch("scripts.c2r_script.check_convert2rhel_inhibitors_before_run", return_value=("", 0))
 @patch("scripts.c2r_script.gather_textual_report", side_effect=Mock(return_value=""))
 @patch("scripts.c2r_script.generate_report_message", side_effect=Mock(return_value=("ERROR", False)))
@@ -425,8 +402,7 @@ def test_main_inhibited_c2r_installed_rollback_errors(
     mock_generate_report_message,
     mock_gather_textual_report,
     mock_inhibitor_check,
-    mock_install_convert2rhel,
-    mock_setup_convert2rhel,
+    mock_install_or_update_convert2rhel,
     mock_gather_json_report,
     run_return_code,
     capsys,
@@ -444,8 +420,7 @@ def test_main_inhibited_c2r_installed_rollback_errors(
     assert '"alert": true' in output
 
     assert mock_update_insights_inventory.call_count == 0
-    assert mock_setup_convert2rhel.call_count == 1
-    assert mock_install_convert2rhel.call_count == 1
+    assert mock_install_or_update_convert2rhel.call_count == 1
     assert mock_inhibitor_check.call_count == 1
     assert mock_run_convert2rhel.call_count == 1
     assert mock_gather_json_report.call_count == 1