diff options
author | Pierre Sassoulas <pierre.sassoulas@gmail.com> | 2021-11-24 13:10:52 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-24 13:10:52 +0100 |
commit | 5e9d20dc32768e873ab84bc4ed0b489fdda40672 (patch) | |
tree | 26e1018b2ff6f299b23d93fa3160e0bd704966cd | |
parent | e8fa46928d91893723a9a038fde2d274d535fd1f (diff) | |
download | pylint-git-5e9d20dc32768e873ab84bc4ed0b489fdda40672.tar.gz |
Primer tests "à la mypy" (#5173)
* Add changelog and warning about unstable API in testutil
* Add primer tests, (running pylint on external libs during tests)
In order to anticipate crash/fatal messages, false positives are harder
to anticipate. Follow-up will be #5359 and later on #5364
Add '__tracebackhide__ = True' so the traceback is manageable
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
-rw-r--r-- | .github/workflows/ci.yaml | 34 | ||||
-rw-r--r-- | .gitignore | 1 | ||||
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | doc/whatsnew/2.12.rst | 7 | ||||
-rw-r--r-- | pylint/constants.py | 1 | ||||
-rw-r--r-- | pylint/testutils/primer.py | 107 | ||||
-rw-r--r-- | requirements_test_min.txt | 1 | ||||
-rw-r--r-- | setup.cfg | 6 | ||||
-rw-r--r-- | tests/primer/packages_to_lint.json | 59 | ||||
-rw-r--r-- | tests/primer/test_primer_external.py | 66 | ||||
-rw-r--r-- | tests/primer/test_primer_stdlib.py | 3 | ||||
-rw-r--r-- | tests/testutils/test_package_to_lint.py | 48 |
12 files changed, 335 insertions, 3 deletions
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 41e9642bc..700fd8f31 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -509,3 +509,37 @@ jobs: . venv/bin/activate pip install -e . pytest -m primer_stdlib -n auto + + pytest-primer-external: + name: Run primer tests on external libs Python ${{ matrix.python-version }} (Linux) + runs-on: ubuntu-latest + needs: prepare-tests-linux + strategy: + matrix: + python-version: [3.8, 3.9, "3.10"] + steps: + - name: Check out code from GitHub + uses: actions/checkout@v2.3.5 + - name: Set up Python ${{ matrix.python-version }} + id: python + uses: actions/setup-python@v2.2.2 + with: + python-version: ${{ matrix.python-version }} + - name: Restore Python virtual environment + id: cache-venv + uses: actions/cache@v2.1.6 + with: + path: venv + key: + ${{ runner.os }}-${{ steps.python.outputs.python-version }}-${{ + needs.prepare-tests-linux.outputs.python-key }} + - name: Fail job if Python cache restore failed + if: steps.cache-venv.outputs.cache-hit != 'true' + run: | + echo "Failed to restore Python venv from cache" + exit 1 + - name: Run pytest + run: | + . venv/bin/activate + pip install -e . + pytest -m primer_external -n auto diff --git a/.gitignore b/.gitignore index 046061c9b..7e7c9cc97 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ build-stamp .pytest_cache/ .mypy_cache/ .benchmarks/ +.pylint_primer_tests/ @@ -15,6 +15,11 @@ Release date: TBA Closes #4982 +* Introduced primer tests and a configuration tests framework. The helper classes available in + ``pylint/testutil/`` are still unstable and might be modified in the near future. + + Closes #4412 #5287 + * Fix ``install graphiz`` message which isn't needed for puml output format. * Fix a crash in the ``check_elif`` extensions where an undetected if in a comprehension diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index eb0e7a9a7..dae98d485 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -35,7 +35,7 @@ New checkers * Fix ``useless-super-delegation`` false positive when default keyword argument is a variable. -* Added new checker `use-implicit-booleanness``: Emitted when using collection +* Added new checker ``use-implicit-booleanness``: Emitted when using collection literals for boolean comparisons * Added new checker ``use-implicit-booleaness-not-comparison``: Emitted when @@ -204,3 +204,8 @@ Other Changes * Fix crash on ``open()`` calls when the ``mode`` argument is not a simple string. Partially closes #5321 + +* Introduced primer tests and a configuration tests framework. The helper classes available in + ``pylint/testutil/`` are still unstable and might be modified in the near future. + + Closes #4412 #5287 diff --git a/pylint/constants.py b/pylint/constants.py index bee1055e1..d30b9dd56 100644 --- a/pylint/constants.py +++ b/pylint/constants.py @@ -8,6 +8,7 @@ import platformdirs from pylint.__pkginfo__ import __version__ +PY37_PLUS = sys.version_info[:2] >= (3, 7) PY38_PLUS = sys.version_info[:2] >= (3, 8) PY39_PLUS = sys.version_info[:2] >= (3, 9) diff --git a/pylint/testutils/primer.py b/pylint/testutils/primer.py new file mode 100644 index 000000000..30fa841ad --- /dev/null +++ b/pylint/testutils/primer.py @@ -0,0 +1,107 @@ +import logging +from pathlib import Path +from typing import Dict, List, Optional, Union + +import git + +PRIMER_DIRECTORY_PATH = Path(".pylint_primer_tests") + + +class PackageToLint: + """Represents data about a package to be tested during primer tests""" + + url: str + """URL of the repository to clone""" + + branch: str + """Branch of the repository to clone""" + + directories: List[str] + """Directories within the repository to run pylint over""" + + commit: Optional[str] + """Commit hash to pin the repository on""" + + pylint_additional_args: List[str] + """Arguments to give to pylint""" + + pylintrc_relpath: Optional[str] + """Path relative to project's main directory to the pylintrc if it exists""" + + def __init__( + self, + url: str, + branch: str, + directories: List[str], + commit: Optional[str] = None, + pylint_additional_args: Optional[List[str]] = None, + pylintrc_relpath: Optional[str] = None, + ) -> None: + self.url = url + self.branch = branch + self.directories = directories + self.commit = commit + self.pylint_additional_args = pylint_additional_args or [] + self.pylintrc_relpath = pylintrc_relpath + + @property + def pylintrc(self) -> Optional[Path]: + if self.pylintrc_relpath is None: + return None + return self.clone_directory / self.pylintrc_relpath + + @property + def clone_directory(self) -> Path: + """Directory to clone repository into""" + clone_name = "/".join(self.url.split("/")[-2:]).replace(".git", "") + return PRIMER_DIRECTORY_PATH / clone_name + + @property + def paths_to_lint(self) -> List[str]: + """The paths we need to lint""" + return [str(self.clone_directory / path) for path in self.directories] + + @property + def pylint_args(self) -> List[str]: + options: List[str] = [] + if self.pylintrc is not None: + # There is an error if rcfile is given but does not exists + options += [f"--rcfile={self.pylintrc}"] + return self.paths_to_lint + options + self.pylint_additional_args + + def lazy_clone(self) -> None: # pragma: no cover + """Concatenates the target directory and clones the file + + Not expected to be tested as the primer won't work if it doesn't. + It's tested in the continuous integration primers, only the coverage + is not calculated on everything. If lazy clone breaks for local use + we'll probably notice because we'll have a fatal when launching the + primer locally. + """ + logging.info("Lazy cloning %s", self.url) + if not self.clone_directory.exists(): + options: Dict[str, Union[str, int]] = { + "url": self.url, + "to_path": str(self.clone_directory), + "branch": self.branch, + "depth": 1, + } + logging.info("Directory does not exists, cloning: %s", options) + git.Repo.clone_from(**options) + return + + remote_sha1_commit = ( + git.cmd.Git().ls_remote(self.url, self.branch).split("\t")[0] + ) + local_sha1_commit = git.Repo(self.clone_directory).head.object.hexsha + if remote_sha1_commit != local_sha1_commit: + logging.info( + "Remote sha is '%s' while local sha is '%s': pulling new commits", + remote_sha1_commit, + local_sha1_commit, + ) + repo = git.Repo(self.clone_directory) + origin = repo.remotes.origin + origin.pull() + else: + logging.info("Repository already up to date.") diff --git a/requirements_test_min.txt b/requirements_test_min.txt index d309c8607..a0e339897 100644 --- a/requirements_test_min.txt +++ b/requirements_test_min.txt @@ -3,3 +3,4 @@ astroid==2.9.0 # Pinned to a specific version for tests pytest~=6.2 pytest-benchmark~=3.4 +gitpython>3 @@ -69,9 +69,10 @@ test = pytest [tool:pytest] testpaths = tests python_files = *test_*.py -addopts = -m "not primer_stdlib" +addopts = --strict-markers -m "not primer_stdlib and not primer_external" markers = primer_stdlib: Checks for crashes and errors when running pylint on stdlib + primer_external: Checks for crashes and errors when running pylint on external libs benchmark: Baseline of pylint performance, if this regress something serious happened [isort] @@ -118,3 +119,6 @@ ignore_missing_imports = True [mypy-toml.decoder] ignore_missing_imports = True + +[mypy-git.*] +ignore_missing_imports = True diff --git a/tests/primer/packages_to_lint.json b/tests/primer/packages_to_lint.json new file mode 100644 index 000000000..507fa3af3 --- /dev/null +++ b/tests/primer/packages_to_lint.json @@ -0,0 +1,59 @@ +{ + "flask": { + "url": "https://github.com/pallets/flask.git", + "branch": "main", + "directories": ["src/flask"] + }, + "pytest": { + "url": "https://github.com/pytest-dev/pytest.git", + "branch": "main", + "directories": ["src/_pytest"] + }, + "psycopg": { + "url": "https://github.com/psycopg/psycopg.git", + "branch": "master", + "directories": ["psycopg/psycopg"] + }, + "keras": { + "url": "https://github.com/keras-team/keras.git", + "branch": "master", + "directories": ["keras"] + }, + "sentry": { + "url": "https://github.com/getsentry/sentry.git", + "branch": "master", + "directories": ["src/sentry"] + }, + "django": { + "url": "https://github.com/django/django.git", + "branch": "main", + "directories": ["django"] + }, + "pandas": { + "url": "https://github.com/pandas-dev/pandas.git", + "branch": "master", + "directories": ["pandas"], + "pylint_additional_args": ["--ignore-patterns=\"test_"] + }, + "black": { + "url": "https://github.com/psf/black.git", + "branch": "main", + "directories": ["src/black/", "src/blackd/", "src/black_primer/", "src/blib2to3/"] + }, + "home-assistant": { + "url": "https://github.com/home-assistant/core.git", + "branch": "dev", + "directories": ["homeassistant"] + }, + "graph-explorer": { + "url": "https://github.com/vimeo/graph-explorer.git", + "branch": "master", + "directories": ["graph_explorer"], + "pylintrc_relpath": ".pylintrc" + }, + "pygame": { + "url": "https://github.com/pygame/pygame.git", + "branch": "main", + "directories": ["src_py"] + } +} diff --git a/tests/primer/test_primer_external.py b/tests/primer/test_primer_external.py new file mode 100644 index 000000000..a3fb9b66c --- /dev/null +++ b/tests/primer/test_primer_external.py @@ -0,0 +1,66 @@ +# 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 +import json +import logging +import subprocess +from pathlib import Path +from typing import Dict, Union + +import pytest +from pytest import LogCaptureFixture + +from pylint.testutils.primer import PackageToLint + +PRIMER_DIRECTORY = Path(".pylint_primer_tests/").resolve() + + +def get_packages_to_lint_from_json( + json_path: Union[Path, str] +) -> Dict[str, PackageToLint]: + result: Dict[str, PackageToLint] = {} + with open(json_path, encoding="utf8") as f: + for name, package_data in json.load(f).items(): + result[name] = PackageToLint(**package_data) + return result + + +PACKAGE_TO_LINT_JSON = Path(__file__).parent / "packages_to_lint.json" +PACKAGES_TO_LINT = get_packages_to_lint_from_json(PACKAGE_TO_LINT_JSON) +"""Dictionary of external packages used during the primer test""" + + +class TestPrimer: + @staticmethod + @pytest.mark.primer_external + @pytest.mark.parametrize("package", PACKAGES_TO_LINT.values(), ids=PACKAGES_TO_LINT) + def test_primer_external_packages_no_crash( + package: PackageToLint, + caplog: LogCaptureFixture, + ) -> None: + __tracebackhide__ = True # pylint: disable=unused-variable + TestPrimer._primer_test(package, caplog) + + @staticmethod + def _primer_test(package: PackageToLint, caplog: LogCaptureFixture) -> None: + """Runs pylint over external packages to check for crashes and fatal messages + + We only check for crashes (bit-encoded exit code 32) and fatal messages + (bit-encoded exit code 1). We assume that these external repositories do not + have any fatal errors in their code so that any fatal errors are pylint false + positives + """ + caplog.set_level(logging.INFO) + package.lazy_clone() + + try: + # We want to test all the code we can + enables = ["--enable-all-extensions", "--enable=all"] + # Duplicate code takes too long and is relatively safe + disables = ["--disable=duplicate-code"] + command = ["pylint"] + enables + disables + package.pylint_args + logging.info("Launching primer:\n%s", " ".join(command)) + subprocess.run(command, check=True) + except subprocess.CalledProcessError as ex: + msg = f"Encountered {{}} during primer test for {package}" + assert ex.returncode != 32, msg.format("a crash") + assert ex.returncode % 2 == 0, msg.format("a message of category 'fatal'") diff --git a/tests/primer/test_primer_stdlib.py b/tests/primer/test_primer_stdlib.py index fdc16adcd..f403cbbf5 100644 --- a/tests/primer/test_primer_stdlib.py +++ b/tests/primer/test_primer_stdlib.py @@ -43,10 +43,11 @@ MODULES_NAMES = [m[1] for m in MODULES_TO_CHECK] @pytest.mark.parametrize( ("test_module_location", "test_module_name"), MODULES_TO_CHECK, ids=MODULES_NAMES ) -def test_lib_module_no_crash( +def test_primer_stdlib_no_crash( test_module_location: str, test_module_name: str, capsys: CaptureFixture ) -> None: """Test that pylint does not produces any crashes or fatal errors on stdlib modules""" + __tracebackhide__ = True # pylint: disable=unused-variable os.chdir(test_module_location) with _patch_stdout(io.StringIO()): try: diff --git a/tests/testutils/test_package_to_lint.py b/tests/testutils/test_package_to_lint.py new file mode 100644 index 000000000..bf90db3e7 --- /dev/null +++ b/tests/testutils/test_package_to_lint.py @@ -0,0 +1,48 @@ +# 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 + +from pylint.testutils.primer import PRIMER_DIRECTORY_PATH, PackageToLint + + +def test_package_to_lint() -> None: + """Test that the PackageToLint is instantiated correctly.""" + expected_dir_path = PRIMER_DIRECTORY_PATH / "vimeo" / "graph-explorer" + expected_path_to_lint = expected_dir_path / "graph_explorer" + expected_pylintrc_path = expected_dir_path / ".pylintrcui" + + args = ["--ignore-pattern='re*'"] + package_to_lint = PackageToLint( + url="https://github.com/vimeo/graph-explorer.git", + branch="main", + directories=["graph_explorer"], + pylintrc_relpath=".pylintrcui", + pylint_additional_args=args, + ) + + assert package_to_lint.url == "https://github.com/vimeo/graph-explorer.git" + assert package_to_lint.branch == "main" + assert package_to_lint.directories == ["graph_explorer"] + assert package_to_lint.pylintrc_relpath == ".pylintrcui" + assert package_to_lint.pylint_additional_args == args + assert package_to_lint.paths_to_lint == [str(expected_path_to_lint)] + assert package_to_lint.clone_directory == expected_dir_path + assert package_to_lint.pylintrc == expected_pylintrc_path + expected_args = [ + str(expected_path_to_lint), + f"--rcfile={expected_pylintrc_path}", + ] + args + assert package_to_lint.pylint_args == expected_args + + +def test_package_to_lint_default_value() -> None: + """Test that the PackageToLint is instantiated correctly with default value.""" + package_to_lint = PackageToLint( + url="https://github.com/pallets/flask.git", + branch="main", + directories=["src/flask"], # Must work on Windows (src\\flask) + ) + assert package_to_lint.pylintrc is None + expected_path_to_lint = ( + PRIMER_DIRECTORY_PATH / "pallets" / "flask" / "src" / "flask" + ) + assert package_to_lint.pylint_args == [str(expected_path_to_lint)] |