Apply other suggestions from review and fix tests

Fixing a couple of tests related to the config parsing.

convert2rhel/toolopts.py

@@ -549,7 +549,7 @@ def options_from_config_files(cfg_path):
         This function will parse the configuration file in the following way:
 
         1) If the path provided by the user in cfg_path is set (Highest
-           priority), then we use only that.
+           priority), then we append that to the config files path as index 0.
 
         Otherwise, if cfg_path is `None`, we proceed to check the following
         paths:
@@ -562,18 +562,18 @@ def options_from_config_files(cfg_path):
 
     :param cfg_path: Path of a custom configuration file
     :type cfg_path: str
-
     :return: Dict with the supported options alongside their values.
     :rtype: dict[str, str]
     """
-    # Paths for the configuration files. In case we have cfg_path defined
-    # (meaning that the user entered something through the `-c` option), we
-    # will use only that, as it has a higher priority over the rest
-    config_paths = [cfg_path] if cfg_path else CONFIG_PATHS
+    # Paths for the configuration files. In case we have cfg_path defined (meaning that the user entered something
+    # through the `-c` option), we will insert that option as index 0 in the list of config files paths.
+    config_paths = CONFIG_PATHS
+    if cfg_path:
+        config_paths.insert(0, cfg_path)
     paths = [os.path.expanduser(path) for path in config_paths if os.path.exists(os.path.expanduser(path))]
 
     if cfg_path and not paths:
-        raise FileNotFoundError("No such file or directory: %s" % ", ".join(paths))
+        raise OSError("No such file or directory: %s" % ", ".join(paths))
 
     found_opts = _parse_options_from_config(paths)
     return found_opts
@@ -582,12 +582,7 @@ def options_from_config_files(cfg_path):
 def _parse_options_from_config(paths):
     """Parse the options from the given config files.
 
-    .. note::
-        If no configuration file is provided through the command line option
-        (`-c`), we will use the default paths and follow their priority.
-
-    :param paths: List of paths to iterate through and gather the options from
-        them.
+    :param paths: List of paths to iterate through and gather the options from them.
     :type paths: list[str]
     :return: Return a dict of loaded values under all headers from all the config files with solved priority
     :rtype: dict[str, str]
@@ -619,13 +614,11 @@ def _parse_options_from_config(paths):
 def _get_options_value(config_file, header, supported_opts):
     """Helper function to iterate through the options in a config file.
 
-    :param config_file: An instance of `py:ConfigParser` after reading the file
-        to iterate through the options.
+    :param config_file: An instance of `py:ConfigParser` after reading the file to iterate through the options.
     :type config_file: configparser.ConfigParser
     :param header: The header name to get options from.
     :type header: str
-    :param supported_opts: List of supported options that can be parsed from
-        the config file.
+    :param supported_opts: List of supported options that can be parsed from the config file.
     :type supported_opts: list[str]
     :return: Dict of keys and values loaded from the config file under the provided header
     :rtype: dict[str, str]
@@ -642,7 +635,7 @@ def _get_options_value(config_file, header, supported_opts):
             loggerinst.warning("Unsupported option '%s' in '%s'" % (option, header))
             continue
 
-        options[option] = config_file.get(header, option).strip('"')
+        options[option] = config_file.get(header, option)
         loggerinst.debug("Found %s in %s" % (option, header))
 
     return options

convert2rhel/unit_tests/toolopts_test.py

@@ -231,13 +231,13 @@ def test_no_rhsm_option_work(argv, raise_exception, no_rhsm_value, monkeypatch,
 def test_config_file(argv, content, output, message, monkeypatch, tmpdir, caplog, global_tool_opts):
     # After each test there were left data from previous
     # Re-init needed delete the set data
-    path = os.path.join(str(tmpdir), "convert2rhel.ini")
-    with open(path, "w") as file:
-        file.write(content)
-    os.chmod(path, 0o600)
+    config_file = tmpdir.join("convert2rhel.ini")
+    config_file.write(content)
+    config_file = str(config_file)
+    os.chmod(config_file, 0o600)
 
     monkeypatch.setattr(sys, "argv", argv)
-    monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=[path])
+    monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=[config_file])
     convert2rhel.toolopts.CLI()
 
     if "activation_key" in output:
@@ -271,12 +271,12 @@ def test_config_file(argv, content, output, message, monkeypatch, tmpdir, caplog
 )
 def test_multiple_auth_src_combined(argv, content, message, output, caplog, monkeypatch, tmpdir, global_tool_opts):
     """Test combination of password file or configuration file and CLI arguments."""
-    path = os.path.join(str(tmpdir), "convert2rhel.file")
-    with open(path, "w") as file:
-        file.write(content)
-    os.chmod(path, 0o600)
+    config_file = tmpdir.join("convert2rhel.ini")
+    config_file.write(content)
+    config_file = str(config_file)
+    os.chmod(config_file, 0o600)
     # The path for file is the last argument
-    argv.append(path)
+    argv.append(config_file)
 
     monkeypatch.setattr(sys, "argv", argv)
     monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=[""])
@@ -315,6 +315,13 @@ def test_multiple_auth_src_cli(argv, message, output, caplog, monkeypatch, globa
         (
             """
 [subscription_manager]
+# userame =
+            """,
+            "No options found for subscription_manager. It seems to be empty or commented.",
+        ),
+        (
+            """
+[subscription_manager]
 incorect_option = yes
             """,
             "Unsupported option",
@@ -329,37 +336,12 @@ def test_multiple_auth_src_cli(argv, message, output, caplog, monkeypatch, globa
     ),
 )
 def test_options_from_config_files_invalid_head_and_options(content, expected_message, tmpdir, caplog):
-    path = os.path.join(str(tmpdir), "convert2rhel.ini")
-
-    with open(path, "w") as file:
-        file.write(content)
-    os.chmod(path, 0o600)
+    config_file = tmpdir.join("convert2rhel.ini")
+    config_file.write(content)
+    config_file = str(config_file)
+    os.chmod(config_file, 0o600)
 
-    opts = convert2rhel.toolopts.options_from_config_files(path)
-
-    assert not opts
-    assert expected_message in caplog.text
-
-
-@pytest.mark.parametrize(
-    ("content", "expected_message"),
-    (
-        (
-            """
-[subscription_manager]
-            """,
-            "No options found for subscription_manager. It seems to be empty or commented.",
-        ),
-    ),
-)
-def test_options_from_config_files_commented_out_options(content, expected_message, tmpdir, caplog):
-    path = os.path.join(str(tmpdir), "convert2rhel.ini")
-
-    with open(path, "w") as file:
-        file.write(content)
-    os.chmod(path, 0o600)
-
-    opts = convert2rhel.toolopts.options_from_config_files(path)
+    opts = convert2rhel.toolopts.options_from_config_files(config_file)
 
     assert not opts
     assert expected_message in caplog.text
@@ -381,7 +363,7 @@ def test_options_from_config_files_commented_out_options(content, expected_messa
 [subscription_manager]
 username = "correct_username"
             """,
-            {"username": "correct_username"},
+            {"username": '"correct_username"'},
         ),
         (
             """
@@ -454,13 +436,12 @@ def test_options_from_config_files_commented_out_options(content, expected_messa
 )
 def test_options_from_config_files_default(content, output, monkeypatch, tmpdir):
     """Test config files in default path."""
-    path = os.path.join(str(tmpdir), "convert2rhel.ini")
-
-    with open(path, "w") as file:
-        file.write(content)
-    os.chmod(path, 0o600)
+    config_file = tmpdir.join("convert2rhel.ini")
+    config_file.write(content)
+    config_file = str(config_file)
+    os.chmod(config_file, 0o600)
 
-    paths = ["/nonexisting/path", path]
+    paths = ["/nonexisting/path", config_file]
     monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=paths)
     opts = convert2rhel.toolopts.options_from_config_files(None)
 
@@ -547,17 +528,17 @@ def test_options_from_config_files_default(content, output, monkeypatch, tmpdir)
 )
 def test_options_from_config_files_specified(content, output, content_lower_priority, monkeypatch, tmpdir):
     """Test user specified path for config file."""
-    path_higher_priority = os.path.join(str(tmpdir), "convert2rhel.ini")
-    with open(path_higher_priority, "w") as file:
-        file.write(content)
-    os.chmod(path_higher_priority, 0o600)
+    higher_priority_config_file = tmpdir.join("convert2rhel.ini")
+    higher_priority_config_file.write(content)
+    higher_priority_config_file = str(higher_priority_config_file)
+    os.chmod(higher_priority_config_file, 0o600)
 
-    path_lower_priority = os.path.join(str(tmpdir), "convert2rhel_lower.ini")
-    with open(path_lower_priority, "w") as file:
-        file.write(content_lower_priority)
-    os.chmod(path_lower_priority, 0o600)
+    lower_priority_config_file = tmpdir.join("convert2rhel_lower.ini")
+    lower_priority_config_file.write(content_lower_priority)
+    lower_priority_config_file = str(lower_priority_config_file)
+    os.chmod(lower_priority_config_file, 0o600)
 
-    paths = [path_higher_priority, path_lower_priority]
+    paths = [higher_priority_config_file, lower_priority_config_file]
     monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=paths)
 
     opts = convert2rhel.toolopts.options_from_config_files(None)