From 3c86f06f6777be06b11a32712152f80df584a655 Mon Sep 17 00:00:00 2001 From: Tatu Aalto Date: Sat, 1 Feb 2025 19:33:52 +0200 Subject: [PATCH] Lint with ruff and mypy --- .github/workflows/push.yaml | 14 ++-- pyproject.toml | 53 +++++++++++- requirements-dev.txt | 6 +- robotstatuschecker.py | 159 ++++++++++++++++++------------------ setup.py | 38 ++++----- tasks.py | 35 +++++++- test/run.py | 31 ++++--- test/tests.robot | 34 ++++---- 8 files changed, 226 insertions(+), 144 deletions(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 22e5aae..cba6aa0 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -20,18 +20,14 @@ jobs: - name: Install dependencies on Python if: matrix.python-version != 'pypy3' run: | - python -m pip install --upgrade pip - pip install -r requirements-dev.txt - pip install robotframework==${{ matrix.rf-version }} - - name: Install dependencies on pypy3 - if: matrix.python-version == 'pypy3' - run: | - python -m pip install --upgrade pip - pip install robotframework==${{ matrix.rf-version }} + python -m pip install --upgrade pip uv + uv pip install --python ${{ matrix.python-version }} --system -r requirements-dev.txt + uv pip install --python ${{ matrix.python-version }} --system robotframework==${{ matrix.rf-version }} - name: Run lint - if: matrix.python-version != 'pypy3' run: | inv lint + env: + GITHUB_ACTIONS_RUNNIN_RUFF_LINT: 'true' - name: Run tests run: | python --version diff --git a/pyproject.toml b/pyproject.toml index 97f5037..b6421c9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,52 @@ -[tool.black] +[tool.ruff] +exclude = [ + "__pycache__", + ".venv", + ".git", +] line-length = 100 -target-version = ['py37'] +lint.ignore = [ + "T201", # Use print + "N818", # Exception name check is disabled + "PTH100", # abspath is OK + "B904", # Raise inside except block + "RET503", # Return statement in finally block +] +target-version = "py38" +lint.select = [ + "E", + "F", + "W", + "C90", + "I", + "N", + "B", + "PYI", + "PL", + "PTH", + "UP", + "A", + "C4", + "DTZ", + "ISC", + "ICN", + "INP", + "PIE", + "T20", + "PYI", + "PT", + "RSE", + "RET", + "SIM", + "RUF" +] +[tool.ruff.lint.per-file-ignores] +"tasks.py" = [ + "T201", + "PTH123", + "PTH120" +] + +[[tool.mypy.overrides]] +module = ["robot.api.*"] +follow_untyped_imports = true diff --git a/requirements-dev.txt b/requirements-dev.txt index ed0b8ed..b6dd7c7 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,7 +2,7 @@ invoke >= 1.4.1 rellu >= 0.6 twine >= 3.4.1 wheel >= 0.36.2 -black >= 21.9b0 -flake8 >= 3.9.2 robotframework-tidy >= 1.6.1 -isort >= 5.9.3 \ No newline at end of file +ruff >= 0.9.4 +mypy >= 1.14.1 +uv >= 0.5.26 diff --git a/robotstatuschecker.py b/robotstatuschecker.py index c9ba78a..9c4edf7 100755 --- a/robotstatuschecker.py +++ b/robotstatuschecker.py @@ -28,7 +28,7 @@ Command-line usage: - python -m robotstatuschecker infile [outfile] + python -m robotstatuschecker infile [outfile] [true] Programmatic usage: @@ -44,25 +44,20 @@ from pathlib import Path from robot.api import ExecutionResult, ResultVisitor -from robot.model import BodyItem -from robot.result import Keyword, Message, Result, TestCase -from robot.utils import Matcher - +from robot.model import BodyItem # type: ignore +from robot.result import Keyword, Message, Result, TestCase # type: ignore +from robot.utils import Matcher # type: ignore __version__ = "4.0.0.dev1" -def process_output(in_path: 'str|Path', - out_path: 'str|Path|None' = None, - verbose: bool = True) -> int: +def process_output(in_path: "str|Path", out_path: "str|Path|None" = None) -> int: """The main programmatic entry point to status checker. Args: in_path: Path to Robot Framework XML output file to process. out_path: Path where to write processed output. If not given, `in_path` is edited in place. - verbose: When `True` (default), prints both `in_path` and - `out_path` to console. Returns: Number of failed tests after post-processing. @@ -71,17 +66,15 @@ def process_output(in_path: 'str|Path', # without also resolving symlinks. in_path = abspath(normpath(in_path)) out_path = abspath(normpath(out_path)) if out_path else None - if verbose: - print(f"Checking {in_path}") + print(f"Checking {in_path}") result = StatusChecker().check(in_path, out_path) - if verbose and out_path: + if out_path: print(f"Output: {out_path}") return result.return_code class StatusChecker(ResultVisitor): - - def check(self, in_path: str, out_path: 'str|None' = None) -> Result: + def check(self, in_path: str, out_path: "str|None" = None) -> Result: result = ExecutionResult(in_path) result.suite.visit(self) result.save(out_path) @@ -89,17 +82,18 @@ def check(self, in_path: str, out_path: 'str|None' = None) -> Result: def visit_test(self, test: TestCase): expected = Expected(test.doc) - if StatusAndMessageChecker(expected).check(test): - if LogMessageChecker(expected).check(test): - self._mark_checked(test) + if StatusAndMessageChecker(expected).check(test) and LogMessageChecker(expected).check( + test + ): + self._mark_checked(test) def _mark_checked(self, test: TestCase): - message = 'Test status has been checked.' - if test.status != 'PASS': - message += f'\n\nOriginal status: {test.status}' + message = "Test status has been checked." + if test.status != "PASS": + message += f"\n\nOriginal status: {test.status}" if test.message: - message += f'\n\nOriginal message:\n{test.message}' - test.status = 'PASS' + message += f"\n\nOriginal message:\n{test.message}" + test.status = "PASS" test.message = message def visit_keyword(self, kw: Keyword): @@ -107,7 +101,6 @@ def visit_keyword(self, kw: Keyword): class Expected: - def __init__(self, doc: str): self.status = self._get_status(doc) self.message = self._get_message(doc) @@ -126,38 +119,37 @@ def _get_message(self, doc: str) -> str: return doc.split(status, 1)[1].split("LOG", 1)[0].strip() return "" - def _get_logs(self, doc: str) -> 'list[ExpectedLog]': + def _get_logs(self, doc: str) -> "list[ExpectedLog]": return [ExpectedLog(item) for item in doc.split("LOG")[1:]] class ExpectedLog: - def __init__(self, doc: str): try: locator, message = doc.strip().split(maxsplit=1) except ValueError: - locator, message = doc.strip(), '' + locator, message = doc.strip(), "" self.locator_str = locator # ':' is legacy message separator. It was softly deprecated in v4.0. - self.locator = [self._parse_locator_part(part) - for part in locator.replace(':', '.').split('.')] + self.locator = [ + self._parse_locator_part(part) for part in locator.replace(":", ".").split(".") + ] self.level, self.message = self._split_level(message) - def _parse_locator_part(self, part: str) -> 'int|str': + def _parse_locator_part(self, part: str) -> "int|str": try: return int(part) except ValueError: return part.lower() - def _split_level(self, message: str) -> 'tuple[str, str]': + def _split_level(self, message: str) -> "tuple[str, str]": for level in ["TRACE", "DEBUG", "INFO", "WARN", "FAIL", "ERROR", "ANY"]: if message.startswith(level): - return level, message[len(level):].strip() + return level, message[len(level) :].strip() return "INFO", message class BaseChecker: - def _message_matches(self, actual: str, expected: str) -> bool: if actual == expected: return True @@ -190,7 +182,6 @@ def _fail(self, test: TestCase, message: str): class StatusAndMessageChecker(BaseChecker): - def __init__(self, expected: Expected): self.status = expected.status self.message = expected.message @@ -206,7 +197,6 @@ def check(self, test: TestCase) -> bool: class LogMessageChecker(BaseChecker): - def __init__(self, expected: Expected): self.logs = expected.logs @@ -225,12 +215,13 @@ def _check(self, test: TestCase, expected: ExpectedLog): item = test for level, part in enumerate(expected.locator): if isinstance(item, Message): - locator = '.'.join(str(part) for part in expected.locator[:level]) - raise NotFound(f"Locator '{locator}' matches message and " - f"it cannot have child '{part}'.") - if part == '*': + locator = ".".join(str(part) for part in expected.locator[:level]) + raise NotFound( + f"Locator '{locator}' matches message and it cannot have child '{part}'." + ) + if part == "*": return self._check_message_by_wildcard(item, expected, level) - elif isinstance(part, int): + if isinstance(part, int): item = self._get_item_by_index(item, part, expected, level) else: item = self._get_item_by_attribute(item, part, expected, level) @@ -239,11 +230,14 @@ def _check(self, test: TestCase, expected: ExpectedLog): assert isinstance(item, Message) self._check_message(item, expected) - def _get_item_by_index(self, parent: 'TestCase|BodyItem', - index: int, - expected: ExpectedLog, - level: 'int|None' = None, - require_message: bool = False) -> 'BodyItem': + def _get_item_by_index( + self, + parent: "TestCase|BodyItem", + index: int, + expected: ExpectedLog, + level: "int|None" = None, + require_message: bool = False, + ) -> "BodyItem": prefix = self._get_error_prefix(parent, expected.locator[:level]) try: item = parent.body[index - 1] @@ -251,31 +245,32 @@ def _get_item_by_index(self, parent: 'TestCase|BodyItem', return item raise NotFound(f"{prefix} does not have message in index {index}.") except IndexError: - if (expected.message == "NONE" - and (level is None or len(expected.locator) == level + 1)): + if expected.message == "NONE" and (level is None or len(expected.locator) == level + 1): raise CheckOk raise NotFound(f"{prefix} does not have child in index {index}.") - def _get_item_by_attribute(self, parent: 'TestCase|BodyItem', - attribute: str, - expected: ExpectedLog, - level: int) -> 'BodyItem': + def _get_item_by_attribute( + self, + parent: "TestCase|BodyItem", + attribute: str, + expected: ExpectedLog, + level: int, + ) -> "BodyItem": item = getattr(parent, attribute, None) if item: return item prefix = self._get_error_prefix(parent, expected.locator[:level]) raise NotFound(f"{prefix} does not have '{attribute}'.") - def _get_error_prefix(self, parent: 'TestCase|BodyItem', - locator: 'list[str|int]') -> str: - typ = getattr(parent, 'type', 'TEST') # `TestCase.type` is new in RF 7.2. + def _get_error_prefix(self, parent: "TestCase|BodyItem", locator: "list[str|int]") -> str: + typ = getattr(parent, "type", "TEST") # `TestCase.type` is new in RF 7.2. prefix = f"{typ.title()} '{self._get_name(parent)}'" if locator: - locator = '.'.join(str(part) for part in locator) - prefix += f" (locator '{locator}')" + locator_str = ".".join(str(part) for part in locator) + prefix += f" (locator '{locator_str}')" return prefix - def _get_name(self, item: 'TestCase|BodyItem') -> str: + def _get_name(self, item: "TestCase|BodyItem") -> str: if isinstance(item, TestCase): return item.name if isinstance(item, Keyword): @@ -283,40 +278,48 @@ def _get_name(self, item: 'TestCase|BodyItem') -> str: return getattr(item, "full_name", item.name) return item.type - def _check_message_by_wildcard(self, parent: 'TestCase|Message', - expected: ExpectedLog, - level: int): + def _check_message_by_wildcard( + self, parent: "TestCase|Message", expected: ExpectedLog, level: int + ): if len(expected.locator) != level + 1: - raise InvalidUsage(f"Message index wildcard '*' can be used only as " - f"the last locator item, got '{expected.locator_str}.") + raise InvalidUsage( + f"Message index wildcard '*' can be used only as " + f"the last locator item, got '{expected.locator_str}." + ) if expected.message == "NONE": - raise InvalidUsage("Message index wildcard '*' cannot be used with " - "'NONE' message.") + raise InvalidUsage("Message index wildcard '*' cannot be used with 'NONE' message.") for item in parent.body: - if (isinstance(item, Message) - and self._message_matches(item.message.strip(), expected.message) - and self._level_matches(item.level, expected.level)): + if ( + isinstance(item, Message) + and self._message_matches(item.message.strip(), expected.message) + and self._level_matches(item.level, expected.level) + ): return prefix = self._get_error_prefix(parent, expected.locator[:level]) - raise AssertionError(f"{prefix} has no message matching '{expected.message}' " - f"with level {expected.level}.") + raise AssertionError( + f"{prefix} has no message matching '{expected.message}' with level {expected.level}." + ) def _level_matches(self, actual: str, expected: str) -> bool: - return actual == expected or expected == "ANY" + return actual == expected or expected == "ANY" # noqa: PLR1714 def _check_message(self, message: Message, expected: ExpectedLog): name = self._get_name(message.parent) locator = expected.locator_str if not self._message_matches(message.message.strip(), expected.message): - raise AssertionError(f"Keyword '{name}' has wrong message " - f"(locator '{locator}').\n\n" - f"Expected:\n{expected.message}\n\n" - f"Actual:\n{message.message}") + raise AssertionError( + f"Keyword '{name}' has wrong message " + f"(locator '{locator}').\n\n" + f"Expected:\n{expected.message}\n\n" + f"Actual:\n{message.message}" + ) if not self._level_matches(message.level, expected.level): - raise AssertionError(f"Keyword '{name}' has message with wrong level " - f"(locator '{locator}').\n\n" - f"Expected: {expected.level}\n" - f"Actual: {message.level}") + raise AssertionError( + f"Keyword '{name}' has message with wrong level " + f"(locator '{locator}').\n\n" + f"Expected: {expected.level}\n" + f"Actual: {message.level}" + ) class InvalidUsage(Exception): diff --git a/setup.py b/setup.py index 7034d08..f68bf4e 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ from os.path import abspath, dirname, join import re -NAME = 'robotstatuschecker' +NAME = "robotstatuschecker" CLASSIFIERS = """ Development Status :: 5 - Production/Stable License :: OSI Approved :: Apache Software License @@ -15,26 +15,26 @@ Framework :: Robot Framework """.strip().splitlines() CURDIR = dirname(abspath(__file__)) -with open(join(CURDIR, NAME+'.py')) as f: +with open(join(CURDIR, NAME + ".py")) as f: VERSION = re.search('\n__version__ = "(.*)"\n', f.read()).group(1) -with open(join(CURDIR, 'README.rst')) as f: +with open(join(CURDIR, "README.rst")) as f: README = f.read() setup( - name = NAME, - version = VERSION, - author = 'Robot Framework Developers', - author_email = 'robotframework@gmail.com', - url = 'https://github.com/robotframework/statuschecker', - download_url = 'https://pypi.python.org/pypi/robotstatuschecker', - license = 'Apache License 2.0', - description = 'A tool for checking that Robot Framework test cases ' - 'have expected statuses and log messages.', - long_description = README, - keywords = 'robotframework testing testautomation atdd', - platforms = 'any', - classifiers = CLASSIFIERS, - py_modules = ['robotstatuschecker'], - install_requires = ['robotframework'], - python_requires = '>=3.6,<4.0' + name=NAME, + version=VERSION, + author="Robot Framework Developers", + author_email="robotframework@gmail.com", + url="https://github.com/robotframework/statuschecker", + download_url="https://pypi.python.org/pypi/robotstatuschecker", + license="Apache License 2.0", + description="A tool for checking that Robot Framework test cases " + "have expected statuses and log messages.", + long_description=README, + keywords="robotframework testing testautomation atdd", + platforms="any", + classifiers=CLASSIFIERS, + py_modules=["robotstatuschecker"], + install_requires=["robotframework"], + python_requires=">=3.6,<4.0", ) diff --git a/tasks.py b/tasks.py index 2940d24..a1790e3 100644 --- a/tasks.py +++ b/tasks.py @@ -1,3 +1,18 @@ +# Copyright 2008-2015 Nokia Networks +# Copyright 2016- Robot Framework Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os import sys from pathlib import Path @@ -22,6 +37,8 @@ .. _issue tracker: https://github.com/robotframework/SeleniumLibrary/issues?q=milestone%3A{version.milestone} """ # noqa +IS_GITHUB_ACTIONS = os.environ.get("GITHUB_ACTIONS_RUNNIN_RUFF_LINT") + @task def set_version(ctx, version): @@ -91,11 +108,21 @@ def init_labels(ctx, username=None, password=None): def lint(ctx): """Run linters - Flake8, Black and robotframework-tidy + Ruff, mypy and robotframework-tidy """ - ctx.run("black --config pyproject.toml tasks.py robotstatuschecker.py") - ctx.run("flake8 --config .flake8 tasks.py robotstatuschecker.py") - ctx.run("isort tasks.py robotstatuschecker.py") + ruff_format = "ruff format" + ruff_check = "ruff check" + if IS_GITHUB_ACTIONS: + ruff_format = f"{ruff_format} --check" + else: + ruff_check = f"{ruff_check} --fix" + ruff_check = f"{ruff_check} robotstatuschecker.py" + print(f"Running ruff format· {ruff_format}") + ctx.run(ruff_format) + print(f"Running ruff check: {ruff_check}") + ctx.run("ruff check robotstatuschecker.py") + print("Running mypy") + ctx.run("mypy --exclude \\.venv robotstatuschecker.py") tidy_command = [ "robotidy", "--lineseparator", diff --git a/test/run.py b/test/run.py index 73141d8..0b33f0e 100755 --- a/test/run.py +++ b/test/run.py @@ -15,6 +15,7 @@ from robotstatuschecker import process_output # noqa + def check_tests(robot_file): output = _run_tests_and_process_output(robot_file) result = ExecutionResult(output) @@ -50,23 +51,27 @@ def visit_test(self, test): def _verify(self, test): status, message = self._get_expected(test) if test.status != status: - raise AssertionError(f"Test '{test.name}' had wrong status.\n" - f"{'- ' * 39}\n" - f"Expected: {status}\n" - f"Message:\n{message}\n" - f"{'- ' * 39}\n" - f"Actual: {test.status}\n" - f"Message:\n{test.message}") + raise AssertionError( + f"Test '{test.name}' had wrong status.\n" + f"{'- ' * 39}\n" + f"Expected: {status}\n" + f"Message:\n{message}\n" + f"{'- ' * 39}\n" + f"Actual: {test.status}\n" + f"Message:\n{test.message}" + ) if test.message != message: - raise AssertionError(f"Test '{test.name}' had wrong message.\n" - f"{'- ' * 39}\n" - f"Expected:\n{message}\n" - f"{'- ' * 39}\n" - f"Actual:\n{test.message}") + raise AssertionError( + f"Test '{test.name}' had wrong message.\n" + f"{'- ' * 39}\n" + f"Expected:\n{message}\n" + f"{'- ' * 39}\n" + f"Actual:\n{test.message}" + ) def _get_expected(self, test): kw = test.setup or test.body[0] - assert kw.name == 'Status', 'Status keyword missing!' + assert kw.name == "Status", "Status keyword missing!" return (kw.body[1].messages[0].message, kw.body[2].messages[0].message) def print_status(self): diff --git a/test/tests.robot b/test/tests.robot index 6b2cffb..92dfdbb 100644 --- a/test/tests.robot +++ b/test/tests.robot @@ -1,9 +1,11 @@ *** Settings *** -Suite Setup Log Suite setup -Suite Teardown Log Suite teardown +Suite Setup Log Suite setup +Suite Teardown Log Suite teardown + *** Variables *** -${CHECKED} Test status has been checked. +${CHECKED} = Test status has been checked. + *** Test Cases *** Implicit PASS @@ -216,7 +218,7 @@ Invalid wildcard usage Log message with REGEXP [Documentation] LOG 2 REGEXP: H[ei]l{2}o w\\w+! LOG 2 REGEXP: Hell.* - ... LOG 3 REGEXP: Multi.*message + ... LOG 3 REGEXP: Multi.*message Status PASS Log Hello world! Log Multi\nline\nmessage @@ -229,7 +231,7 @@ Log message with GLOB Log message with STARTS [Documentation] LOG 2 STARTS: Hello LOG 2 STARTS: Hell - ... LOG 3 STARTS: Multi + ... LOG 3 STARTS: Multi Status PASS Log Hello world! Log Multi\nline\nmessage @@ -241,7 +243,7 @@ Empty log message NONE log message [Documentation] - ... LOG 2 NONE + ... LOG 2 NONE ... LOG 2:1 NONE ... LOG 3:1 Message ... LOG 3:2 NONE @@ -265,23 +267,23 @@ Test Setup Check Is Done By SETUP Marker Error When No Setup [Documentation] ... LOG setup.1:1 PASS - ... LOG 2:1 KALA + ... LOG 2:1 KALA Status FAIL Test '${TEST NAME}' does not have 'setup'. Log KALA Test Setup Check Is Done By SETUP Marker and wildcard is used [Documentation] - ... LOG SETUP:10 NONE + ... LOG SETUP:10 NONE ... LOG SETUP.2:* PASS - ... LOG SETUP.2 PASS - ... LOG 1:* HAUKI + ... LOG SETUP.2 PASS + ... LOG 1:* HAUKI [Setup] Status PASS Log HAUKI Test Teardown Check Is Done By TEARDOWN Marker [Documentation] ... LOG TEARDOWN:1 foobar - ... LOG TEARDOWN foobar + ... LOG TEARDOWN foobar Status PASS [Teardown] Log foobar @@ -293,15 +295,15 @@ Error When No Teardown Test Teardown Check Is Done By TEARDOWN Marker and wildcard is used [Documentation] ... LOG TEARDOWN:* foobar - ... LOG TEARDOWN foobar + ... LOG TEARDOWN foobar Status PASS [Teardown] Log foobar Keyword teardown [Documentation] Keyword setup isn't tested because it isn't supported by all - ... Robot versions we support at the moment. + ... Robot versions we support at the moment. ... - ... LOG 2.teardown.1 DEBUG User Keyword + ... LOG 2.teardown.1 DEBUG User Keyword ... LOG 2.TEARDOWN.1:1 DEBUG User Keyword Status PASS Keyword with teardown @@ -421,10 +423,10 @@ FAILURE: Log locator parent with wildcard matches message Status FAIL Locator '2.1' matches message and it cannot have child '*'. Log Hello! -#Control structures +# Control structures # Fail FIXME # -#Invalid attribute +# Invalid attribute # Fail FIXME