From 6626c239e513348ee7ed6133868c86f82bf26b67 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Wed, 10 Nov 2021 09:27:15 +0100 Subject: [PATCH 01/23] Create a framework of functional tests for configuration files Also migrate three standard unittest to the new framework for testing. --- pylint/testutils/configuration_test.py | 85 +++++++++++++++++++ .../ini/pylintrc_with_message_control.ini | 5 ++ .../pylintrc_with_message_control.result.json | 7 ++ .../setup_cfg_with_message_control.cfg | 5 ++ ...setup_cfg_with_message_control.result.json | 7 ++ .../toml_with_message_control.result.json | 7 ++ .../toml/toml_with_message_control.toml | 7 ++ tests/config/test_config.py | 47 ---------- .../config/test_functional_config_loading.py | 79 +++++++++++++++++ 9 files changed, 202 insertions(+), 47 deletions(-) create mode 100644 pylint/testutils/configuration_test.py create mode 100644 tests/config/functional/ini/pylintrc_with_message_control.ini create mode 100644 tests/config/functional/ini/pylintrc_with_message_control.result.json create mode 100644 tests/config/functional/setup_cfg/setup_cfg_with_message_control.cfg create mode 100644 tests/config/functional/setup_cfg/setup_cfg_with_message_control.result.json create mode 100644 tests/config/functional/toml/toml_with_message_control.result.json create mode 100644 tests/config/functional/toml/toml_with_message_control.toml create mode 100644 tests/config/test_functional_config_loading.py diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py new file mode 100644 index 0000000000..00d828b08e --- /dev/null +++ b/pylint/testutils/configuration_test.py @@ -0,0 +1,85 @@ +# 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 function for configuration testing.""" +import copy +import json +import unittest +from pathlib import Path +from typing import Any, Dict, Tuple, Union + +from pylint.lint import Run + +USER_SPECIFIC = str(Path(__file__).parent.parent.parent) + + +def get_expected_or_default(pyproject_toml_path: str, suffix: str, default: Any) -> str: + def get_path_according_to_suffix() -> Path: + path = Path(pyproject_toml_path) + 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() + return expected + + +EXPECTED_CONF_APPEND_KEY = "functional_append" +EXPECTED_CONF_REMOVE_KEY = "functional_remove" +EXPECTED_CONF_SPECIAL_KEYS = [EXPECTED_CONF_APPEND_KEY, EXPECTED_CONF_REMOVE_KEY] + + +def get_expected_configuration( + configuration_path: str, default_configuration: Dict[str, Any] +) -> Dict[str, Any]: + """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 not in EXPECTED_CONF_SPECIAL_KEYS: + result[key] = value + if EXPECTED_CONF_APPEND_KEY in to_override: + for key, value in to_override[EXPECTED_CONF_APPEND_KEY].items(): + result[key] += value + if EXPECTED_CONF_REMOVE_KEY in to_override: + for key, value in to_override[EXPECTED_CONF_REMOVE_KEY].items(): + result[key] -= value + return result + + +def get_expected_output(configuration_path: str) -> Tuple[int, str]: + """Get the expected output of a functional test.""" + + def get_relative_path(path: str) -> str: + """Get the relative path we want without user specific information""" + return path.replace(USER_SPECIFIC, "")[1:] + + output = get_expected_or_default(configuration_path, suffix="out", default="") + exit_code = 2 if output else 0 + return exit_code, output.format( + abspath=configuration_path, relpath=get_relative_path(configuration_path) + ) + + +def run_using_a_configuration_file( + configuration_path: Union[Path, str], file_to_lint: str = __file__ +) -> Tuple[unittest.mock.Mock, unittest.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] + # 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`. + 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 diff --git a/tests/config/functional/ini/pylintrc_with_message_control.ini b/tests/config/functional/ini/pylintrc_with_message_control.ini new file mode 100644 index 0000000000..95d99036f6 --- /dev/null +++ b/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 diff --git a/tests/config/functional/ini/pylintrc_with_message_control.result.json b/tests/config/functional/ini/pylintrc_with_message_control.result.json new file mode 100644 index 0000000000..21938c3192 --- /dev/null +++ b/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 +} diff --git a/tests/config/functional/setup_cfg/setup_cfg_with_message_control.cfg b/tests/config/functional/setup_cfg/setup_cfg_with_message_control.cfg new file mode 100644 index 0000000000..d5e4c564c7 --- /dev/null +++ b/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 diff --git a/tests/config/functional/setup_cfg/setup_cfg_with_message_control.result.json b/tests/config/functional/setup_cfg/setup_cfg_with_message_control.result.json new file mode 100644 index 0000000000..21938c3192 --- /dev/null +++ b/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 +} diff --git a/tests/config/functional/toml/toml_with_message_control.result.json b/tests/config/functional/toml/toml_with_message_control.result.json new file mode 100644 index 0000000000..21938c3192 --- /dev/null +++ b/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 +} diff --git a/tests/config/functional/toml/toml_with_message_control.toml b/tests/config/functional/toml/toml_with_message_control.toml new file mode 100644 index 0000000000..0e58d89186 --- /dev/null +++ b/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" diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 8ce5d9b425..723d4636e6 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -46,53 +46,6 @@ 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 diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py new file mode 100644 index 0000000000..5d3696c2fc --- /dev/null +++ b/tests/config/test_functional_config_loading.py @@ -0,0 +1,79 @@ +# 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 + +""" +This launch the configuration functional tests. This permit to test configuration +files by providing a file with the appropriate extension in the ``tests/config/functional`` +directory. + +Let's say you have a regression_list_crash.toml file to test. Then if there is an error in the conf, +add ``regression_list_crash.out`` alongside your file with the expected output of pylint in it. Use +``{relpath}`` and ``{abspath}`` for the path of the file. The exit code will have to be 2 (error) +if this file exists. + +You must also define a ``regression_list_crash.result.json`` if you want to check the parsed configuration. +This file will be loaded as a dict and will override the default value of the default pylint +configuration. If you need to append or remove a value use the special key ``"functional_append"`` +and ``"functional_remove":``. Check the existing code for example. +""" + +# pylint: disable=redefined-outer-name + +from pathlib import Path +from typing import Any, Dict + +import pytest +from _pytest.capture import CaptureFixture + +from pylint.testutils.configuration_test import ( + get_expected_configuration, + get_expected_output, + run_using_a_configuration_file, +) + +HERE = Path(__file__).parent +FUNCTIONAL_DIR = HERE / "functional" +# We use string then recast to path so we can use -k in pytest otherwise +# we get 'configuration_path0' as a test name. Relative to the functional +# directory because otherwise the string is very lengthy +CONFIGURATION_PATHS = [ + str(path.relative_to(FUNCTIONAL_DIR)) + for ext in ("toml", "ini", "cfg") + for path in FUNCTIONAL_DIR.rglob(f"*.{ext}") +] +FILE_TO_LINT = str(HERE / "file_to_lint.py") + + +@pytest.fixture() +def default_configuration(tmp_path: Path) -> Dict[str, Any]: + empty_pylintrc = tmp_path / "pylintrc" + empty_pylintrc.write_text("") + mock_exit, _, runner = run_using_a_configuration_file( + str(empty_pylintrc), FILE_TO_LINT + ) + mock_exit.assert_called_once_with(0) + return runner.linter.config.__dict__ + + +@pytest.mark.parametrize("configuration_path", CONFIGURATION_PATHS) +def test_functional_config_loading( + configuration_path: str, + default_configuration: Dict[str, Any], + capsys: CaptureFixture, +): + """Functional tests for configurations.""" + configuration_path = str(FUNCTIONAL_DIR / configuration_path) + msg = f"Wrong result with configuration {configuration_path}" + expected_code, expected_output = get_expected_output(configuration_path) + expected_loaded_configuration = get_expected_configuration( + configuration_path, default_configuration + ) + mock_exit, _, runner = run_using_a_configuration_file( + configuration_path, FILE_TO_LINT + ) + mock_exit.assert_called_once_with(expected_code) + out, err = capsys.readouterr() + # rstrip() applied so we can have a final newline in the expected test file + assert expected_output.rstrip() == out.rstrip(), msg + assert expected_loaded_configuration == runner.linter.config.__dict__ + assert not err, msg From 88d08e25c41c8a7b937058c452cfdec25700b1e1 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Wed, 10 Nov 2021 10:10:14 +0100 Subject: [PATCH 02/23] Better handling of removal in functional conf tests --- pylint/testutils/configuration_test.py | 9 +++++++-- .../toml/toml_with_enable.result.json | 9 +++++++++ .../functional/toml/toml_with_enable.toml | 5 +++++ tests/config/test_functional_config_loading.py | 18 ++++++++++++++++-- 4 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 tests/config/functional/toml/toml_with_enable.result.json create mode 100644 tests/config/functional/toml/toml_with_enable.toml diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 00d828b08e..723e4afa34 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -7,6 +7,7 @@ import unittest from pathlib import Path from typing import Any, Dict, Tuple, Union +from unittest.mock import Mock from pylint.lint import Run @@ -48,7 +49,11 @@ def get_expected_configuration( result[key] += value if EXPECTED_CONF_REMOVE_KEY in to_override: for key, value in to_override[EXPECTED_CONF_REMOVE_KEY].items(): - result[key] -= value + new_value = [] + for old_value in result[key]: + if old_value not in value: + new_value.append(old_value) + result[key] = new_value return result @@ -68,7 +73,7 @@ def get_relative_path(path: str) -> str: def run_using_a_configuration_file( configuration_path: Union[Path, str], file_to_lint: str = __file__ -) -> Tuple[unittest.mock.Mock, unittest.mock.Mock, Run]: +) -> 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] diff --git a/tests/config/functional/toml/toml_with_enable.result.json b/tests/config/functional/toml/toml_with_enable.result.json new file mode 100644 index 0000000000..0bdbc840d0 --- /dev/null +++ b/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"]] + } +} diff --git a/tests/config/functional/toml/toml_with_enable.toml b/tests/config/functional/toml/toml_with_enable.toml new file mode 100644 index 0000000000..a1e7b65af5 --- /dev/null +++ b/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" diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 5d3696c2fc..f839214385 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -18,12 +18,13 @@ """ # pylint: disable=redefined-outer-name - +import logging from pathlib import Path from typing import Any, Dict import pytest from _pytest.capture import CaptureFixture +from _pytest.logging import LogCaptureFixture from pylint.testutils.configuration_test import ( get_expected_configuration, @@ -60,8 +61,10 @@ def test_functional_config_loading( configuration_path: str, default_configuration: Dict[str, Any], capsys: CaptureFixture, + caplog: LogCaptureFixture, ): """Functional tests for configurations.""" + caplog.set_level(logging.INFO) configuration_path = str(FUNCTIONAL_DIR / configuration_path) msg = f"Wrong result with configuration {configuration_path}" expected_code, expected_output = get_expected_output(configuration_path) @@ -75,5 +78,16 @@ def test_functional_config_loading( out, err = capsys.readouterr() # rstrip() applied so we can have a final newline in the expected test file assert expected_output.rstrip() == out.rstrip(), msg - assert expected_loaded_configuration == runner.linter.config.__dict__ + assert sorted(expected_loaded_configuration.keys()) == sorted( + runner.linter.config.__dict__.keys() + ), msg + for key in expected_loaded_configuration: + key_msg = f"{msg} for key '{key}':" + expected_value = expected_loaded_configuration[key] + if isinstance(expected_value, list): + assert sorted(expected_value) == sorted( + runner.linter.config.__dict__[key] + ), key_msg + else: + assert expected_value == runner.linter.config.__dict__[key], key_msg assert not err, msg From a4507960d147f3a04d51dd1bf3ce27a6692f419d Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Wed, 10 Nov 2021 10:39:58 +0100 Subject: [PATCH 03/23] Simplify a list creation in OptionParser() --- pylint/config/option_parser.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pylint/config/option_parser.py b/pylint/config/option_parser.py index 16d1ea87ec..66b5737234 100644 --- a/pylint/config/option_parser.py +++ b/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)) From a26e3563975ca4656b87ba3679c720accb15f919 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Wed, 10 Nov 2021 18:41:07 +0100 Subject: [PATCH 04/23] Easier understanding of what happen for the expected result --- pylint/testutils/configuration_test.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 723e4afa34..d3d4550f65 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -4,6 +4,7 @@ """Utility function for configuration testing.""" import copy import json +import logging import unittest from pathlib import Path from typing import Any, Dict, Tuple, Union @@ -24,6 +25,9 @@ def get_path_according_to_suffix() -> Path: if expected_result_path.exists(): with open(expected_result_path, encoding="utf8") as f: expected = f.read() + logging.info("%s exists.", expected_result_path) + else: + logging.info("%s not found, using '%s'.", expected_result_path, default) return expected @@ -65,7 +69,14 @@ def get_relative_path(path: str) -> str: return path.replace(USER_SPECIFIC, "")[1:] output = get_expected_or_default(configuration_path, suffix="out", default="") - exit_code = 2 if output else 0 + if output: + 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=get_relative_path(configuration_path) ) From a8e61d360e29984cd104efb8593e454c2913fce3 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Thu, 11 Nov 2021 19:16:42 +0100 Subject: [PATCH 05/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- pylint/testutils/configuration_test.py | 8 ++++---- tests/config/test_functional_config_loading.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index d3d4550f65..01907332c5 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -1,7 +1,7 @@ # 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 function for configuration testing.""" +"""Utility functions for configuration testing.""" import copy import json import logging @@ -65,7 +65,7 @@ def get_expected_output(configuration_path: str) -> Tuple[int, str]: """Get the expected output of a functional test.""" def get_relative_path(path: str) -> str: - """Get the relative path we want without user specific information""" + """Get the relative path we want without the user specific path""" return path.replace(USER_SPECIFIC, "")[1:] output = get_expected_or_default(configuration_path, suffix="out", default="") @@ -88,10 +88,10 @@ def run_using_a_configuration_file( """Simulate a run with a configuration without really launching the checks.""" configuration_path = str(configuration_path) args = ["--rcfile", configuration_path, file_to_lint] - # If we used `pytest.raises(SystemExit)`, the `runner` variable + # 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. Do not mock + # 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`. diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index f839214385..25373195c3 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -14,7 +14,7 @@ You must also define a ``regression_list_crash.result.json`` if you want to check the parsed configuration. This file will be loaded as a dict and will override the default value of the default pylint configuration. If you need to append or remove a value use the special key ``"functional_append"`` -and ``"functional_remove":``. Check the existing code for example. +and ``"functional_remove":``. Check the existing code for examples. """ # pylint: disable=redefined-outer-name @@ -34,9 +34,9 @@ HERE = Path(__file__).parent FUNCTIONAL_DIR = HERE / "functional" -# We use string then recast to path so we can use -k in pytest otherwise -# we get 'configuration_path0' as a test name. Relative to the functional -# directory because otherwise the string is very lengthy +# We use string then recast to path so we can use -k in pytest. +# Otherwise we get 'configuration_path0' as a test name. The path is relative to the functional +# directory because otherwise the string would be very lengthy. CONFIGURATION_PATHS = [ str(path.relative_to(FUNCTIONAL_DIR)) for ext in ("toml", "ini", "cfg") From 476ce6b3240b1c0c166f47ee3718b10251110aec Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:18:26 +0100 Subject: [PATCH 06/23] Update pylint/testutils/configuration_test.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- pylint/testutils/configuration_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 01907332c5..f861d4640a 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -12,7 +12,7 @@ from pylint.lint import Run -USER_SPECIFIC = str(Path(__file__).parent.parent.parent) +USER_SPECIFIC_PATH = str(Path(__file__).parent.parent.parent) def get_expected_or_default(pyproject_toml_path: str, suffix: str, default: Any) -> str: @@ -66,7 +66,8 @@ def get_expected_output(configuration_path: str) -> Tuple[int, str]: def get_relative_path(path: str) -> str: """Get the relative path we want without the user specific path""" - return path.replace(USER_SPECIFIC, "")[1:] + # Second [1:] is to remove the closing '/' + return "".join(path.split(USER_SPECIFIC_PATH)[1:][1:]) output = get_expected_or_default(configuration_path, suffix="out", default="") if output: From 2146e3dd4e42639fe6a7cd610d81dd20b0caba9f Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:27:03 +0100 Subject: [PATCH 07/23] Move test_can_read_toml_rich_types to functional framework --- .../functional/toml/rich_types.result.json | 7 +++++++ tests/config/functional/toml/rich_types.toml | 11 ++++++++++ tests/config/test_config.py | 20 ------------------- 3 files changed, 18 insertions(+), 20 deletions(-) create mode 100644 tests/config/functional/toml/rich_types.result.json create mode 100644 tests/config/functional/toml/rich_types.toml diff --git a/tests/config/functional/toml/rich_types.result.json b/tests/config/functional/toml/rich_types.result.json new file mode 100644 index 0000000000..21938c3192 --- /dev/null +++ b/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 +} diff --git a/tests/config/functional/toml/rich_types.toml b/tests/config/functional/toml/rich_types.toml new file mode 100644 index 0000000000..91178390ec --- /dev/null +++ b/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 diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 723d4636e6..47febda326 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -46,26 +46,6 @@ def check_configuration_file_reader( assert bool(runner.linter.config.reports) == expected_reports_truthey -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: """We can read and open a properly formatted toml file.""" config_file = tmp_path / "pyproject.toml" From 0676bfb8d868506278268cb4e0317d8dc3ecfc6e Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:34:15 +0100 Subject: [PATCH 08/23] Factorize duplicated code from test_config using the new testutil --- tests/config/conftest.py | 10 +++++ tests/config/test_config.py | 37 ++++--------------- .../config/test_functional_config_loading.py | 8 ++-- 3 files changed, 22 insertions(+), 33 deletions(-) create mode 100644 tests/config/conftest.py diff --git a/tests/config/conftest.py b/tests/config/conftest.py new file mode 100644 index 0000000000..5a6778af2f --- /dev/null +++ b/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") diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 47febda326..f89e624168 100644 --- a/tests/config/test_config.py +++ b/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,7 +22,7 @@ def check_configuration_file_reader( assert bool(runner.linter.config.reports) == expected_reports_truthey -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( @@ -59,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) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 25373195c3..28b4a60941 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -42,15 +42,14 @@ for ext in ("toml", "ini", "cfg") for path in FUNCTIONAL_DIR.rglob(f"*.{ext}") ] -FILE_TO_LINT = str(HERE / "file_to_lint.py") @pytest.fixture() -def default_configuration(tmp_path: Path) -> Dict[str, Any]: +def default_configuration(tmp_path: Path, file_to_lint_path: str) -> Dict[str, Any]: empty_pylintrc = tmp_path / "pylintrc" empty_pylintrc.write_text("") mock_exit, _, runner = run_using_a_configuration_file( - str(empty_pylintrc), FILE_TO_LINT + str(empty_pylintrc), file_to_lint_path ) mock_exit.assert_called_once_with(0) return runner.linter.config.__dict__ @@ -60,6 +59,7 @@ def default_configuration(tmp_path: Path) -> Dict[str, Any]: def test_functional_config_loading( configuration_path: str, default_configuration: Dict[str, Any], + file_to_lint_path: str, capsys: CaptureFixture, caplog: LogCaptureFixture, ): @@ -72,7 +72,7 @@ def test_functional_config_loading( configuration_path, default_configuration ) mock_exit, _, runner = run_using_a_configuration_file( - configuration_path, FILE_TO_LINT + configuration_path, file_to_lint_path ) mock_exit.assert_called_once_with(expected_code) out, err = capsys.readouterr() From e9c89f4ad167932f4b2e9de9abd82f05040220dc Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:41:18 +0100 Subject: [PATCH 09/23] More precise typing for Configuration type when Any is used --- pylint/testutils/configuration_test.py | 10 ++++++++-- tests/config/test_functional_config_loading.py | 8 +++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index f861d4640a..12798a3564 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -13,9 +13,15 @@ from pylint.lint import Run USER_SPECIFIC_PATH = str(Path(__file__).parent.parent.parent) +# We use Any in this typing because the configuration contain real object +# that could be a lot of things. +ConfigurationValue = Any +PylintConfiguration = Dict[str, ConfigurationValue] -def get_expected_or_default(pyproject_toml_path: str, suffix: str, default: Any) -> str: +def get_expected_or_default( + pyproject_toml_path: str, suffix: str, default: ConfigurationValue +) -> str: def get_path_according_to_suffix() -> Path: path = Path(pyproject_toml_path) return path.parent / f"{path.stem}.{suffix}" @@ -37,7 +43,7 @@ def get_path_according_to_suffix() -> Path: def get_expected_configuration( - configuration_path: str, default_configuration: Dict[str, Any] + configuration_path: str, default_configuration: PylintConfiguration ) -> Dict[str, Any]: """Get the expected parsed configuration of a configuration functional test""" result = copy.deepcopy(default_configuration) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 28b4a60941..6727bdb8fd 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -20,13 +20,13 @@ # pylint: disable=redefined-outer-name import logging from pathlib import Path -from typing import Any, Dict import pytest from _pytest.capture import CaptureFixture from _pytest.logging import LogCaptureFixture from pylint.testutils.configuration_test import ( + PylintConfiguration, get_expected_configuration, get_expected_output, run_using_a_configuration_file, @@ -45,7 +45,9 @@ @pytest.fixture() -def default_configuration(tmp_path: Path, file_to_lint_path: str) -> Dict[str, Any]: +def default_configuration( + tmp_path: Path, file_to_lint_path: str +) -> PylintConfiguration: empty_pylintrc = tmp_path / "pylintrc" empty_pylintrc.write_text("") mock_exit, _, runner = run_using_a_configuration_file( @@ -58,7 +60,7 @@ def default_configuration(tmp_path: Path, file_to_lint_path: str) -> Dict[str, A @pytest.mark.parametrize("configuration_path", CONFIGURATION_PATHS) def test_functional_config_loading( configuration_path: str, - default_configuration: Dict[str, Any], + default_configuration: PylintConfiguration, file_to_lint_path: str, capsys: CaptureFixture, caplog: LogCaptureFixture, From 38b7c410d1a926f67bd5a35d297c9fa1d84d108a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:45:20 +0100 Subject: [PATCH 10/23] Create a constant for accepted configuration extension --- tests/config/test_functional_config_loading.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 6727bdb8fd..437ae50faf 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -37,9 +37,10 @@ # We use string then recast to path so we can use -k in pytest. # Otherwise we get 'configuration_path0' as a test name. The path is relative to the functional # directory because otherwise the string would be very lengthy. +ACCEPTED_CONFIGURATION_EXTENSIONS = ("toml", "ini", "cfg") CONFIGURATION_PATHS = [ str(path.relative_to(FUNCTIONAL_DIR)) - for ext in ("toml", "ini", "cfg") + for ext in ACCEPTED_CONFIGURATION_EXTENSIONS for path in FUNCTIONAL_DIR.rglob(f"*.{ext}") ] From 58a1d660c00442ec3d8db92d59a457cf89a232de Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 08:54:21 +0100 Subject: [PATCH 11/23] Simplification of for loop for functional conf test following review See https://github.com/PyCQA/pylint/pull/5287#discussion_r747301970 --- pylint/testutils/configuration_test.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 12798a3564..3bb63bf29c 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -39,7 +39,6 @@ def get_path_according_to_suffix() -> Path: EXPECTED_CONF_APPEND_KEY = "functional_append" EXPECTED_CONF_REMOVE_KEY = "functional_remove" -EXPECTED_CONF_SPECIAL_KEYS = [EXPECTED_CONF_APPEND_KEY, EXPECTED_CONF_REMOVE_KEY] def get_expected_configuration( @@ -52,18 +51,18 @@ def get_expected_configuration( ) to_override = json.loads(config_as_json) for key, value in to_override.items(): - if key not in EXPECTED_CONF_SPECIAL_KEYS: + 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 - if EXPECTED_CONF_APPEND_KEY in to_override: - for key, value in to_override[EXPECTED_CONF_APPEND_KEY].items(): - result[key] += value - if EXPECTED_CONF_REMOVE_KEY in to_override: - for key, value in to_override[EXPECTED_CONF_REMOVE_KEY].items(): - new_value = [] - for old_value in result[key]: - if old_value not in value: - new_value.append(old_value) - result[key] = new_value return result From 2a98bb089860cb0f1151543ff9a9e342fb34ea9d Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 09:01:25 +0100 Subject: [PATCH 12/23] Better explanation on why we use logging following review See https://github.com/PyCQA/pylint/pull/5287#discussion_r746970574 --- pylint/testutils/configuration_test.py | 6 ++++++ tests/config/test_functional_config_loading.py | 2 ++ 2 files changed, 8 insertions(+) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 3bb63bf29c..2ff8ae0f22 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -31,6 +31,9 @@ def get_path_according_to_suffix() -> Path: if expected_result_path.exists(): with open(expected_result_path, encoding="utf8") as f: expected = f.read() + # logging is helpful to 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) @@ -76,6 +79,9 @@ def get_relative_path(path: str) -> str: 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 ) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 437ae50faf..48abe8191b 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -67,6 +67,8 @@ def test_functional_config_loading( caplog: LogCaptureFixture, ): """Functional tests for configurations.""" + # logging is helpful to see what's expected and why. The output of the + # program is checked during the test so printing messes with the result. caplog.set_level(logging.INFO) configuration_path = str(FUNCTIONAL_DIR / configuration_path) msg = f"Wrong result with configuration {configuration_path}" From 774c37303746016ae1e73ec84c1deb4e344171a7 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 09:05:45 +0100 Subject: [PATCH 13/23] Add a docstring to get_expected_or_default following review See https://github.com/PyCQA/pylint/pull/5287\#discussion_r746971021 --- pylint/testutils/configuration_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 2ff8ae0f22..9c3c6349d4 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -22,6 +22,8 @@ def get_expected_or_default( pyproject_toml_path: 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(pyproject_toml_path) return path.parent / f"{path.stem}.{suffix}" From 47dae6a432b060bfc44c6e93e5703cdc8d307f7b Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 09:09:56 +0100 Subject: [PATCH 14/23] Simplification of for loop on dict by using items() --- tests/config/test_functional_config_loading.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 48abe8191b..73b21c3967 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -86,9 +86,8 @@ def test_functional_config_loading( assert sorted(expected_loaded_configuration.keys()) == sorted( runner.linter.config.__dict__.keys() ), msg - for key in expected_loaded_configuration: + for key, expected_value in expected_loaded_configuration.items(): key_msg = f"{msg} for key '{key}':" - expected_value = expected_loaded_configuration[key] if isinstance(expected_value, list): assert sorted(expected_value) == sorted( runner.linter.config.__dict__[key] From 388a2db19f5685dff4e59a8417d165a954846a88 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 09:11:46 +0100 Subject: [PATCH 15/23] Rename variable that was still named for toml only --- pylint/testutils/configuration_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 9c3c6349d4..e5a04001d7 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -20,12 +20,12 @@ def get_expected_or_default( - pyproject_toml_path: str, suffix: str, default: ConfigurationValue + 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(pyproject_toml_path) + path = Path(tested_configuration_file) return path.parent / f"{path.stem}.{suffix}" expected = default From b1f3b41ffd809b52802e40debcf4e6c9bf799fcc Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 09:34:47 +0100 Subject: [PATCH 16/23] Add a regression test for #4746 This permits to introduce an example of configuration file with an error. --- pylint/config/option_manager_mixin.py | 3 +++ pylint/testutils/configuration_test.py | 9 ++++----- .../toml/issue_4746/loaded_plugin_does_not_exists.out | 2 ++ .../issue_4746/loaded_plugin_does_not_exists.result.json | 3 +++ .../toml/issue_4746/loaded_plugin_does_not_exists.toml | 8 ++++++++ 5 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.out create mode 100644 tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.result.json create mode 100644 tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml diff --git a/pylint/config/option_manager_mixin.py b/pylint/config/option_manager_mixin.py index 6720bc40e9..924998e776 100644 --- a/pylint/config/option_manager_mixin.py +++ b/pylint/config/option_manager_mixin.py @@ -271,6 +271,9 @@ 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: + mod_name = config_file + self.current_name = mod_name + self.set_current_module(mod_name) parser = self.cfgfile_parser if config_file.endswith(".toml"): self._parse_toml(config_file, parser) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index e5a04001d7..7000a660cc 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -76,8 +76,8 @@ def get_expected_output(configuration_path: str) -> Tuple[int, str]: def get_relative_path(path: str) -> str: """Get the relative path we want without the user specific path""" - # Second [1:] is to remove the closing '/' - return "".join(path.split(USER_SPECIFIC_PATH)[1:][1:]) + # [1:] is to remove the opening '/' + return path.split(USER_SPECIFIC_PATH)[1][1:] output = get_expected_or_default(configuration_path, suffix="out", default="") if output: @@ -91,9 +91,8 @@ def get_relative_path(path: str) -> str: 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=get_relative_path(configuration_path) - ) + relpath = get_relative_path(configuration_path) + return exit_code, output.format(abspath=configuration_path, relpath=relpath) def run_using_a_configuration_file( diff --git a/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.out b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.out new file mode 100644 index 0000000000..a6837722ad --- /dev/null +++ b/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) diff --git a/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.result.json b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.result.json new file mode 100644 index 0000000000..0b6175c4a7 --- /dev/null +++ b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.result.json @@ -0,0 +1,3 @@ +{ + "load_plugins": ["pylint_websockets"] +} diff --git a/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml new file mode 100644 index 0000000000..ea79aa3a5e --- /dev/null +++ b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml @@ -0,0 +1,8 @@ +# The pylint_websockets plugin does not exist and therefore this toml is invalid +[tool.poe.tasks] +docs = {cmd = "sphinx-build docs build", help = "Build documentation"} + +[tool.pylint.MASTER] +load-plugins = 'pylint_websockets' + +format = ["black", "isort"] From 59e119d17cf7ed8f69927cbd93079580b80c8982 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 16:44:53 +0100 Subject: [PATCH 17/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- pylint/testutils/configuration_test.py | 4 ++-- tests/config/test_functional_config_loading.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 7000a660cc..6273505928 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -13,7 +13,7 @@ from pylint.lint import Run USER_SPECIFIC_PATH = str(Path(__file__).parent.parent.parent) -# We use Any in this typing because the configuration contain real object +# 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] @@ -33,7 +33,7 @@ def get_path_according_to_suffix() -> Path: if expected_result_path.exists(): with open(expected_result_path, encoding="utf8") as f: expected = f.read() - # logging is helpful to to realize your file is not taken into + # 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) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 73b21c3967..3f18c8ccd3 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -2,7 +2,7 @@ # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE """ -This launch the configuration functional tests. This permit to test configuration +This launches the configuration functional tests. This permits to test configuration files by providing a file with the appropriate extension in the ``tests/config/functional`` directory. From a08e38e505232810db491d3111d9e23f77204701 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 16:48:30 +0100 Subject: [PATCH 18/23] Add proper typing in configuration test following review See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748064197 --- pylint/testutils/configuration_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index 6273505928..fc0842be63 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -48,7 +48,7 @@ def get_path_according_to_suffix() -> Path: def get_expected_configuration( configuration_path: str, default_configuration: PylintConfiguration -) -> Dict[str, Any]: +) -> PylintConfiguration: """Get the expected parsed configuration of a configuration functional test""" result = copy.deepcopy(default_configuration) config_as_json = get_expected_or_default( From 7fac09bee12c4cf68aa527a37778df3c1e00f827 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 16:51:02 +0100 Subject: [PATCH 19/23] Remove option that are not required for the test itself See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748118048 --- .../toml/issue_4746/loaded_plugin_does_not_exists.toml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml index ea79aa3a5e..c015a94486 100644 --- a/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml +++ b/tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml @@ -1,8 +1,3 @@ # The pylint_websockets plugin does not exist and therefore this toml is invalid -[tool.poe.tasks] -docs = {cmd = "sphinx-build docs build", help = "Build documentation"} - [tool.pylint.MASTER] load-plugins = 'pylint_websockets' - -format = ["black", "isort"] From 4a50fa5b7e0297181ee99ab668dcfc6ef317d151 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 16:54:03 +0100 Subject: [PATCH 20/23] Proper import for python import of CaptureFixture See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748122852 --- tests/config/test_functional_config_loading.py | 3 +-- tests/lint/test_pylinter.py | 2 +- tests/lint/unittest_lint.py | 2 +- tests/message/unittest_message_definition_store.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/config/test_functional_config_loading.py b/tests/config/test_functional_config_loading.py index 3f18c8ccd3..452d7cc902 100644 --- a/tests/config/test_functional_config_loading.py +++ b/tests/config/test_functional_config_loading.py @@ -22,8 +22,7 @@ from pathlib import Path import pytest -from _pytest.capture import CaptureFixture -from _pytest.logging import LogCaptureFixture +from pytest import CaptureFixture, LogCaptureFixture from pylint.testutils.configuration_test import ( PylintConfiguration, diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py index 900d53fb37..15693d0249 100644 --- a/tests/lint/test_pylinter.py +++ b/tests/lint/test_pylinter.py @@ -2,9 +2,9 @@ from typing import Any from unittest.mock import patch -from _pytest.capture import CaptureFixture from astroid import AstroidBuildingError from py._path.local import LocalPath # type: ignore +from pytest import CaptureFixture from pylint.lint.pylinter import PyLinter from pylint.utils import FileState diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index ad9b28fee0..a167b03fd8 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -52,7 +52,7 @@ import platformdirs import pytest -from _pytest.capture import CaptureFixture +from pytest import CaptureFixture from pylint import checkers, config, exceptions, interfaces, lint, testutils from pylint.checkers.utils import check_messages diff --git a/tests/message/unittest_message_definition_store.py b/tests/message/unittest_message_definition_store.py index 4d108489e7..e13163080a 100644 --- a/tests/message/unittest_message_definition_store.py +++ b/tests/message/unittest_message_definition_store.py @@ -5,7 +5,7 @@ from io import StringIO import pytest -from _pytest.capture import CaptureFixture +from pytest import CaptureFixture from pylint.checkers import BaseChecker from pylint.exceptions import InvalidMessageError, UnknownMessageError From dddaf0d7209f5c41ab8ecc13785d4f1a46aaf367 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 17:05:58 +0100 Subject: [PATCH 21/23] Use the existing function for relative path instead of a custom one --- pylint/testutils/configuration_test.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/pylint/testutils/configuration_test.py b/pylint/testutils/configuration_test.py index fc0842be63..2fac8f1c15 100644 --- a/pylint/testutils/configuration_test.py +++ b/pylint/testutils/configuration_test.py @@ -12,7 +12,7 @@ from pylint.lint import Run -USER_SPECIFIC_PATH = str(Path(__file__).parent.parent.parent) +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 @@ -73,12 +73,6 @@ def get_expected_configuration( def get_expected_output(configuration_path: str) -> Tuple[int, str]: """Get the expected output of a functional test.""" - - def get_relative_path(path: str) -> str: - """Get the relative path we want without the user specific path""" - # [1:] is to remove the opening '/' - return path.split(USER_SPECIFIC_PATH)[1][1:] - 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. @@ -91,8 +85,10 @@ def get_relative_path(path: str) -> str: else: logging.info(".out file does not exists, so the expected exit code is 0") exit_code = 0 - relpath = get_relative_path(configuration_path) - return exit_code, output.format(abspath=configuration_path, relpath=relpath) + return exit_code, output.format( + abspath=configuration_path, + relpath=Path(configuration_path).relative_to(USER_SPECIFIC_PATH), + ) def run_using_a_configuration_file( From 10135bb56441f8fd8b0a43ff383283fbe8bdd44a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 17:58:40 +0100 Subject: [PATCH 22/23] Simplification in OptionManagerMixin following review See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748115210 --- pylint/config/option_manager_mixin.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pylint/config/option_manager_mixin.py b/pylint/config/option_manager_mixin.py index 924998e776..3595d466d9 100644 --- a/pylint/config/option_manager_mixin.py +++ b/pylint/config/option_manager_mixin.py @@ -271,9 +271,8 @@ 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: - mod_name = config_file - self.current_name = mod_name - self.set_current_module(mod_name) + self.current_name = config_file + self.set_current_module(self.current_name) parser = self.cfgfile_parser if config_file.endswith(".toml"): self._parse_toml(config_file, parser) From a4a5a3a3a2a003c2590945517852b11a2fb45d4a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Nov 2021 21:22:43 +0100 Subject: [PATCH 23/23] Update pylint/config/option_manager_mixin.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- pylint/config/option_manager_mixin.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pylint/config/option_manager_mixin.py b/pylint/config/option_manager_mixin.py index 3595d466d9..1871e7fd6c 100644 --- a/pylint/config/option_manager_mixin.py +++ b/pylint/config/option_manager_mixin.py @@ -271,8 +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.current_name = config_file - self.set_current_module(self.current_name) + self.set_current_module(config_file) parser = self.cfgfile_parser if config_file.endswith(".toml"): self._parse_toml(config_file, parser)