Port ModifiedRPMFilesDiff to Action framework

This new action provides information about difference between 'rpm -Va'
from before and after the conversion.

Changed the pretend_os to use monkeypatch for setting the
no_rpm_va value instead of directly setting it. This caused problems,
because the set value stay for the next tests.

convert2rhel/actions/post_conversion/modified_rpm_files_diff.py

@@ -0,0 +1,82 @@
+# Copyright(C) 2024 Red Hat, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+__metaclass__ = type
+
+import difflib
+import logging
+import os
+
+from convert2rhel import actions, utils
+from convert2rhel.logger import LOG_DIR
+from convert2rhel.systeminfo import system_info
+from convert2rhel.toolopts import POST_RPM_VA_LOG_FILENAME, PRE_RPM_VA_LOG_FILENAME
+
+
+logger = logging.getLogger(__name__)
+
+
+class ModifiedRPMFilesDiff(actions.Action):
+    id = "MODIFIED_RPM_FILES_DIFF"
+
+    def run(self):
+        """
+        Get a list of modified rpm files after the conversion and
+        compare it to the one from before the conversion.
+        """
+        super(ModifiedRPMFilesDiff, self).run()
+
+        logger.task("Final: Show RPM files modified by the conversion")
+
+        system_info.generate_rpm_va(log_filename=POST_RPM_VA_LOG_FILENAME)
+
+        pre_rpm_va_log_path = os.path.join(LOG_DIR, PRE_RPM_VA_LOG_FILENAME)
+        if not os.path.exists(pre_rpm_va_log_path):
+            logger.info("Skipping comparison of the 'rpm -Va' output from before and after the conversion.")
+            self.add_message(
+                level="INFO",
+                id="SKIPPED_MODIFIED_RPM_FILES_DIFF",
+                title="Skipped comparison of 'rpm -Va' output from before and after the conversion.",
+                description="Comparison of 'rpm -Va' output was skipped due missing output "
+                "of the 'rpm -Va' run before the conversion.",
+                diagnosis="This is caused mainly by using '--no-rpm-va' argument for convert2rhel.",
+            )
+            return
+
+        pre_rpm_va = utils.get_file_content(pre_rpm_va_log_path, True)
+        post_rpm_va_log_path = os.path.join(LOG_DIR, POST_RPM_VA_LOG_FILENAME)
+        post_rpm_va = utils.get_file_content(post_rpm_va_log_path, True)
+        modified_rpm_files_diff = "\n".join(
+            difflib.unified_diff(
+                pre_rpm_va,
+                post_rpm_va,
+                fromfile=pre_rpm_va_log_path,
+                tofile=post_rpm_va_log_path,
+                n=0,
+                lineterm="",
+            )
+        )
+
+        if modified_rpm_files_diff:
+            logger.info(
+                "Comparison of modified rpm files from before and after the conversion:\n%s" % modified_rpm_files_diff
+            )
+            self.add_message(
+                level="INFO",
+                id="FOUND_MODIFIED_RPM_FILES",
+                title="Modified rpm files from before and after the conversion were found.",
+                description="Comparison of modified rpm files from before and after "
+                "the conversion: \n%s" % modified_rpm_files_diff,
+            )

convert2rhel/main.py

@@ -373,9 +373,6 @@ def post_ponr_changes():
     """Start the conversion itself"""
     post_ponr_conversion()
 
-    loggerinst.task("Final: Show RPM files modified by the conversion")
-    systeminfo.system_info.modified_rpm_files_diff()
-
     loggerinst.task("Final: Update GRUB2 configuration")
     grub.update_grub_after_conversion()
 

convert2rhel/systeminfo.py

@@ -395,33 +395,6 @@ def generate_rpm_va(self, log_filename=PRE_RPM_VA_LOG_FILENAME):
         utils.store_content_to_file(output_file, rpm_va)
         self.logger.info("The 'rpm -Va' output has been stored in the %s file." % output_file)
 
-    def modified_rpm_files_diff(self):
-        """Get a list of modified rpm files after the conversion and compare it to the one from before the conversion."""
-        self.generate_rpm_va(log_filename=POST_RPM_VA_LOG_FILENAME)
-
-        pre_rpm_va_log_path = os.path.join(logger.LOG_DIR, PRE_RPM_VA_LOG_FILENAME)
-        if not os.path.exists(pre_rpm_va_log_path):
-            self.logger.info("Skipping comparison of the 'rpm -Va' output from before and after the conversion.")
-            return
-        pre_rpm_va = utils.get_file_content(pre_rpm_va_log_path, True)
-        post_rpm_va_log_path = os.path.join(logger.LOG_DIR, POST_RPM_VA_LOG_FILENAME)
-        post_rpm_va = utils.get_file_content(post_rpm_va_log_path, True)
-        modified_rpm_files_diff = "\n".join(
-            difflib.unified_diff(
-                pre_rpm_va,
-                post_rpm_va,
-                fromfile=pre_rpm_va_log_path,
-                tofile=post_rpm_va_log_path,
-                n=0,
-                lineterm="",
-            )
-        )
-
-        if modified_rpm_files_diff:
-            self.logger.info(
-                "Comparison of modified rpm files from before and after the conversion:\n%s" % modified_rpm_files_diff
-            )
-
     @staticmethod
     def is_rpm_installed(name):
         _, return_code = run_subprocess(["rpm", "-q", name], print_cmd=False, print_output=False)

convert2rhel/unit_tests/actions/post_conversion/modified_rpm_files_diff_test.py

@@ -0,0 +1,129 @@
+# Copyright(C) 2024 Red Hat, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+__metaclass__ = type
+
+import logging
+
+import pytest
+import six
+
+from convert2rhel import actions, logger, systeminfo, toolopts, utils
+from convert2rhel.actions.post_conversion import modified_rpm_files_diff
+from convert2rhel.systeminfo import system_info
+
+
+six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
+from six.moves import mock
+
+
+@pytest.fixture
+def modified_rpm_files_diff_instance():
+    return modified_rpm_files_diff.ModifiedRPMFilesDiff()
+
+
+def test_modified_rpm_files_diff_with_no_rpm_va(monkeypatch, modified_rpm_files_diff_instance, caplog):
+    monkeypatch.setattr(toolopts.tool_opts, "no_rpm_va", mock.Mock(return_value=True))
+
+    # This can be removed when systeminfo is ported to use global logger
+    monkeypatch.setattr(systeminfo.system_info, "logger", logging.getLogger(__name__))
+
+    modified_rpm_files_diff_instance.run()
+
+    expected = set(
+        (
+            actions.ActionMessage(
+                level="INFO",
+                id="SKIPPED_MODIFIED_RPM_FILES_DIFF",
+                title="Skipped comparison of 'rpm -Va' output from before and after the conversion.",
+                description="Comparison of 'rpm -Va' output was skipped due missing output "
+                "of the 'rpm -Va' run before the conversion.",
+                diagnosis="This is caused mainly by using '--no-rpm-va' argument for convert2rhel.",
+            ),
+        ),
+    )
+
+    assert expected.issubset(modified_rpm_files_diff_instance.messages)
+    assert expected.issuperset(modified_rpm_files_diff_instance.messages)
+    assert "Skipping comparison of the 'rpm -Va' output from before and after the conversion." in caplog.messages
+
+
+@pytest.mark.parametrize(
+    ("rpm_va_pre_output", "rpm_va_post_output", "expected_raw", "different"),
+    (
+        (
+            """S.5.?..T. c     /etc/yum.repos.d/CentOS-Linux-AppStream.repo
+            S.5.?..T.        /etc/yum.repos.d/CentOS-Linux-BaseOS.repo""",
+            """S.5.?..T. c     /etc/yum.repos.d/CentOS-Linux-AppStream.repo
+            S.5.?..T.        /etc/yum.repos.d/CentOS-Linux-BaseOS.repo""",
+            [],
+            False,
+        ),
+        (
+            """S.5.?..T. c     /etc/yum.repos.d/CentOS-Linux-AppStream.repo""",
+            """S.5.?..T. c     /etc/yum.repos.d/CentOS-Linux-AppStream.repo
+            S.5.?..T.        /etc/yum.repos.d/CentOS-Linux-BaseOS.repo""",
+            actions.ActionMessage(
+                level="INFO",
+                id="FOUND_MODIFIED_RPM_FILES",
+                title="Modified rpm files from before and after the conversion were found.",
+                description="Comparison of modified rpm files from before and after the conversion: \n"
+                "--- {path}/rpm_va.log\n"
+                "+++ {path}/rpm_va_after_conversion.log\n"
+                "@@ -1,0 +2 @@\n"
+                "+S.5.?..T.        /etc/yum.repos.d/CentOS-Linux-BaseOS.repo",
+            ),
+            True,
+        ),
+    ),
+)
+def test_modified_rpm_files_diff_without_differences_after_conversion(
+    monkeypatch,
+    modified_rpm_files_diff_instance,
+    caplog,
+    tmpdir,
+    rpm_va_pre_output,
+    rpm_va_post_output,
+    different,
+    expected_raw,
+):
+    monkeypatch.setattr(utils, "run_subprocess", mock.Mock(return_value=(rpm_va_pre_output, 0)))
+    monkeypatch.setattr(logger, "LOG_DIR", str(tmpdir))
+    # Need to patch explicitly since the modified_rpm_files_diff is already instanciated in the fixture
+    monkeypatch.setattr(modified_rpm_files_diff, "LOG_DIR", str(tmpdir))
+
+    # This can be removed when systeminfo is ported to use global logger
+    monkeypatch.setattr(systeminfo.system_info, "logger", logging.getLogger(__name__))
+
+    # Generate the pre-conversion rpm -Va output
+    system_info.generate_rpm_va()
+
+    # Change the output to the post conversion output
+    monkeypatch.setattr(utils, "run_subprocess", mock.Mock(return_value=(rpm_va_post_output, 0)))
+
+    modified_rpm_files_diff_instance.run()
+
+    if different:
+        # Add the test paths to the right places of diff
+        expected_raw.description = expected_raw.description.format(path=str(tmpdir))
+        expected = {expected_raw}
+
+    assert ("Comparison of modified rpm files from before and after the conversion:" in caplog.text) == different
+
+    if different:
+        assert expected.issubset(modified_rpm_files_diff_instance.messages)
+        assert expected.issuperset(modified_rpm_files_diff_instance.messages)
+    else:
+        assert modified_rpm_files_diff_instance.messages == []

convert2rhel/unit_tests/conftest.py

@@ -209,7 +209,7 @@ def pretend_os(request, pkg_root, monkeypatch):
         "_get_architecture",
         value=lambda: "x86_64",
     )
-    tool_opts.no_rpm_va = True
+    monkeypatch.setattr(tool_opts, "no_rpm_va", True)
 
     # We can't depend on a test environment (containers) having an init system so we have to
     # disable probing for the right value by hardcoding an anwer

convert2rhel/unit_tests/main_test.py

@@ -292,7 +292,6 @@ def test_main(monkeypatch, tmp_path):
     monkeypatch.setattr(report, "_summary", report_summary_mock)
     monkeypatch.setattr(utils, "ask_to_continue", ask_to_continue_mock)
     monkeypatch.setattr(main, "post_ponr_conversion", post_ponr_conversion_mock)
-    monkeypatch.setattr(system_info, "modified_rpm_files_diff", rpm_files_diff_mock)
     monkeypatch.setattr(grub, "update_grub_after_conversion", update_grub_after_conversion_mock)
     monkeypatch.setattr(utils, "remove_tmp_dir", remove_tmp_dir_mock)
     monkeypatch.setattr(utils, "restart_system", restart_system_mock)
@@ -319,7 +318,6 @@ def test_main(monkeypatch, tmp_path):
     assert clear_versionlock_mock.call_count == 1
     assert ask_to_continue_mock.call_count == 1
     assert post_ponr_conversion_mock.call_count == 1
-    assert rpm_files_diff_mock.call_count == 1
     assert remove_tmp_dir_mock.call_count == 1
     assert restart_system_mock.call_count == 1
     assert finish_collection_mock.call_count == 1

convert2rhel/unit_tests/systeminfo_test.py

@@ -42,40 +42,6 @@ def register_system_info_logger(monkeypatch):
     monkeypatch.setattr(system_info, "logger", logging.getLogger(__name__))
 
 
-class TestRPMFilesDiff:
-    def test_modified_rpm_files_diff_with_no_rpm_va(self, monkeypatch):
-        monkeypatch.setattr(tool_opts, "no_rpm_va", mock.Mock(return_value=True))
-        assert system_info.modified_rpm_files_diff() is None
-
-    def test_modified_rpm_files_diff_without_differences_after_conversion(self, monkeypatch):
-        monkeypatch.setattr(system_info, "generate_rpm_va", mock.Mock())
-        monkeypatch.setattr(utils, "get_file_content", mock.Mock(side_effect=(["rpm1", "rpm2"], ["rpm1", "rpm2"])))
-
-        assert system_info.modified_rpm_files_diff() is None
-
-    def test_modified_rpm_files_diff_with_differences_after_conversion(self, monkeypatch, caplog):
-        monkeypatch.setattr(system_info, "generate_rpm_va", mock.Mock())
-        monkeypatch.setattr(os.path, "exists", mock.Mock(return_value=True))
-        monkeypatch.setattr(tool_opts, "no_rpm_va", False)
-        monkeypatch.setattr(
-            utils,
-            "get_file_content",
-            mock.Mock(
-                side_effect=(
-                    [".M.......  g /etc/pki/ca-trust/extracted/java/cacerts"],
-                    [
-                        ".M.......  g /etc/pki/ca-trust/extracted/java/cacerts",
-                        "S.5....T.  c /etc/yum.conf",
-                    ],
-                )
-            ),
-        )
-
-        system_info.modified_rpm_files_diff()
-
-        assert any("S.5....T.  c /etc/yum.conf" in elem.message for elem in caplog.records if elem.levelname == "INFO")
-
-
 class TestGenerateRPMVA:
     def test_generate_rpm_va(self, global_tool_opts, monkeypatch, tmpdir):
         global_tool_opts.no_rpm_va = False