From 17d7d7238a660b9000bbb6136656fcee36029bc8 Mon Sep 17 00:00:00 2001 From: Serg Tereshchenko Date: Wed, 18 May 2022 22:04:21 +0300 Subject: [PATCH] fix: Resolve code review discussions --- .github/workflows/base.yml | 3 - noxfile.py | 14 +++- setup.cfg | 10 ++- tests/pyright/__snapshots__/test_typing.ambr | 76 ++++++++++++-------- tests/pyright/base.py | 23 +++++- tests/pyright/test_file.py | 27 +++---- tests/pyright/test_typing.py | 11 ++- 7 files changed, 110 insertions(+), 54 deletions(-) diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index 56d6619..d62c103 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -53,9 +53,6 @@ jobs: with: node-version: '16' - - name: Install pyright - run: npm install -g pyright@1.1.223 - # Conda install - name: Install conda v3.7 uses: conda-incubator/setup-miniconda@v2 diff --git a/noxfile.py b/noxfile.py index c67c777..7eb2ed9 100644 --- a/noxfile.py +++ b/noxfile.py @@ -3,6 +3,7 @@ import logging import nox # noqa +from nox.command import CommandFailed from pathlib import Path # noqa import sys @@ -28,7 +29,6 @@ PY37: {"coverage": True, "pkg_specs": {"pip": ">19"}}, # , "pytest-html": "1.9.0" } - # set the default activated sessions, minimal for CI nox.options.sessions = ["tests", "flake8"] # , "docs", "gh_pages" nox.options.reuse_existing_virtualenvs = True # this can be done using -r @@ -98,7 +98,10 @@ def tests(session: PowerSession, coverage, pkg_specs): conda_prefix = Path(session.bin) if conda_prefix.name == "bin": conda_prefix = conda_prefix.parent - session.run2("conda list", env={"CONDA_PREFIX": str(conda_prefix), "CONDA_DEFAULT_ENV": session.get_session_id()}) + try: + session.run2("conda list", env={"CONDA_PREFIX": str(conda_prefix), "CONDA_DEFAULT_ENV": session.get_session_id()}) + except CommandFailed: + pass # Fail if the assumed python version is not the actual one session.run2("python ci_tools/check_python_version.py %s" % session.python) @@ -107,6 +110,13 @@ def tests(session: PowerSession, coverage, pkg_specs): # Important: do not surround the command into double quotes as in the shell ! # session.run('python', '-c', 'import os; os.chdir(\'./docs/\'); import %s' % pkg_name) + # Type checking is supported from python 3.7 + if float(session.python) >= 3.7: + try: + session.run2("npm install -g pyright@1.1.247") + except CommandFailed: + print("Failed to install pyright, typing tests would be skipped") + # finally run all tests if not coverage: # install self so that it is recognized by pytest diff --git a/setup.cfg b/setup.cfg index 5dd4126..daa4b6d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -35,14 +35,20 @@ setup_requires = pytest-runner install_requires = makefun>=1.5.0 - typing_extensions;python_version>='3.6' + # we're using typing_extensions even for new versions of python, so we don't pollute our code + # with try/except every time we use new feature. + # typing_extensions will use version from typing module if it detects new enough python. + typing-extensions>=4.2;python_version>='3.6' # note: do not use double quotes in these, this triggers a weird bug in PyCharm in debug mode only funcsigs;python_version<'3.3' enum34;python_version<'3.4' tests_require = pytest pytest_cases - syrupy;python_version>'3.6' + # syrupy is used for snapshot testing with pyright, + # pyright must be installed separately, we're not using https://pypi.org/project/pyright/ + # becouse it's just slow wrapper around node version. + syrupy>2;python_version>'3.6' # for some reason these pytest dependencies were not declared in old versions of pytest six;python_version<'3.6' attr;python_version<'3.6' diff --git a/tests/pyright/__snapshots__/test_typing.ambr b/tests/pyright/__snapshots__/test_typing.ambr index 1df0c00..67c27e8 100644 --- a/tests/pyright/__snapshots__/test_typing.ambr +++ b/tests/pyright/__snapshots__/test_typing.ambr @@ -1,40 +1,58 @@ # name: test_typing - [ - { - 'message': ' + list([ + dict({ + 'message': ''' No overloads for "function_decorator" match the provided arguments   Argument types: (Literal[True]) - ', - 'range': { - 'end': { + ''', + 'range': dict({ + 'end': dict({ 'character': 47, - 'line': 10, - }, - 'start': { + 'line': 15, + }), + 'start': dict({ 'character': 9, - 'line': 10, - }, - }, + 'line': 15, + }), + }), 'rule': 'reportGeneralTypeIssues', 'severity': 'error', - }, - { - 'message': ' + }), + dict({ + 'message': ''' Argument of type "Literal[2]" cannot be assigned to parameter "scope" of type "str" in function "__call__"   "Literal[2]" is incompatible with "str" - ', - 'range': { - 'end': { - 'character': 26, - 'line': 30, - }, - 'start': { - 'character': 25, - 'line': 30, - }, - }, + ''', + 'range': dict({ + 'end': dict({ + 'character': 22, + 'line': 36, + }), + 'start': dict({ + 'character': 21, + 'line': 36, + }), + }), 'rule': 'reportGeneralTypeIssues', 'severity': 'error', - }, - ] ---- + }), + dict({ + 'message': ''' + Argument of type "Literal[2]" cannot be assigned to parameter "scope" of type "str" in function "__call__" +   "Literal[2]" is incompatible with "str" + ''', + 'range': dict({ + 'end': dict({ + 'character': 34, + 'line': 52, + }), + 'start': dict({ + 'character': 33, + 'line': 52, + }), + }), + 'rule': 'reportGeneralTypeIssues', + 'severity': 'error', + }), + ]) +# --- diff --git a/tests/pyright/base.py b/tests/pyright/base.py index 71015cc..324789a 100644 --- a/tests/pyright/base.py +++ b/tests/pyright/base.py @@ -1,18 +1,35 @@ import json +import shutil import subprocess +__all__ = ["run_pyright", "pyright_installed"] + +try: + pyright_bin = shutil.which("pyright") + pyright_installed = pyright_bin is not None +except AttributeError: + # shutil.which works from python 3.3 onward + pyright_bin = None + pyright_installed = False + def run_pyright(filename): + """ + Executes pyright type checker against a file, and returns json output. + + Used together with syrupy snapshot to check if typing is working as expected. + """ result = subprocess.run( - ["pyright", "--outputjson", filename], + [pyright_bin, "--outputjson", filename], capture_output=True, text=True, ) assert result.stdout, result.stderr output = json.loads(result.stdout) - def clean_row(data): + def format_row(data): + # Remove "file" from json report, it has no use here. del data["file"] return data - return [clean_row(row) for row in output["generalDiagnostics"]] + return [format_row(row) for row in output["generalDiagnostics"]] diff --git a/tests/pyright/test_file.py b/tests/pyright/test_file.py index ce791c1..f9db440 100644 --- a/tests/pyright/test_file.py +++ b/tests/pyright/test_file.py @@ -1,3 +1,7 @@ +""" +Tests in this file do almost nothing at runtime, but serve as a source for +testing with pyright from test_typing.py +""" from typing import Any, Callable import pytest @@ -7,16 +11,16 @@ def test_invalid_parameter(): with pytest.raises(TypeError): - # Error, invalid argument + # Error, invalid argument. + # This triggers error in type checking and in runtime. @function_decorator(invalid_param=True) - def decorator_wint_invalid_param(fn=DECORATED): + def decorator_with_invalid_param(fn=DECORATED): return fn def test_normal_decorator(): @function_decorator def decorator(scope="test", fn=DECORATED): # type: (str, Any) -> Callable[..., Any] - assert isinstance(scope, str) return fn # Ok @@ -24,20 +28,15 @@ def decorator(scope="test", fn=DECORATED): # type: (str, Any) -> Callable[..., def decorated_flat(): pass - assert decorated_flat - - with pytest.raises(AssertionError): - # Error, Literal[2] is incompatible with str - @decorator(scope=2) - def decorated_with_invalid_options(): - pass - # Ok, should reveal correct type for `scope` @decorator(scope="success") def decorated_with_valid_options(): pass - assert decorated_with_valid_options + # Error, Literal[2] is incompatible with str + @decorator(scope=2) + def decorated_with_invalid_options(): + pass def test_function_decorator_with_params(): @@ -51,4 +50,6 @@ def decorator_with_params(scope = "test", fn=DECORATED): # type: (str, Any) -> def decorated_with_valid_options(): pass - assert decorated_with_valid_options + @decorator_with_params(scope=2) + def decorated_with_invalid_options(): + pass diff --git a/tests/pyright/test_typing.py b/tests/pyright/test_typing.py index b3ee190..c9ac45d 100644 --- a/tests/pyright/test_typing.py +++ b/tests/pyright/test_typing.py @@ -1,12 +1,19 @@ -import pytest import sys -from .base import run_pyright + +import pytest + +from .base import run_pyright, pyright_installed @pytest.mark.skipif( sys.version_info < (3, 7), reason="Requires Python 3.7+", ) +@pytest.mark.skipif( + not pyright_installed, + reason="Pyright not installed", +) def test_typing(snapshot): + """Test that pyright detects the typing issues on `test_file` correctly.""" actual = run_pyright("tests/pyright/test_file.py") assert actual == snapshot