diff options
| -rw-r--r-- | .github/workflows/ci.yaml | 36 | ||||
| -rw-r--r-- | .gitignore | 1 | ||||
| -rw-r--r-- | pylint/constants.py | 1 | ||||
| -rw-r--r-- | pylint/testutils/primer.py | 100 | ||||
| -rw-r--r-- | requirements_test_min.txt | 1 | ||||
| -rw-r--r-- | setup.cfg | 7 | ||||
| -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 | 5 |
9 files changed, 271 insertions, 5 deletions
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 41e9642bc..75fde57c6 100644 --- a/.github/workflows/ci.yaml +++ b/.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/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 -n auto -k test_primer_external_packages_no_crash 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/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..e96ee0ea6 --- /dev/null +++ b/pylint/testutils/primer.py @@ -0,0 +1,100 @@ +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, + ) -> 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: + """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.") 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,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 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..e6ba0df67 --- /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 + @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..48264e6df 100644 --- a/tests/primer/test_primer_stdlib.py +++ b/tests/primer/test_primer_stdlib.py @@ -39,14 +39,15 @@ MODULES_TO_CHECK = [ 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""" + __tracebackhide__ = True # pylint: disable=unused-variable os.chdir(test_module_location) with _patch_stdout(io.StringIO()): try: |