From 5d5e7d7e4d280ac33b8ac5df73e154ebf31b5933 Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Tue, 22 Aug 2023 16:57:25 +0200 Subject: [PATCH 1/3] pre-commit: Add codespell and ruff --- .flake8 | 2 - .pre-commit-config.yaml | 20 ++--- algorithms_keeper/api.py | 20 ++--- algorithms_keeper/parser/files_parser.py | 2 +- algorithms_keeper/parser/python_parser.py | 3 +- algorithms_keeper/parser/record.py | 4 +- .../parser/rules/naming_convention.py | 24 +++--- .../parser/rules/require_doctest.py | 6 +- .../parser/rules/require_type_hint.py | 4 +- algorithms_keeper/parser/rules/use_fstring.py | 8 +- algorithms_keeper/utils.py | 4 +- pyproject.toml | 78 ++++++++++++++++++- tests/data/descriptive_name.py | 2 - tests/test_api.py | 11 +-- tests/test_check_runs.py | 4 +- tests/test_commands.py | 7 +- tests/test_installations.py | 2 +- tests/test_main.py | 12 +-- tests/test_parser.py | 9 +-- tests/test_pull_requests.py | 14 ++-- tests/test_rules.py | 17 ++-- tests/test_utils.py | 36 ++++----- tests/utils.py | 32 ++++---- 23 files changed, 195 insertions(+), 126 deletions(-) delete mode 100644 .flake8 diff --git a/.flake8 b/.flake8 deleted file mode 100644 index 2bcd70e..0000000 --- a/.flake8 +++ /dev/null @@ -1,2 +0,0 @@ -[flake8] -max-line-length = 88 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4ee772b..a1b564a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,17 +7,19 @@ repos: types: [python] - id: trailing-whitespace - - repo: https://github.com/psf/black - rev: 23.1.0 + - repo: https://github.com/codespell-project/codespell + rev: v2.2.5 hooks: - - id: black + - id: codespell + additional_dependencies: + - tomli - - repo: https://github.com/PyCQA/isort - rev: 5.12.0 + - repo: https://github.com/psf/black + rev: 23.7.0 hooks: - - id: isort + - id: black - - repo: https://github.com/PyCQA/flake8 - rev: 6.0.0 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.0.285 hooks: - - id: flake8 + - id: ruff diff --git a/algorithms_keeper/api.py b/algorithms_keeper/api.py index 6693bfb..b17ab88 100644 --- a/algorithms_keeper/api.py +++ b/algorithms_keeper/api.py @@ -30,13 +30,14 @@ def _get_private_key() -> str: if private_key is None: private_key_path = os.getenv("GITHUB_PRIVATE_KEY_PATH") if private_key_path is None: - raise RuntimeError( + msg = ( "Provide the private key using the GITHUB_PRIVATE_KEY environment " - + "variable or in a file using the GITHUB_PRIVATE_KEY_PATH " - + "environment variable. The path should either be absolute or " - + "relative to the repository root." + "variable or in a file using the GITHUB_PRIVATE_KEY_PATH environment " + "variable. The path should either be absolute or relative to the " + "repository root." ) - with open(private_key_path, "r") as f: + raise RuntimeError(msg) + with open(private_key_path) as f: # noqa: PTH123 private_key = f.read() return private_key @@ -93,10 +94,11 @@ def log(response: ClientResponse, body: bytes) -> None: # pragma: no cover if response.status in STATUS_OK: loggerlevel = logger.info # Comments and reviews are too long to be logged for INFO level - if response.url.name in ("comments", "reviews"): - data = response.url.name.upper() - else: - data = body.decode(UTF_8_CHARSET) + data = ( + response.url.name.upper() + if response.url.name in ("comments", "reviews") + else body.decode(UTF_8_CHARSET) + ) else: loggerlevel = logger.error data = body.decode(UTF_8_CHARSET) diff --git a/algorithms_keeper/parser/files_parser.py b/algorithms_keeper/parser/files_parser.py index ae4f306..f99da1a 100644 --- a/algorithms_keeper/parser/files_parser.py +++ b/algorithms_keeper/parser/files_parser.py @@ -53,7 +53,7 @@ def validate_extension(self) -> str: filepath = file.path if not filepath.suffix: if ".github" not in filepath.parts: - if filepath.parent.name: + if filepath.parent.name: # noqa: SIM114 invalid_filepath.append(file.name) # Dotfiles are not considered as invalid if they are present in the # root of the repository diff --git a/algorithms_keeper/parser/python_parser.py b/algorithms_keeper/parser/python_parser.py index e3e37e6..61cebb9 100644 --- a/algorithms_keeper/parser/python_parser.py +++ b/algorithms_keeper/parser/python_parser.py @@ -74,7 +74,8 @@ class PythonParser(BaseFilesParser): ".txt", # Good old Python file ".py", - ) + DOCS_EXTENSIONS + *DOCS_EXTENSIONS, + ) def __init__( self, diff --git a/algorithms_keeper/parser/record.py b/algorithms_keeper/parser/record.py index d5a3aaa..f19b232 100644 --- a/algorithms_keeper/parser/record.py +++ b/algorithms_keeper/parser/record.py @@ -83,12 +83,12 @@ def add_error( # It seems that ``ParserSyntaxError`` is not a subclass of ``SyntaxError``, # the same information is stored under a different attribute. There is no # filename information in ``ParserSyntaxError``, thus the parameter `filepath`. - if isinstance(exc, SyntaxError): # pragma: no cover + if isinstance(exc, SyntaxError): # noqa: SIM108, pragma: no cover lineno = exc.lineno or 1 else: lineno = exc.raw_line body = ( - f"An error occured while parsing the file: `{filepath}`\n" + f"An error occurred while parsing the file: `{filepath}`\n" f"```python\n{message}\n```" ) self._comments.append(ReviewComment(body, filepath, lineno)) diff --git a/algorithms_keeper/parser/rules/naming_convention.py b/algorithms_keeper/parser/rules/naming_convention.py index b01326d..11d362a 100644 --- a/algorithms_keeper/parser/rules/naming_convention.py +++ b/algorithms_keeper/parser/rules/naming_convention.py @@ -10,14 +10,14 @@ INVALID_CAMEL_CASE_NAME_COMMENT: str = ( "Class names should follow the [`CamelCase`]" - + "(https://en.wikipedia.org/wiki/Camel_case) naming convention. " - + "Please update the following name accordingly: `{nodename}`" + "(https://en.wikipedia.org/wiki/Camel_case) naming convention. " + "Please update the following name accordingly: `{nodename}`" ) INVALID_SNAKE_CASE_NAME_COMMENT: str = ( "Variable and function names should follow the [`snake_case`]" - + "(https://en.wikipedia.org/wiki/Snake_case) naming convention. " - + "Please update the following name accordingly: `{nodename}`" + "(https://en.wikipedia.org/wiki/Snake_case) naming convention. " + "Please update the following name accordingly: `{nodename}`" ) @@ -35,9 +35,8 @@ def valid(self, name: str) -> bool: name = name.strip("_") if name[0].islower() or "_" in name: return False - else: - if name.lower() != name and name.upper() != name: - return False + elif name.lower() != name and name.upper() != name: + return False return True @@ -159,7 +158,7 @@ def visit_ClassDef(self, node: cst.ClassDef) -> None: def visit_Attribute(self, node: cst.Attribute) -> None: # The attribute node can come through other context as well but we only care # about the ones coming from assignments. - if self._assigntarget_counter > 0: + if self._assigntarget_counter > 0: # noqa: SIM102 # We only care about assignment attribute to *self*. if m.matches(node, m.Attribute(value=m.Name(value="self"))): self._validate_nodename( @@ -169,10 +168,11 @@ def visit_Attribute(self, node: cst.Attribute) -> None: def visit_Element(self, node: cst.Element) -> None: # We only care about elements in *List* or *Tuple* specifically coming from # inside the multiple assignments. - if self._assigntarget_counter > 0: - if m.matches(node, m.Element(value=m.Name())): - nodename = cst.ensure_type(node.value, cst.Name).value - self._validate_nodename(node, nodename, NamingConvention.SNAKE_CASE) + if self._assigntarget_counter > 0 and m.matches( + node, m.Element(value=m.Name()) + ): + nodename = cst.ensure_type(node.value, cst.Name).value + self._validate_nodename(node, nodename, NamingConvention.SNAKE_CASE) def visit_For(self, node: cst.For) -> None: if m.matches(node, m.For(target=m.Name())): diff --git a/algorithms_keeper/parser/rules/require_doctest.py b/algorithms_keeper/parser/rules/require_doctest.py index 9121111..b8f5679 100644 --- a/algorithms_keeper/parser/rules/require_doctest.py +++ b/algorithms_keeper/parser/rules/require_doctest.py @@ -8,7 +8,7 @@ MISSING_DOCTEST: str = ( "As there is no test file in this pull request nor any test function or class in " - + "the file `{filepath}`, please provide doctest for the function `{nodename}`" + "the file `{filepath}`, please provide doctest for the function `{nodename}`" ) INIT: str = "__init__" @@ -184,7 +184,7 @@ def spam(self): pass """ ), - # Check that `_skip_doctest` attribute is reseted after leaving the class. + # Check that `_skip_doctest` attribute is reset after leaving the class. Invalid( """ def bar(): @@ -221,7 +221,7 @@ def visit_ClassDef(self, node: cst.ClassDef) -> None: # Temporary storage of the ``skip_doctest`` value only during the class visit. # If the class-level docstring contains doctest, then the checks should only be # skipped for all its methods and not for other functions/class in the module. - # After leaving the class, ``skip_doctest`` should be resetted to whatever the + # After leaving the class, ``skip_doctest`` should be reset to whatever the # value was before. self._temporary = self._skip_doctest self._skip_doctest = self._has_doctest(node) diff --git a/algorithms_keeper/parser/rules/require_type_hint.py b/algorithms_keeper/parser/rules/require_type_hint.py index abfe1bf..4172810 100644 --- a/algorithms_keeper/parser/rules/require_type_hint.py +++ b/algorithms_keeper/parser/rules/require_type_hint.py @@ -7,8 +7,8 @@ MISSING_RETURN_TYPE_HINT: str = ( "Please provide return type hint for the function: `{nodename}`. " - + "**If the function does not return a value, please provide " - + "the type hint as:** `def function() -> None:`" + "**If the function does not return a value, please provide " + "the type hint as:** `def function() -> None:`" ) IGNORE_PARAM: set[str] = {"self", "cls"} diff --git a/algorithms_keeper/parser/rules/use_fstring.py b/algorithms_keeper/parser/rules/use_fstring.py index b861ddd..aa1240f 100644 --- a/algorithms_keeper/parser/rules/use_fstring.py +++ b/algorithms_keeper/parser/rules/use_fstring.py @@ -8,10 +8,10 @@ class UseFstringRule(CstLintRule): MESSAGE: str = ( "As mentioned in the [Contributing Guidelines]" - + "(https://github.com/TheAlgorithms/Python/blob/master/CONTRIBUTING.md), " - + "please do not use printf style formatting or `str.format()`. " - + "Use [f-string](https://realpython.com/python-f-strings/) instead to be " - + "more readable and efficient." + "(https://github.com/TheAlgorithms/Python/blob/master/CONTRIBUTING.md), " + "please do not use printf style formatting or `str.format()`. " + "Use [f-string](https://realpython.com/python-f-strings/) instead to be " + "more readable and efficient." ) VALID = [ diff --git a/algorithms_keeper/utils.py b/algorithms_keeper/utils.py index d4c9199..13141f8 100644 --- a/algorithms_keeper/utils.py +++ b/algorithms_keeper/utils.py @@ -142,7 +142,7 @@ async def get_user_open_pr_numbers( ) pr_numbers = [] async for pull in gh.getiter(search_url, oauth_token=await gh.access_token): - pr_numbers.append(pull["number"]) + pr_numbers.append(pull["number"]) # noqa: PERF401 return pr_numbers @@ -211,7 +211,7 @@ async def get_pr_files(gh: GitHubAPI, *, pull_request: Mapping[str, Any]) -> lis async for data in gh.getiter( pull_request["url"] + "/files", oauth_token=await gh.access_token ): - files.append( + files.append( # noqa: PERF401 File( data["filename"], Path(data["filename"]), diff --git a/pyproject.toml b/pyproject.toml index 11d88d2..9d7efdf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,3 @@ -[tool.isort] -profile = "black" - [tool.mypy] ignore_missing_imports = true warn_unused_configs = true @@ -19,6 +16,81 @@ module = "tests.data.*" disallow_untyped_defs = false check_untyped_defs = false +[tool.ruff] +select = [ + "A", # flake8-builtins + "AIR", # Airflow + "ASYNC", # flake8-async + "B", # flake8-bugbear + "BLE", # flake8-blind-except + "C4", # flake8-comprehensions + "C90", # McCabe cyclomatic complexity + "CPY", # flake8-copyright + "DJ", # flake8-django + "DTZ", # flake8-datetimez + "E", # pycodestyle + "EM", # flake8-errmsg + "EXE", # flake8-executable + "F", # Pyflakes + "FLY", # flynt + "G", # flake8-logging-format + "I", # isort + "ICN", # flake8-import-conventions + "INP", # flake8-no-pep420 + "INT", # flake8-gettext + "ISC", # flake8-implicit-str-concat + "N", # pep8-naming + "NPY", # NumPy-specific rules + "PD", # pandas-vet + "PERF", # Perflint + "PGH", # pygrep-hooks + "PIE", # flake8-pie + "PL", # Pylint + "PT", # flake8-pytest-style + "PTH", # flake8-use-pathlib + "PYI", # flake8-pyi + "RSE", # flake8-raise + "RUF", # Ruff-specific rules + "S", # flake8-bandit + "SIM", # flake8-simplify + "SLF", # flake8-self + "SLOT", # flake8-slots + "T10", # flake8-debugger + "TCH", # flake8-type-checking + "TID", # flake8-tidy-imports + "UP", # pyupgrade + "W", # pycodestyle + "YTT", # flake8-2020 + # "ANN", # flake8-annotations + # "ARG", # flake8-unused-arguments + # "COM", # flake8-commas + # "D", # pydocstyle + # "ERA", # eradicate + # "FA", # flake8-future-annotations + # "FBT", # flake8-boolean-trap + # "FIX", # flake8-fixme + # "Q", # flake8-quotes + # "RET", # flake8-return + # "T20", # flake8-print + # "TD", # flake8-todos + # "TRY", s# tryceratops +] +ignore = ["N802", "PGH003", "RUF012", "SLF001"] +line-length = 125 +target-version = "py38" + +[tool.ruff.mccabe] +max-complexity = 13 + +[tool.ruff.per-file-ignores] +"tests/*" = ["PT006", "PT007", "S101"] +"tests/test_commands.py" = ["B008"] +"tests/test_pull_requests.py" = ["B008"] + +[tool.ruff.pylint] +allow-magic-value-types = ["bytes", "int", "str"] +max-args = 6 # Recommended: 5 + [tool.pytest.ini_options] testpaths = "tests" addopts = """\ diff --git a/tests/data/descriptive_name.py b/tests/data/descriptive_name.py index 9a5c6a8..662d52b 100644 --- a/tests/data/descriptive_name.py +++ b/tests/data/descriptive_name.py @@ -66,5 +66,3 @@ def c(self, a: int = 10) -> None: class C: """A class which requires descriptive names""" - - pass diff --git a/tests/test_api.py b/tests/test_api.py index 64652e0..bf5478f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -4,7 +4,6 @@ import pytest import pytest_asyncio from gidgethub import apps, sansio -from pytest import MonkeyPatch from algorithms_keeper.api import GitHubAPI, token_cache @@ -23,7 +22,7 @@ async def github_api() -> AsyncGenerator[GitHubAPI, None]: assert session.closed is True -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_initialization() -> None: async with aiohttp.ClientSession() as session: github_api = GitHubAPI(number, session, "algorithms-keeper") @@ -35,8 +34,10 @@ async def test_initialization() -> None: assert github_api.requester == "algorithms-keeper" -@pytest.mark.asyncio -async def test_access_token(github_api: GitHubAPI, monkeypatch: MonkeyPatch) -> None: +@pytest.mark.asyncio() +async def test_access_token( + github_api: GitHubAPI, monkeypatch: pytest.MonkeyPatch +) -> None: monkeypatch.setattr(apps, "get_installation_access_token", mock_return) monkeypatch.setenv("GITHUB_APP_ID", "") monkeypatch.setenv("GITHUB_PRIVATE_KEY", "") @@ -49,7 +50,7 @@ async def test_access_token(github_api: GitHubAPI, monkeypatch: MonkeyPatch) -> assert cached_token == token -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_headers_and_log(github_api: GitHubAPI) -> None: request_headers = sansio.create_headers("algorithms-keeper") resp = await github_api._request( diff --git a/tests/test_check_runs.py b/tests/test_check_runs.py index 15e898c..70b6d35 100644 --- a/tests/test_check_runs.py +++ b/tests/test_check_runs.py @@ -20,7 +20,7 @@ # Reminder: ``Event.delivery_id`` is used as a short description for the respective # test case and as a way to id the specific test case in the parametrized group. -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "event, gh, expected", ( @@ -187,7 +187,7 @@ post_data=[{"labels": [Label.FAILED_TEST]}], ), ), - # Check run complated and it's a failure, there is ``FAILED_TEST`` label on + # Check run completed and it's a failure, there is ``FAILED_TEST`` label on # the pull request, so don't do anything. ( Event( diff --git a/tests/test_commands.py b/tests/test_commands.py index 9b0d04b..2a59e84 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,7 +2,6 @@ import pytest from gidgethub.sansio import Event -from pytest import MonkeyPatch from algorithms_keeper import utils from algorithms_keeper.constants import Label @@ -29,8 +28,8 @@ @pytest.fixture(scope="module", autouse=True) def patch_module( - monkeypatch: MonkeyPatch = MonkeyPatch(), -) -> Generator[MonkeyPatch, None, None]: + monkeypatch: pytest.MonkeyPatch = pytest.MonkeyPatch(), +) -> Generator[pytest.MonkeyPatch, None, None]: async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes: filename = kwargs["file"].name if filename in { @@ -83,7 +82,7 @@ def test_command_regex_match(text: str, group: str) -> None: # Reminder: ``Event.delivery_id`` is used as a short description for the respective # test case and as a way to id the specific test case in the parametrized group. -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "event, gh, expected", ( diff --git a/tests/test_installations.py b/tests/test_installations.py index e33286c..5f46216 100644 --- a/tests/test_installations.py +++ b/tests/test_installations.py @@ -20,7 +20,7 @@ # Reminder: ``Event.delivery_id`` is used as a short description for the respective # test case and as a way to id the specific test case in the parametrized group. -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "event, gh, expected", ( diff --git a/tests/test_main.py b/tests/test_main.py index 1b394b8..fc0a44b 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -6,7 +6,7 @@ from .utils import number -@pytest.fixture +@pytest.fixture() def client(loop, aiohttp_client): # type: ignore app = web.Application() app.router.add_get("/", main.index) @@ -15,7 +15,7 @@ def client(loop, aiohttp_client): # type: ignore return loop.run_until_complete(aiohttp_client(app)) -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_ping(client): # type: ignore headers = {"X-GitHub-Event": "ping", "X-GitHub-Delivery": "1234"} data = {"zen": "testing is good"} @@ -24,7 +24,7 @@ async def test_ping(client): # type: ignore assert await response.text() == "pong" -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_failure(client): # type: ignore # Even in the face of an exception, the server should not crash. # Missing key headers. @@ -32,7 +32,7 @@ async def test_failure(client): # type: ignore assert response.status == 500 -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_success(client): # type: ignore headers = {"X-GitHub-Event": "project", "X-GitHub-Delivery": "1234"} # Sending a payload that shouldn't trigger any networking, but no errors @@ -42,7 +42,7 @@ async def test_success(client): # type: ignore assert response.status == 200 -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_index(client): # type: ignore response = await client.get("/") assert response.status == 200 @@ -50,7 +50,7 @@ async def test_index(client): # type: ignore assert "algorithms-keeper" in (await response.text()) -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_health(client): # type: ignore response = await client.get("/health") assert response.status == 200 diff --git a/tests/test_parser.py b/tests/test_parser.py index b843eff..6cf830e 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -2,7 +2,6 @@ from typing import List import pytest -from pytest import MonkeyPatch from algorithms_keeper.constants import Label from algorithms_keeper.parser import PythonParser, rules @@ -20,7 +19,7 @@ # Don't mix this up with `utils.get_file_content` def get_source(filename: str) -> bytes: - with open(DATA_DIRPATH / filename) as file: + with Path.open(DATA_DIRPATH / filename) as file: file_content = file.read() return file_content.encode("utf-8") @@ -28,7 +27,7 @@ def get_source(filename: str) -> bytes: def get_parser(filenames: str, status: str = "added") -> PythonParser: files = [] for name in filenames.split(","): - name = name.strip() + name = name.strip() # noqa: PLW2901 files.append(File(name, Path(name), "", status)) return PythonParser(files, pull_request=PR) @@ -36,7 +35,7 @@ def get_parser(filenames: str, status: str = "added") -> PythonParser: @pytest.mark.parametrize( "parser, expected", ( - # Non-python files having preffix `test_` should not be considered. + # Non-python files having prefix `test_` should not be considered. (get_parser("test_data.txt, sol1.py"), TOTAL_RULES_COUNT), (get_parser("tests/test_file.py, data/no_error.py"), TOTAL_RULES_COUNT - 1), (get_parser("data/no_error.py, data/doctest.py"), TOTAL_RULES_COUNT), @@ -187,7 +186,7 @@ def test_same_lineno_multiple_source() -> None: ), ) def test_combinations( - monkeypatch: MonkeyPatch, + monkeypatch: pytest.MonkeyPatch, filename: str, expected: int, labels: List[str], diff --git a/tests/test_pull_requests.py b/tests/test_pull_requests.py index 11e9daf..67686f4 100644 --- a/tests/test_pull_requests.py +++ b/tests/test_pull_requests.py @@ -3,7 +3,6 @@ import pytest from gidgethub.sansio import Event -from pytest import MonkeyPatch from algorithms_keeper import utils from algorithms_keeper.constants import Label @@ -47,8 +46,8 @@ @pytest.fixture(scope="module", autouse=True) def patch_module( - monkeypatch: MonkeyPatch = MonkeyPatch(), -) -> Generator[MonkeyPatch, None, None]: + monkeypatch: pytest.MonkeyPatch = pytest.MonkeyPatch(), +) -> Generator[pytest.MonkeyPatch, None, None]: async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes: filename = kwargs["file"].name if filename in { @@ -67,7 +66,7 @@ async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes: monkeypatch.undo() -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "event, gh, expected", # Pull request opened by the user, the bot found that the user has number of @@ -157,7 +156,10 @@ async def mock_get_file_content(*args: Any, **kwargs: Any) -> bytes: ids=parametrize_id, ) async def test_max_pr_by_user( - monkeypatch: MonkeyPatch, event: Event, gh: MockGitHubAPI, expected: ExpectedData + monkeypatch: pytest.MonkeyPatch, + event: Event, + gh: MockGitHubAPI, + expected: ExpectedData, ) -> None: # There are only two possible cases for the ``MAX_PR_BY_USER`` constant: # - The value is some arbitrary positive number greater than 0. @@ -174,7 +176,7 @@ async def test_max_pr_by_user( assert gh == expected -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "event, gh, expected", ( diff --git a/tests/test_rules.py b/tests/test_rules.py index ea77c93..6af2963 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -56,7 +56,7 @@ def _gen_all_test_cases(rules: LintRuleCollectionT) -> List[GenTestCaseType]: for rule in rules: if not issubclass(rule, CstLintRule): continue - for test_type in {"VALID", "INVALID"}: + for test_type in ("VALID", "INVALID"): if cases := getattr(rule, test_type, None): for index, test_case in enumerate(cases): all_cases.append((rule, test_case, f"{test_type}_{index}")) @@ -102,12 +102,11 @@ def test_rules( else: assert len(reports) > 0, ( 'Expected a report for this "invalid" test case but `self.report` was ' - + "not called:\n" - + test_case.code, + "not called:\n" + test_case.code, ) assert len(reports) <= 1, ( 'Expected one report from this "invalid" test case. Found multiple:\n' - + "\n".join(str(e) for e in reports), + "\n".join(str(e) for e in reports), ) report = reports[0] # type: ignore @@ -129,7 +128,7 @@ def test_rules( if test_case.expected_message is not None: assert test_case.expected_message == report.message, ( f"Expected message:\n {test_case.expected_message}\n" - + f"But got:\n {report.message}" + f"But got:\n {report.message}" ) patch = report.patch @@ -138,13 +137,13 @@ def test_rules( if patch is None: assert expected_replacement is None, ( "The rule for this test case has no auto-fix, but expected source was " - + "specified." + "specified." ) return assert expected_replacement is not None, ( "The rule for this test case has an auto-fix, but no expected source was " - + "specified." + "specified." ) expected_replacement = _dedent(expected_replacement) @@ -152,6 +151,6 @@ def test_rules( assert patched_code == expected_replacement, ( "Auto-fix did not produce expected result.\n" - + f"Expected:\n{expected_replacement}\n" - + f"But found:\n{patched_code}" + f"Expected:\n{expected_replacement}\n" + f"But found:\n{patched_code}" ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 55c5d56..ba9f0d7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -31,7 +31,7 @@ ) -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_issue_for_commit() -> None: getitem = { search_url: { @@ -49,7 +49,7 @@ async def test_get_issue_for_commit() -> None: assert result["state"] == "open" -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_issue_for_commit_not_found() -> None: getitem = { search_url: { @@ -65,7 +65,7 @@ async def test_get_issue_for_commit_not_found() -> None: assert result is None -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_check_runs_for_commit() -> None: getitem = { check_run_url: { @@ -89,7 +89,7 @@ async def test_get_check_runs_for_commit() -> None: assert {check_run["status"] for check_run in result["check_runs"]} == {"completed"} -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "pr_or_issue", [{"issue_url": issue_url}, {"labels_url": labels_url}], @@ -103,7 +103,7 @@ async def test_add_label_to_pr_or_issue(pr_or_issue: Dict[str, str]) -> None: assert {"labels": [Label.FAILED_TEST]} in gh.post_data -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_add_multiple_labels() -> None: pr_or_issue = {"number": number, "issue_url": issue_url} gh = MockGitHubAPI() @@ -116,7 +116,7 @@ async def test_add_multiple_labels() -> None: assert {"labels": [Label.TYPE_HINT, Label.REVIEW]} in gh.post_data -@pytest.mark.asyncio +@pytest.mark.asyncio() @pytest.mark.parametrize( "pr_or_issue", [{"issue_url": issue_url}, {"labels_url": labels_url}], @@ -130,7 +130,7 @@ async def test_remove_label_from_pr_or_issue(pr_or_issue: Dict[str, str]) -> Non assert f"{labels_url}/{parse_label}" in gh.delete_url -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_remove_multiple_labels() -> None: parse_label1 = urllib.parse.quote(Label.TYPE_HINT) parse_label2 = urllib.parse.quote(Label.REVIEW) @@ -145,7 +145,7 @@ async def test_remove_multiple_labels() -> None: assert f"{labels_url}/{parse_label2}" in gh.delete_url -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_user_open_pr_numbers() -> None: getiter = { pr_user_search_url: { @@ -161,7 +161,7 @@ async def test_get_user_open_pr_numbers() -> None: assert gh.getiter_url[0] == pr_user_search_url -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_add_comment_to_pr_or_issue() -> None: # PR and issue both have `comments_url` key. pr_or_issue = {"number": number, "comments_url": comments_url} @@ -173,7 +173,7 @@ async def test_add_comment_to_pr_or_issue() -> None: assert {"body": comment} in gh.post_data -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_close_pr_no_reviewers() -> None: pull_request = { "url": pr_url, @@ -192,7 +192,7 @@ async def test_close_pr_no_reviewers() -> None: assert gh.delete_data == [] -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_close_pr_with_reviewers() -> None: pull_request = { "url": pr_url, @@ -211,7 +211,7 @@ async def test_close_pr_with_reviewers() -> None: assert {"reviewers": ["test1", "test2"]} in gh.delete_data -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_close_issue() -> None: # Issues don't have `requested_reviewers` field. issue = {"url": issue_url, "comments_url": comments_url} @@ -226,7 +226,7 @@ async def test_close_issue() -> None: assert gh.delete_url == [] -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_close_pr_or_issue_with_label() -> None: # PRs don't have `labels_url` attribute. pull_request = { @@ -248,7 +248,7 @@ async def test_close_pr_or_issue_with_label() -> None: assert gh.delete_url == [] -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_pr_files() -> None: getiter = { files_url: [ @@ -266,7 +266,7 @@ async def test_get_pr_files() -> None: assert files_url in gh.getiter_url -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_file_content() -> None: getitem = { contents_url: { @@ -294,7 +294,7 @@ async def test_get_file_content() -> None: assert contents_url in gh.getitem_url -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_create_pr_review() -> None: pull_request = {"url": pr_url, "head": {"sha": sha}} gh = MockGitHubAPI() @@ -305,7 +305,7 @@ async def test_create_pr_review() -> None: assert gh.post_data[0]["event"] == "COMMENT" -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_add_reaction() -> None: comment = {"url": comment_url} gh = MockGitHubAPI() @@ -314,7 +314,7 @@ async def test_add_reaction() -> None: assert {"content": "+1"} in gh.post_data -@pytest.mark.asyncio +@pytest.mark.asyncio() async def test_get_pr_for_issue() -> None: getitem = {pr_url: None} issue = {"pull_request": {"url": pr_url}} diff --git a/tests/utils.py b/tests/utils.py index 7ec891c..5c21677 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -84,10 +84,8 @@ async def getitem(self, url: str, **kwargs: Any) -> Any: if getitem_return is not None: return getitem_return[url] else: - raise AssertionError( - "Expected to be supplied with the 'getitem' argument when " - "instantiating the object but got 'None' instead." - ) + msg = "Expected to be supplied with the 'getitem' argument when instantiating the object but got 'None' instead." + raise AssertionError(msg) async def getiter(self, url: str, **kwargs: Any) -> AsyncGenerator[Any, None]: self.getiter_url.append(url) @@ -95,10 +93,8 @@ async def getiter(self, url: str, **kwargs: Any) -> AsyncGenerator[Any, None]: if getiter_return is not None: data = getiter_return[url] else: - raise AssertionError( - "Expected to be supplied with the 'getiter' argument when " - "instantiating the object but got 'None' instead." - ) + msg = "Expected to be supplied with the 'getiter' argument when instantiating the object but got 'None' instead." + raise AssertionError(msg) if isinstance(data, dict) and "items" in data: data = data["items"] for item in data: @@ -113,7 +109,7 @@ async def post(self, url: str, *, data: Any, **kwargs: Any) -> Any: data.pop("comments") elif url == comments_url: # Comments can contain arbitrary data, so we will replace it with the - # dummy commen and compare that. + # dummy comment and compare that. data["body"] = comment self.post_data.append(data) # XXX Only used for installation. Is it really necessary? @@ -144,29 +140,29 @@ def __eq__(self, expected: object) -> bool: return NotImplemented for f in fields(expected): field_name = f.name - # Let the ``AttributeError`` propogate, if any. + # Let the ``AttributeError`` propagate, if any. actual_value = getattr(self, field_name) expected_value = getattr(expected, field_name) assert len(actual_value) == len(expected_value), ( f"Expected the length of '{self.__class__.__name__}.{field_name}' be " - + f"equal to the length of '{expected.__class__.__name__}.{field_name}'" - + f"\n\nActual value: {actual_value}" - + f"\n\nExpected value: {expected_value}" + f"equal to the length of '{expected.__class__.__name__}.{field_name}'" + f"\n\nActual value: {actual_value}" + f"\n\nExpected value: {expected_value}" ) for element in expected_value: assert element in actual_value, ( "Expected the element of " - + f"'{expected.__class__.__name__}.{field_name}' be a member of " - + f"'{self.__class__.__name__}.{field_name}'\n\nElement: {element}" - + f"\n\nActual value: {actual_value}" - + f"\n\nExpected value: {expected_value}" + f"'{expected.__class__.__name__}.{field_name}' be a member of " + f"'{self.__class__.__name__}.{field_name}'\n\nElement: {element}" + f"\n\nActual value: {actual_value}" + f"\n\nExpected value: {expected_value}" ) return True # ---------------------- Common data used throughout the tests ---------------------- # Meta information -token = "12345" +token = "12345" # noqa: S105 number = 1 repository = "user/testing" sha = "a06212024d8f1c339c55c5ea4568ech155368c21" From 95e6b40d3bfc0e2c7dc12d6827f6e73903cddc86 Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Tue, 22 Aug 2023 17:05:21 +0200 Subject: [PATCH 2/3] line-length = 88 --- pyproject.toml | 2 +- tests/utils.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9d7efdf..ae68fe3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,7 +76,7 @@ select = [ # "TRY", s# tryceratops ] ignore = ["N802", "PGH003", "RUF012", "SLF001"] -line-length = 125 +line-length = 88 target-version = "py38" [tool.ruff.mccabe] diff --git a/tests/utils.py b/tests/utils.py index 5c21677..dc01984 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -84,7 +84,10 @@ async def getitem(self, url: str, **kwargs: Any) -> Any: if getitem_return is not None: return getitem_return[url] else: - msg = "Expected to be supplied with the 'getitem' argument when instantiating the object but got 'None' instead." + msg = ( + "Expected to be supplied with the 'getitem' argument when " + "instantiating the object but got 'None' instead." + ) raise AssertionError(msg) async def getiter(self, url: str, **kwargs: Any) -> AsyncGenerator[Any, None]: @@ -93,7 +96,10 @@ async def getiter(self, url: str, **kwargs: Any) -> AsyncGenerator[Any, None]: if getiter_return is not None: data = getiter_return[url] else: - msg = "Expected to be supplied with the 'getiter' argument when instantiating the object but got 'None' instead." + msg = ( + "Expected to be supplied with the 'getiter' argument when " + "instantiating the object but got 'None' instead." + ) raise AssertionError(msg) if isinstance(data, dict) and "items" in data: data = data["items"] From ef155638615964b1a8caebf2cf419a2eae7bad8b Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Tue, 22 Aug 2023 17:08:41 +0200 Subject: [PATCH 3/3] noqa: SIM102 --- algorithms_keeper/parser/rules/naming_convention.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/algorithms_keeper/parser/rules/naming_convention.py b/algorithms_keeper/parser/rules/naming_convention.py index 11d362a..8e4d1b0 100644 --- a/algorithms_keeper/parser/rules/naming_convention.py +++ b/algorithms_keeper/parser/rules/naming_convention.py @@ -168,11 +168,10 @@ def visit_Attribute(self, node: cst.Attribute) -> None: def visit_Element(self, node: cst.Element) -> None: # We only care about elements in *List* or *Tuple* specifically coming from # inside the multiple assignments. - if self._assigntarget_counter > 0 and m.matches( - node, m.Element(value=m.Name()) - ): - nodename = cst.ensure_type(node.value, cst.Name).value - self._validate_nodename(node, nodename, NamingConvention.SNAKE_CASE) + if self._assigntarget_counter > 0: # noqa: SIM102 + if m.matches(node, m.Element(value=m.Name())): + nodename = cst.ensure_type(node.value, cst.Name).value + self._validate_nodename(node, nodename, NamingConvention.SNAKE_CASE) def visit_For(self, node: cst.For) -> None: if m.matches(node, m.For(target=m.Name())):