Skip to content

Commit

Permalink
Puts issue pylint-dev#4118 under test
Browse files Browse the repository at this point in the history
Taken in isolation, this commit may appear heavy-weight. The intent here
is to have a linter run and retain all state information about that run
so we can inspect what happened.

What this commit shows is that the checkers the linter knows about have
the right config, but still multi-job [MJ] runs are incorrect.
    see: assert checker.config.min_similarity_lines == min_similarity_lines

I think the MJ failiures are because SimilarChecker.reduce_map_data()
constructs a new SimilarChecker and then the linter doesn't call
set_option().

The process by which the set_option() calls are made in the normal run
is opaque to me, but perhaps someone else has a better idea(?).
  • Loading branch information
doublethefish committed Mar 2, 2021
1 parent 80da123 commit 1b477cd
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 14 deletions.
102 changes: 98 additions & 4 deletions tests/checkers/unittest_similar.py
Expand Up @@ -16,11 +16,13 @@
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/PyCQA/pylint/blob/master/COPYING

import os.path
from contextlib import redirect_stdout
from io import StringIO
from pathlib import Path

import pytest
from test_config import run_with_config_file

from pylint.checkers import similar
from pylint.lint import PyLinter
Expand All @@ -33,6 +35,8 @@
SIMILAR4 = str(INPUT / "similar4")
MULTILINE = str(INPUT / "multiline-import")
HIDE_CODE_WITH_IMPORTS = str(INPUT / "hide_code_with_imports.py")
SIMILAR_A = str(INPUT / "similar_lines_a.py")
SIMILAR_B = str(INPUT / "similar_lines_b.py")


def test_ignore_comments():
Expand Down Expand Up @@ -247,10 +251,7 @@ def test_get_map_data():
# Add a parallel checker to ensure it can map and reduce
linter.register_checker(similar.SimilarChecker(linter))

source_streams = (
str(INPUT / "similar_lines_a.py"),
str(INPUT / "similar_lines_b.py"),
)
source_streams = ()
expected_linelists = (
(
"",
Expand Down Expand Up @@ -375,3 +376,96 @@ def test_get_map_data():
# There doesn't seem to be a faster way of doing this, yet.
lines = (line for idx, line in lineset_obj.enumerate_stripped())
assert tuple(expected_lines) == tuple(lines)


def _simcheck_with_config(num_jobs, min_similarity_lines, tmp_path):
"""Runs similarity-checks for a given number of jobs and min_lines via rc-file
Does not error
A num_jobs value of None does not set any job config at all.
Uses the given pytest tmp_path to write the config file.
"""

# First write the config file configuring the similarity config we want to use
config_file = tmp_path / "setup.cfg"
print(tmp_path)
print(os.listdir(tmp_path))
if num_jobs is None:
# We have differences between setting the config at all and not, allow
# None to singify no job config
jobs_config = ""
else:
jobs_config = f"""
[pylint.messages control]
jobs = {num_jobs}
"""

config = f"""
[MASTER]
persistent=no
{jobs_config}
[SIMILARITIES]
min-similarity-lines={min_similarity_lines}
"""
config_file.write_text(config)

# Get the path to two files that have at least some similarities
source_streams = [
SIMILAR_A,
SIMILAR_B,
]
for fname in source_streams:
assert os.path.exists(fname), f"File not found! {fname}"

args = [
"--disable",
"all", # disable all checks
"--enable",
"similarities", # enable the only checks we care about
"--persistent=no",
] + source_streams

runner, exit_code, stdout = run_with_config_file(config_file, args)
linter = runner.linter
expected_num_jobs = 1 if num_jobs is None else num_jobs
assert linter.config.jobs == expected_num_jobs

checkers = linter.get_checkers()
hitonce = False
for checker in checkers:
if isinstance(checker, similar.SimilarChecker):
assert checker.config.min_similarity_lines == min_similarity_lines
assert not hitonce
hitonce = True
assert hitonce

return exit_code, stdout


@pytest.mark.parametrize("min_similarity_lines", [0, 1, 4, 1000])
def test_configuration_is_passed_to_workers(tmp_path, min_similarity_lines):
"""Tests check_parallel passes the configuration to sub-workers
Partially tests bug #4118 and #4173"""
exit_no_job, stdout_no_job = _simcheck_with_config(
None, min_similarity_lines, tmp_path
)
exit_single, stdout_single = _simcheck_with_config(
1, min_similarity_lines, tmp_path
)
exit_multi, stdout_multi = _simcheck_with_config(
2, min_similarity_lines, tmp_path
)

# Check that each run agrees on the specific similarity errors found
# Each run should be identical
assert stdout_no_job == stdout_single
assert stdout_single == stdout_multi

# Check exit-codes are the same i.e. each of the runs agree on error states.
# This should always be true if the output comparison checks above pass.
assert exit_no_job == exit_single
assert exit_single == exit_multi
50 changes: 40 additions & 10 deletions tests/test_config.py
@@ -1,23 +1,51 @@
# pylint: disable=missing-module-docstring, missing-function-docstring, protected-access
import unittest.mock
from io import StringIO

from test_self import _patch_streams

import pylint.lint


def check_configuration_file_reader(config_file):
"""Initialize pylint with the given configuration file and check that
what we initialized the linter with what was expected.
def run_with_config_file(config_file, args):
"""Initialize and runs pylint with the given configuration filei and args.
Returns tuple of (runner, exit_code)
"""
args = ["--rcfile", str(config_file), __file__]
# prepend args with the rcfile loader
args = ["--rcfile", str(config_file), __file__] + args
out = StringIO()
# 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"):
with _patch_streams(out):
runner = pylint.lint.Run(args)
exit_code = mocked_exit.call_args[0][0]
return runner, exit_code, out.getvalue()


def check_null_runner_with_config_file(config_file, args):
"""Initialize pylint with the given configuration file, but doesn't run checks
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`.
Returns tuple of (runner, exit_code, stdout)
"""
# TODO: annotate why we don't patch check_single_file() or _check_files()
with unittest.mock.patch("pylint.lint.pylinter.check_parallel"):
return run_with_config_file(config_file, args)


def check_configuration_file_reader(config_file):
"""Initialize pylint with the given configuration file and check that
what we initialized the linter with what was expected.
Returns the runner
"""
runner, exit_code, _ = check_null_runner_with_config_file(config_file, [])

# "logging-not-lazy" and "logging-format-interpolation"
expected_disabled = {"W1201", "W1202"}
Expand All @@ -26,7 +54,9 @@ def check_configuration_file_reader(config_file):
assert runner.linter.config.jobs == 10
assert runner.linter.config.reports

mocked_exit.assert_called_once_with(0)
# For the config tests we check that the config parsed properly
assert exit_code == 0

return runner


Expand Down

0 comments on commit 1b477cd

Please sign in to comment.