Apply suggestions from code review

Co-authored-by: Adam Hošek <[email protected]>

convert2rhel/actions/post_conversion/hostmetering.py

@@ -152,7 +152,7 @@ def _check_env_var(self):
             return False
 
         if tool_opts.configure_host_metering not in ("force", "auto"):
-            logger.debug("Value for environment variable not recognized: %s" % tool_opts.configure_host_metering)
+            logger.debug("Value for environment variable not recognized: {}".format(tool_opts.configure_host_metering))
             self.add_message(
                 level="WARNING",
                 id="UNRECOGNIZED_OPTION_CONFIGURE_HOST_METERING",

convert2rhel/toolopts/__init__.py

@@ -17,7 +17,6 @@
 __metaclass__ = type
 
 import logging
-import os
 
 from convert2rhel.utils.subscription import setup_rhsm_parts
 
@@ -27,7 +26,10 @@
 
 class ToolOpts:
     def _handle_config_conflict(self, config_sources):
-        file_config = next((config for config in config_sources if config.SOURCE == "configuration file"), None)
+        file_config = next(
+            (config for config in config_sources if config.SOURCE == "configuration file"),
+            None,
+        )
         # No file config detected, let's just bail out.
         if not file_config:
             return

convert2rhel/unit_tests/utils/utils_test.py

@@ -40,7 +40,7 @@
 
 from convert2rhel import exceptions, systeminfo, toolopts, unit_tests, utils  # Imports unit_tests/__init__.py
 from convert2rhel.systeminfo import system_info
-from convert2rhel.unit_tests import RunCmdInPtyMocked, RunSubprocessMocked, is_rpm_based_os
+from convert2rhel.unit_tests import RunCmdInPtyMocked, RunSubprocessMocked, conftest, is_rpm_based_os
 
 
 DOWNLOADED_RPM_NVRA = "kernel-4.18.0-193.28.1.el8_2.x86_64"
@@ -1134,3 +1134,40 @@ def test_remove_pkgs_with_empty_list(self, caplog):
 )
 def test_remove_epoch_from_yum_nevra_notation(pkg_nevra, nvra_without_epoch):
     assert utils._remove_epoch_from_yum_nevra_notation(pkg_nevra) == nvra_without_epoch
+
+
+@pytest.mark.parametrize(
+    ("env_name", "env_value", "tool_opts_name", "message"),
+    (
+        (
+            "CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK",
+            True,
+            "skip_kernel_currency_check",
+            "The environment variable CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK is deprecated and is set to be removed on Convert2RHEL 2.4.0.\n"
+            "Please, use the configuration file instead.",
+        ),
+    ),
+)
+def test_warn_deprecated_env(global_tool_opts, monkeypatch, env_name, env_value, tool_opts_name, message, caplog):
+    """Test setting the value based on env variable and it's logging."""
+    monkeypatch.setattr(utils, "tool_opts", global_tool_opts)
+    monkeypatch.setenv(env_name, env_value)
+
+    utils.warn_deprecated_env(env_name)
+    assert getattr(global_tool_opts, tool_opts_name) == str(env_value)
+    assert caplog.records[-1].message == message
+
+
+def test_warn_deprecated_env_wrong_name(global_tool_opts, monkeypatch, caplog):
+    """Test when unsupported env variable is used, nothing is set."""
+    monkeypatch.setattr(utils, "tool_opts", global_tool_opts)
+
+    utils.warn_deprecated_env("UNSUPPORTED_ENV_VAR")
+
+    # Get tool_opts with default values
+    default_tool_opts = toolopts.ToolOpts()
+    default_tool_opts.initialize(config_sources=[conftest.CliConfigMock(), conftest.FileConfigMock()])
+
+    for item, value in global_tool_opts.__dict__.items():
+        assert default_tool_opts.__dict__[item] == value
+    assert not caplog.text

convert2rhel/utils/__init__.py

@@ -1144,9 +1144,9 @@ def warn_deprecated_env(env_name):
         return None
 
     root_logger.warning(
-        "The environment variable {} is deprecated in favor of using a configuration file and will be removed in version Convert2RHEL 2.4.0."
+        "The environment variable {} is deprecated and is set to be removed on Convert2RHEL 2.4.0.\n"
+        "Please, use the configuration file instead.".format(env_name)
     )
     key = env_var_to_toolopts_map[env_name]
     value = os.getenv(env_name, None)
-    print(vars(tool_opts))
     tool_opts.update_opts(key, value)