Create a framework of functional tests for configuration files (#5287)

* Migrate old unittest to the new framework for testing.

* Add a regression test for #4746 : This permits to introduce an example
of configuration file with an error.

* Proper import for pytest import of CaptureFixture

Co-authored-by: Daniël van Noord <[email protected]>

Author: Pierre-Sassoulas

pylint/config/option_manager_mixin.py

@@ -271,6 +271,7 @@ def read_config_file(self, config_file=None, verbose=None):
 
         use_config_file = config_file and os.path.exists(config_file)
         if use_config_file:
+            self.set_current_module(config_file)
             parser = self.cfgfile_parser
             if config_file.endswith(".toml"):
                 self._parse_toml(config_file, parser)

pylint/config/option_parser.py

@@ -24,8 +24,7 @@ def format_option_help(self, formatter=None):
             formatter = self.formatter
         outputlevel = getattr(formatter, "output_level", 0)
         formatter.store_option_strings(self)
-        result = []
-        result.append(formatter.format_heading("Options"))
+        result = [formatter.format_heading("Options")]
         formatter.indent()
         if self.option_list:
             result.append(optparse.OptionContainer.format_option_help(self, formatter))

pylint/testutils/configuration_test.py

@@ -0,0 +1,110 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
+
+"""Utility functions for configuration testing."""
+import copy
+import json
+import logging
+import unittest
+from pathlib import Path
+from typing import Any, Dict, Tuple, Union
+from unittest.mock import Mock
+
+from pylint.lint import Run
+
+USER_SPECIFIC_PATH = Path(__file__).parent.parent.parent
+# We use Any in this typing because the configuration contains real objects and constants
+# that could be a lot of things.
+ConfigurationValue = Any
+PylintConfiguration = Dict[str, ConfigurationValue]
+
+
+def get_expected_or_default(
+    tested_configuration_file: str, suffix: str, default: ConfigurationValue
+) -> str:
+    """Return the expected value from the file if it exists, or the given default."""
+
+    def get_path_according_to_suffix() -> Path:
+        path = Path(tested_configuration_file)
+        return path.parent / f"{path.stem}.{suffix}"
+
+    expected = default
+    expected_result_path = get_path_according_to_suffix()
+    if expected_result_path.exists():
+        with open(expected_result_path, encoding="utf8") as f:
+            expected = f.read()
+        # logging is helpful to realize your file is not taken into
+        # account after a misspell of the file name. The output of the
+        # program is checked during the test so printing messes with the result.
+        logging.info("%s exists.", expected_result_path)
+    else:
+        logging.info("%s not found, using '%s'.", expected_result_path, default)
+    return expected
+
+
+EXPECTED_CONF_APPEND_KEY = "functional_append"
+EXPECTED_CONF_REMOVE_KEY = "functional_remove"
+
+
+def get_expected_configuration(
+    configuration_path: str, default_configuration: PylintConfiguration
+) -> PylintConfiguration:
+    """Get the expected parsed configuration of a configuration functional test"""
+    result = copy.deepcopy(default_configuration)
+    config_as_json = get_expected_or_default(
+        configuration_path, suffix="result.json", default="{}"
+    )
+    to_override = json.loads(config_as_json)
+    for key, value in to_override.items():
+        if key == EXPECTED_CONF_APPEND_KEY:
+            for fkey, fvalue in value.items():
+                result[fkey] += fvalue
+        elif key == EXPECTED_CONF_REMOVE_KEY:
+            for fkey, fvalue in value.items():
+                new_value = []
+                for old_value in result[fkey]:
+                    if old_value not in fvalue:
+                        new_value.append(old_value)
+                result[fkey] = new_value
+        else:
+            result[key] = value
+    return result
+
+
+def get_expected_output(configuration_path: str) -> Tuple[int, str]:
+    """Get the expected output of a functional test."""
+    output = get_expected_or_default(configuration_path, suffix="out", default="")
+    if output:
+        # logging is helpful to see what the expected exit code is and why.
+        # The output of the program is checked during the test so printing
+        # messes with the result.
+        logging.info(
+            "Output exists for %s so the expected exit code is 2", configuration_path
+        )
+        exit_code = 2
+    else:
+        logging.info(".out file does not exists, so the expected exit code is 0")
+        exit_code = 0
+    return exit_code, output.format(
+        abspath=configuration_path,
+        relpath=Path(configuration_path).relative_to(USER_SPECIFIC_PATH),
+    )
+
+
+def run_using_a_configuration_file(
+    configuration_path: Union[Path, str], file_to_lint: str = __file__
+) -> Tuple[Mock, Mock, Run]:
+    """Simulate a run with a configuration without really launching the checks."""
+    configuration_path = str(configuration_path)
+    args = ["--rcfile", configuration_path, file_to_lint]
+    # We do not capture the `SystemExit` as then the `runner` variable
+    # would not be accessible outside the `with` block.
+    with unittest.mock.patch("sys.exit") as mocked_exit:
+        # Do not actually run checks, that could be slow. We don't mock
+        # `Pylinter.check`: it calls `Pylinter.initialize` which is
+        # needed to properly set up messages inclusion/exclusion
+        # in `_msg_states`, used by `is_message_enabled`.
+        check = "pylint.lint.pylinter.check_parallel"
+        with unittest.mock.patch(check) as mocked_check_parallel:
+            runner = Run(args)
+    return mocked_exit, mocked_check_parallel, runner

tests/config/conftest.py

@@ -0,0 +1,10 @@
+from pathlib import Path
+
+import pytest
+
+HERE = Path(__file__).parent
+
+
+@pytest.fixture()
+def file_to_lint_path() -> str:
+    return str(HERE / "file_to_lint.py")

tests/config/functional/ini/pylintrc_with_message_control.ini

@@ -0,0 +1,5 @@
+# Check that we can read the "regular" INI .pylintrc file
+[messages control]
+disable = logging-not-lazy,logging-format-interpolation
+jobs = 10
+reports = yes

tests/config/functional/ini/pylintrc_with_message_control.result.json

@@ -0,0 +1,7 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  },
+  "jobs": 10,
+  "reports": true
+}

tests/config/functional/setup_cfg/setup_cfg_with_message_control.cfg

@@ -0,0 +1,5 @@
+# setup.cfg is an INI file where section names are prefixed with "pylint."
+[pylint.messages control]
+disable = logging-not-lazy,logging-format-interpolation
+jobs = 10
+reports = yes

tests/config/functional/setup_cfg/setup_cfg_with_message_control.result.json

@@ -0,0 +1,7 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  },
+  "jobs": 10,
+  "reports": true
+}

tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.out

@@ -0,0 +1,2 @@
+************* Module {abspath}
+{relpath}:1:0: E0013: Plugin 'pylint_websockets' is impossible to load, is it installed ? ('No module named 'pylint_websockets'') (bad-plugin-value)

tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.result.json

@@ -0,0 +1,3 @@
+{
+  "load_plugins": ["pylint_websockets"]
+}

tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml

@@ -0,0 +1,3 @@
+# The pylint_websockets plugin does not exist and therefore this toml is invalid
+[tool.pylint.MASTER]
+load-plugins = 'pylint_websockets'

tests/config/functional/toml/rich_types.result.json

@@ -0,0 +1,7 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  },
+  "jobs": 10,
+  "reports": true
+}

tests/config/functional/toml/rich_types.toml

@@ -0,0 +1,11 @@
+# Check that we can read a TOML file where lists, integers and
+# booleans are expressed as such (and not as strings), using TOML
+# type system.
+
+[tool.pylint."messages control"]
+disable = [
+    "logging-not-lazy",
+    "logging-format-interpolation",
+]
+jobs = 10
+reports = true

tests/config/functional/toml/toml_with_enable.result.json

@@ -0,0 +1,9 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]],
+    "enable": [["suppressed-message"], ["locally-disabled"]]
+  },
+  "functional_remove": {
+    "disable": [["suppressed-message"], ["locally-disabled"]]
+  }
+}

tests/config/functional/toml/toml_with_enable.toml

@@ -0,0 +1,5 @@
+# Check that we can add or remove value in list
+# (This is mostly a check for the functional test themselves)
+[tool.pylint."messages control"]
+disable = "logging-not-lazy,logging-format-interpolation"
+enable = "locally-disabled,suppressed-message"

tests/config/functional/toml/toml_with_message_control.result.json

@@ -0,0 +1,7 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  },
+  "jobs": 10,
+  "reports": true
+}

tests/config/functional/toml/toml_with_message_control.toml

@@ -0,0 +1,7 @@
+# Check that we can read a TOML file where lists and integers are
+# expressed as strings.
+
+[tool.pylint."messages control"]
+disable = "logging-not-lazy,logging-format-interpolation"
+jobs = "10"
+reports = "yes"

tests/config/test_config.py

@@ -1,33 +1,9 @@
-# pylint: disable=missing-module-docstring, missing-function-docstring, protected-access
 import os
-import unittest.mock
 from pathlib import Path
-from typing import Optional, Set, Union
+from typing import Optional, Set
 
-import pylint.lint
 from pylint.lint.run import Run
-
-# We use an external file and not __file__ or pylint warning in this file
-# makes the tests fails because the exit code changes
-FILE_TO_LINT = str(Path(__file__).parent / "file_to_lint.py")
-
-
-def get_runner_from_config_file(
-    config_file: Union[str, Path], expected_exit_code: int = 0
-) -> Run:
-    """Initialize pylint with the given configuration file and return the Run"""
-    args = ["--rcfile", str(config_file), FILE_TO_LINT]
-    # If we used `pytest.raises(SystemExit)`, the `runner` variable
-    # would not be accessible outside the `with` block.
-    with unittest.mock.patch("sys.exit") as mocked_exit:
-        # Do not actually run checks, that could be slow. Do not mock
-        # `Pylinter.check`: it calls `Pylinter.initialize` which is
-        # needed to properly set up messages inclusion/exclusion
-        # in `_msg_states`, used by `is_message_enabled`.
-        with unittest.mock.patch("pylint.lint.pylinter.check_parallel"):
-            runner = pylint.lint.Run(args)
-    mocked_exit.assert_called_once_with(expected_exit_code)
-    return runner
+from pylint.testutils.configuration_test import run_using_a_configuration_file
 
 
 def check_configuration_file_reader(
@@ -46,74 +22,7 @@ def check_configuration_file_reader(
     assert bool(runner.linter.config.reports) == expected_reports_truthey
 
 
-def test_can_read_ini(tmp_path: Path) -> None:
-    # Check that we can read the "regular" INI .pylintrc file
-    config_file = tmp_path / ".pylintrc"
-    config_file.write_text(
-        """
-[messages control]
-disable = logging-not-lazy,logging-format-interpolation
-jobs = 10
-reports = yes
-"""
-    )
-    run = get_runner_from_config_file(config_file)
-    check_configuration_file_reader(run)
-
-
-def test_can_read_setup_cfg(tmp_path: Path) -> None:
-    # Check that we can read a setup.cfg (which is an INI file where
-    # section names are prefixed with "pylint."
-    config_file = tmp_path / "setup.cfg"
-    config_file.write_text(
-        """
-[pylint.messages control]
-disable = logging-not-lazy,logging-format-interpolation
-jobs = 10
-reports = yes
-"""
-    )
-    run = get_runner_from_config_file(config_file)
-    check_configuration_file_reader(run)
-
-
-def test_can_read_toml(tmp_path: Path) -> None:
-    # Check that we can read a TOML file where lists and integers are
-    # expressed as strings.
-    config_file = tmp_path / "pyproject.toml"
-    config_file.write_text(
-        """
-[tool.pylint."messages control"]
-disable = "logging-not-lazy,logging-format-interpolation"
-jobs = "10"
-reports = "yes"
-"""
-    )
-    run = get_runner_from_config_file(config_file)
-    check_configuration_file_reader(run)
-
-
-def test_can_read_toml_rich_types(tmp_path: Path) -> None:
-    # Check that we can read a TOML file where lists, integers and
-    # booleans are expressed as such (and not as strings), using TOML
-    # type system.
-    config_file = tmp_path / "pyproject.toml"
-    config_file.write_text(
-        """
-[tool.pylint."messages control"]
-disable = [
-    "logging-not-lazy",
-    "logging-format-interpolation",
-]
-jobs = 10
-reports = true
-"""
-    )
-    run = get_runner_from_config_file(config_file)
-    check_configuration_file_reader(run)
-
-
-def test_can_read_toml_env_variable(tmp_path: Path) -> None:
+def test_can_read_toml_env_variable(tmp_path: Path, file_to_lint_path: str) -> None:
     """We can read and open a properly formatted toml file."""
     config_file = tmp_path / "pyproject.toml"
     config_file.write_text(
@@ -126,5 +35,8 @@ def test_can_read_toml_env_variable(tmp_path: Path) -> None:
     )
     env_var = "tmp_path_env"
     os.environ[env_var] = str(config_file)
-    run = get_runner_from_config_file(f"${env_var}")
-    check_configuration_file_reader(run)
+    mock_exit, _, runner = run_using_a_configuration_file(
+        f"${env_var}", file_to_lint_path
+    )
+    mock_exit.assert_called_once_with(0)
+    check_configuration_file_reader(runner)