summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPierre Sassoulas <pierre.sassoulas@gmail.com>2021-11-24 13:10:52 +0100
committerGitHub <noreply@github.com>2021-11-24 13:10:52 +0100
commit5e9d20dc32768e873ab84bc4ed0b489fdda40672 (patch)
tree26e1018b2ff6f299b23d93fa3160e0bd704966cd
parente8fa46928d91893723a9a038fde2d274d535fd1f (diff)
downloadpylint-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.yaml34
-rw-r--r--.gitignore1
-rw-r--r--ChangeLog5
-rw-r--r--doc/whatsnew/2.12.rst7
-rw-r--r--pylint/constants.py1
-rw-r--r--pylint/testutils/primer.py107
-rw-r--r--requirements_test_min.txt1
-rw-r--r--setup.cfg6
-rw-r--r--tests/primer/packages_to_lint.json59
-rw-r--r--tests/primer/test_primer_external.py66
-rw-r--r--tests/primer/test_primer_stdlib.py3
-rw-r--r--tests/testutils/test_package_to_lint.py48
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/
diff --git a/ChangeLog b/ChangeLog
index 30b15d0ea..b3c5fbcf7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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
diff --git a/setup.cfg b/setup.cfg
index 18f7da882..3aaf37699 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -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)]