Add primer tests, (running pylint on external libs during tests)

In order to anticipate crash/fatal messages, false positives are harder
to anticipate.

Author: Pierre-Sassoulas

.github/workflows/ci.yaml

@@ -508,4 +508,38 @@ jobs:
         run: |
           . venv/bin/activate
           pip install -e .
-          pytest -m primer_stdlib -n auto
+          pytest -m primer -n auto -k test_primer_stdlib_no_crash
+
+  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/[email protected]
+      - name: Set up Python ${{ matrix.python-version }}
+        id: python
+        uses: actions/[email protected]
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Restore Python virtual environment
+        id: cache-venv
+        uses: actions/[email protected]
+        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 -n auto -k test_primer_external_packages_no_crash

.gitignore

@@ -20,3 +20,4 @@ build-stamp
 .pytest_cache/
 .mypy_cache/
 .benchmarks/
+.pylint_primer_tests/

pylint/constants.py

@@ -8,6 +8,7 @@
 
 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)
 

pylint/testutils/primer.py

@@ -0,0 +1,102 @@
+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] = None
+    """Commit hash to pin the repository on"""
+
+    pylint_additional_args: List[str] = []
+    """Arguments to give to pylint"""
+
+    pylintrc_relpath: Optional[str] = None
+    """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,
+    ):
+        self.url = url
+        self.branch = branch
+        self.directories = directories
+        self.commit = commit
+        if pylint_additional_args is None:
+            pylint_additional_args = []
+        self.pylint_additional_args = pylint_additional_args
+        self.pylintrc_relpath = pylintrc_relpath
+
+    @property
+    def pylintrc(self) -> Optional[str]:
+        if self.pylintrc_relpath is None:
+            return None
+        return f"{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:
+        """Concatenates the target directory and clones the file"""
+        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",
+                remote_sha1_commit,
+                local_sha1_commit,
+            )
+            repo = git.Repo(self.clone_directory)
+            origin = repo.remotes.origin
+            origin.pull()
+        else:
+            logging.info("Already up to date.")

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

setup.cfg

@@ -69,9 +69,9 @@ test = pytest
 [tool:pytest]
 testpaths = tests
 python_files = *test_*.py
-addopts = -m "not primer_stdlib"
+addopts = --strict-markers -m "not primer"
 markers =
-    primer_stdlib: Checks for crashes and errors when running pylint on stdlib
+    primer: Checks for crashes and errors when running pylint on stdlib or  on external libs
     benchmark: Baseline of pylint performance, if this regress something serious happened
 
 [isort]
@@ -118,3 +118,6 @@ ignore_missing_imports = True
 
 [mypy-toml.decoder]
 ignore_missing_imports = True
+
+[mypy-git.*]
+ignore_missing_imports = True

tests/primer/packages_to_lint.json

@@ -0,0 +1,49 @@
+{
+  "flask": {
+    "url": "https://github.com/pallets/flask.git",
+    "branch": "main",
+    "directories": ["src/flask"]
+  },
+  "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"]
+  }
+}

tests/primer/test_primer_external.py

@@ -0,0 +1,69 @@
+# 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.constants import PY37_PLUS
+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
+    @pytest.mark.parametrize("package", PACKAGES_TO_LINT.values(), ids=PACKAGES_TO_LINT)
+    def test_primer_external_packages_no_crash(
+        package: PackageToLint,
+        caplog: LogCaptureFixture,
+    ) -> None:
+        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))
+            if PY37_PLUS:
+                subprocess.run(command, check=True, capture_output=True)
+            else:
+                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'")

tests/primer/test_primer_stdlib.py

@@ -39,11 +39,11 @@ def _patch_stdout(out):
 MODULES_NAMES = [m[1] for m in MODULES_TO_CHECK]
 
 
-@pytest.mark.primer_stdlib
+@pytest.mark.primer
 @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"""