From 1ddc17a7302382befb32ec635d2a2d94d0181479 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Fri, 17 Jan 2025 12:41:30 +0100 Subject: [PATCH 1/2] feat: support --add-target-as-marker-with-amount --- pytest-embedded/pytest_embedded/plugin.py | 53 +++++-- pytest-embedded/pytest_embedded/utils.py | 17 +++ pytest-embedded/tests/test_base.py | 166 +++++++++++++++++----- ruff.toml | 49 ------- 4 files changed, 185 insertions(+), 100 deletions(-) diff --git a/pytest-embedded/pytest_embedded/plugin.py b/pytest-embedded/pytest_embedded/plugin.py index 07a07a7a..1c568352 100644 --- a/pytest-embedded/pytest_embedded/plugin.py +++ b/pytest-embedded/pytest_embedded/plugin.py @@ -52,6 +52,7 @@ PackageNotInstalledError, UnknownServiceError, find_by_suffix, + targets_to_marker, to_list, utcnow_str, ) @@ -168,10 +169,17 @@ def pytest_addoption(parser): esp_group.addoption('--beta-target', help='serial target beta version chip type. (Default: same as [--target])') esp_group.addoption( '--add-target-as-marker', - help='add target param as a function marker. Useful in CI with runners with different tags.' + help='[DEPRECATED, use --add-target-as-marker-with-amount instead] ' + 'add target param as a function marker. Useful in CI with runners with different tags.' 'y/yes/true for True and n/no/false for False. ' '(Default: False, parametrization not supported, `|` will be escaped to `-`)', ) + esp_group.addoption( + '--add-target-as-marker-with-amount', + help='add target param as a function marker with the amount of the target. Useful in CI with runners with ' + 'different tags. y/yes/true for True and n/no/false for False. ' + '(Default: False, parametrization not supported, `|` will be escaped to `+`)', + ) esp_group.addoption( '--flash-port', help='serial port for flashing. Only set this value when the flashing port is different from the serial port.' @@ -1203,6 +1211,7 @@ def pytest_configure(config: Config) -> None: check_duplicates=config.getoption('check_duplicates', False), prettify_junit_report=_str_bool(config.getoption('prettify_junit_report', False)), add_target_as_marker=_str_bool(config.getoption('add_target_as_marker', False)), + add_target_as_marker_with_amount=_str_bool(config.getoption('add_target_as_marker_with_amount', False)), ) config.pluginmanager.register(config.stash[_pytest_embedded_key]) config.addinivalue_line('markers', 'skip_if_soc') @@ -1223,12 +1232,14 @@ def __init__( check_duplicates: bool = False, prettify_junit_report: bool = False, add_target_as_marker: bool = False, + add_target_as_marker_with_amount: bool = False, ): self.parallel_count = parallel_count self.parallel_index = parallel_index self.check_duplicates = check_duplicates self.prettify_junit_report = prettify_junit_report self.add_target_as_marker = add_target_as_marker + self.add_target_as_marker_with_amount = add_target_as_marker_with_amount @staticmethod def _raise_dut_failed_cases_if_exists(duts: t.Iterable[Dut]) -> None: @@ -1253,8 +1264,37 @@ def _duplicate_items(items: t.List[_T]) -> t.List[_T]: return duplicates + @staticmethod + def get_param(item: Function, key: str, default: t.Any = None) -> t.Any: + # funcargs is not calculated while collection + # callspec is something defined in parametrize + if not hasattr(item, 'callspec'): + return default + + return item.callspec.params.get(key, default) or default + @pytest.hookimpl(hookwrapper=True, trylast=True) def pytest_collection_modifyitems(self, config: Config, items: t.List[Function]): + # ------ add marker based on target ------ + if self.add_target_as_marker_with_amount or self.add_target_as_marker: + for item in items: + item_target = self.get_param(item, 'target') + if not item_target: + continue + + if not isinstance(item_target, str): + raise ValueError(f'`target` should be a string, got {type(item_target)} instead') + + # --add-target-as-marker-with-amount + count = self.get_param(item, 'count', 1) + if self.add_target_as_marker_with_amount: + _marker = targets_to_marker(to_list(parse_multi_dut_args(count, item_target))) + if self.add_target_as_marker: + _marker = '-'.join(to_list(parse_multi_dut_args(count, item_target))) + + item.add_marker(_marker) + + # ------ pytest.mark.skip_if_soc ------ for item in items: skip_marker = item.get_closest_marker('skip_if_soc') if not skip_marker: @@ -1289,17 +1329,6 @@ def pytest_collection_modifyitems(self, config: Config, items: t.List[Function]) reason = f'Filtered by {skip_marker.args[0]}, for {target}.' item.add_marker(pytest.mark.skip(reason=reason)) - if self.add_target_as_marker: - for item in items: - item_target = item.callspec.getparam('target') - - if not isinstance(item_target, str): - raise ValueError(f'`target` should be a string, got {type(item_target)} instead') - - if item_target: - # https://github.com/pytest-dev/pytest/pull/12277 - item.add_marker(item_target.replace('|', '-')) # '|' is not supported until 8.2.0 - yield if self.check_duplicates: diff --git a/pytest-embedded/pytest_embedded/utils.py b/pytest-embedded/pytest_embedded/utils.py index 5ec4d7d4..a9593f7a 100644 --- a/pytest-embedded/pytest_embedded/utils.py +++ b/pytest-embedded/pytest_embedded/utils.py @@ -6,6 +6,7 @@ import os import re import typing as t +from collections import defaultdict from dataclasses import dataclass if t.TYPE_CHECKING: @@ -361,3 +362,19 @@ def wrapped(self, *args, **kwargs): return wrapped return decorator + + +def targets_to_marker(targets: t.Iterable[str]) -> str: + """ + Convert esp targets to pytest marker with amount, "+" for multiple targets types + + For example: + - [esp32s2, esp32s2, esp32s3] -> esp32s2_2+esp32s3 + - [esp32] -> esp32 + - [esp32, esp32s2] -> esp32+esp32s2 + """ + t_amount = defaultdict(int) + for target in sorted(targets): + t_amount[target] += 1 + + return '+'.join([f'{t}_{t_amount[t]}' if t_amount[t] > 1 else t for t in t_amount]) diff --git a/pytest-embedded/tests/test_base.py b/pytest-embedded/tests/test_base.py index 5f1a494f..d2db85ce 100644 --- a/pytest-embedded/tests/test_base.py +++ b/pytest-embedded/tests/test_base.py @@ -6,9 +6,7 @@ def test_help(testdir): - result = testdir.runpytest( - '--help' - ) + result = testdir.runpytest('--help') result.stdout.fnmatch_lines([ 'embedded:', @@ -88,8 +86,10 @@ def test_fixture_redirect(pexpect_proc, dut, redirect): result = testdir.runpytest( '-s', - '--app-path', os.path.join(testdir.tmpdir, 'hello_world_esp32'), - '--root-logdir', os.getcwd(), + '--app-path', + os.path.join(testdir.tmpdir, 'hello_world_esp32'), + '--root-logdir', + os.getcwd(), ) result.assert_outcomes(passed=6) @@ -127,10 +127,10 @@ def test_fixture_redirect(dut, redirect): result = testdir.runpytest( '-s', - '--count', 2, - '--app-path', f'{os.path.join(testdir.tmpdir, "hello_world_esp32")}' - f'|' - f'{os.path.join(testdir.tmpdir, "hello_world_esp32c3")}', + '--count', + 2, + '--app-path', + f'{os.path.join(testdir.tmpdir, "hello_world_esp32")}|{os.path.join(testdir.tmpdir, "hello_world_esp32c3")}', ) result.assert_outcomes(passed=5) @@ -150,17 +150,20 @@ def test_default_app_path(app): result.assert_outcomes(passed=1) -@pytest.mark.parametrize('parallel_count, parallel_index, res', [ - (5, 1, 1), - (5, 6, 0), - (4, 1, 2), - (4, 3, 1), - (4, 4, 0), - (3, 1, 2), - (3, 3, 1), - (2, 1, 3), - (2, 2, 2), -]) +@pytest.mark.parametrize( + 'parallel_count, parallel_index, res', + [ + (5, 1, 1), + (5, 6, 0), + (4, 1, 2), + (4, 3, 1), + (4, 4, 0), + (3, 1, 2), + (3, 3, 1), + (2, 1, 3), + (2, 2, 2), + ], +) def test_parallel_run(testdir, parallel_count, parallel_index, res): testdir.makepyfile(r""" def test_1(dut): pass @@ -171,8 +174,10 @@ def test_5(dut): pass """) result = testdir.runpytest( - '--parallel-count', parallel_count, - '--parallel-index', parallel_index, + '--parallel-count', + parallel_count, + '--parallel-index', + parallel_index, ) result.assert_outcomes(passed=res) @@ -541,14 +546,18 @@ def test_set_log_extension(dut): def test_duplicate_case_name(testdir, capsys): - testdir.makepyfile(test_duplicate_name_one=r""" + testdir.makepyfile( + test_duplicate_name_one=r""" def test_duplicate_case(): pass - """) - testdir.makepyfile(test_duplicate_name_two=""" + """ + ) + testdir.makepyfile( + test_duplicate_name_two=""" def test_duplicate_case(): pass - """) + """ + ) testdir.runpytest('--check-duplicates', 'y') assert "ValueError: Duplicated test function names: ['test_duplicate_case']" in capsys.readouterr().out @@ -556,15 +565,19 @@ def test_duplicate_case(): def test_duplicate_module_name(testdir, capsys): test_sub_dir = str(testdir.mkpydir('test_dir')) - dup_module_path = testdir.makepyfile(test_duplicate_module=r""" + dup_module_path = testdir.makepyfile( + test_duplicate_module=r""" def test_duplicate_one(): pass - """) + """ + ) os.rename(f'{dup_module_path}', os.path.join(test_sub_dir, dup_module_path.basename)) - testdir.makepyfile(test_duplicate_module=r""" + testdir.makepyfile( + test_duplicate_module=r""" def test_duplicate_two(): pass - """) + """ + ) testdir.runpytest('--check-duplicates', 'y') assert "ValueError: Duplicated test scripts: ['test_duplicate_module.py']" in capsys.readouterr().out @@ -582,12 +595,14 @@ def test_temp_disable_packages(): import pytest_embedded_idf # noqa -@pytest.mark.temp_disable_packages('pytest_embedded_serial', - 'pytest_embedded_serial_esp', - 'pytest_embedded_idf', - 'pytest_embedded_qemu', - 'pytest_embedded_jtag', - 'pytest_embedded_arduino') +@pytest.mark.temp_disable_packages( + 'pytest_embedded_serial', + 'pytest_embedded_serial_esp', + 'pytest_embedded_idf', + 'pytest_embedded_qemu', + 'pytest_embedded_jtag', + 'pytest_embedded_arduino', +) def test_quick_example(testdir): testdir.makepyfile(r""" from pytest_embedded import Dut @@ -626,9 +641,82 @@ def test_unclosed_file_handler(test_input, dut): assert test_input == test_input """) result = testdir.runpytest( - '--embedded-services', 'serial', - '--count', '3', - '--port', '/dev/ttyUSB0|/dev/ttyUSB1|/dev/ttyUSB2', + '--embedded-services', + 'serial', + '--count', + '3', + '--port', + '/dev/ttyUSB0|/dev/ttyUSB1|/dev/ttyUSB2', '-x', # fail at the first fail ) result.assert_outcomes(passed=1024) + + +class TestTargetMarkers: + def test_add_target_as_marker_simple(self, pytester): + pytester.makepyfile(""" + import pytest + @pytest.mark.parametrize('target', ['esp32'], indirect=True) + def test_example(target): + pass + """) + + result = pytester.runpytest('--add-target-as-marker', 'y') + + result.assert_outcomes(passed=1) + result.stdout.fnmatch_lines([ + '*Unknown pytest.mark.esp32 - is this a typo?*' # Check marker is present + ]) + + def test_add_target_as_marker_multi_target(self, pytester): + pytester.makepyfile(""" + import pytest + @pytest.mark.parametrize('target,count', [ + ('esp32|esp8266', 2), + ('esp32', 2), + ('esp32|esp8266|esp32s2', 3), + ], indirect=True) + def test_example(target): + pass + """) + + result = pytester.runpytest('--add-target-as-marker', 'y') + + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines([ + '*Unknown pytest.mark.esp32-esp8266 - is this a typo?*', + '*Unknown pytest.mark.esp32-esp32 - is this a typo?*', + '*Unknown pytest.mark.esp32-esp8266-esp32s2 - is this a typo?*', + ]) + + def test_add_target_as_marker_with_amount(self, pytester): + pytester.makepyfile(""" + import pytest + @pytest.mark.parametrize('target,count', [ + ('esp32|esp8266', 2), + ('esp32', 2), + ('esp32|esp8266|esp32s2', 3), + ], indirect=True) + def test_example(target): + pass + """) + + result = pytester.runpytest('--add-target-as-marker-with-amount', 'y', '-vvvv') + + result.assert_outcomes(passed=3) + result.stdout.fnmatch_lines([ + '*Unknown pytest.mark.esp32+esp8266 - is this a typo?*', + '*Unknown pytest.mark.esp32_2 - is this a typo?*', + '*Unknown pytest.mark.esp32+esp32s2+esp8266 - is this a typo?*', + ]) + + def test_no_target_no_marker(self, pytester): + pytester.makepyfile(""" + def test_example(): + pass + """) + + result = pytester.runpytest('--add-target-as-marker', 'y', '--embedded-services', 'esp', '--target', 'esp32') + + result.assert_outcomes(passed=1) + assert 'Unknown pytest.mark.esp32 - is this a typo?' not in result.stdout.str() diff --git a/ruff.toml b/ruff.toml index 71af517c..c4bbf26f 100644 --- a/ruff.toml +++ b/ruff.toml @@ -6,60 +6,12 @@ select = [ 'F', # Pyflakes 'E', # pycodestyle 'W', # pycodestyle -# 'C90', # mccabe 'I', # isort -# 'N', # pep8-naming -# 'D', # pydocstyle 'UP', # pyupgrade 'YTT', # flake8-2020 -# 'ANN', # flake8-annotations -# 'ASYNC', # flake8-async -# 'TRIO', # flake8-trio -# 'S', # flake8-bandit -# 'BLE', # flake8-blind-except -# 'FBT', # flake8-boolean-trap -# 'B', # flake8-bugbear 'A', # flake8-builtins -# 'COM', # flake8-commas -# 'CPY', # flake8-copyright -# 'C4', # flake8-comprehensions -# 'DTZ', # flake8-datetimez -# 'T10', # flake8-debugger -# 'DJ', # flake8-django -# 'EM', # flake8-errmsg -# 'EXE', # flake8-executable -# 'FA', # flake8-future-annotations -# 'ISC', # flake8-implicit-str-concat -# 'ICN', # flake8-import-conventions -# 'G', # flake8-logging-format -# 'INP', # flake8-no-pep420 -# 'PIE', # flake8-pie -# 'T20', # flake8-print -# 'PYI', # flake8-pyi -# 'PT', # flake8-pytest-style -# 'Q', # flake8-quotes -# 'RSE', # flake8-raise -# 'RET', # flake8-return -# 'SLF', # flake8-self -# 'SLOT', # flake8-slots -# 'SIM', # flake8-simplify -# 'TID', # flake8-tidy-imports -# 'TCH', # flake8-type-checking -# 'INT', # flake8-gettext 'ARG', # flake8-unused-arguments -# 'PTH', # flake8-use-pathlib -# 'TD', # flake8-todos -# 'FIX', # flake8-fixme 'ERA', # eradicate -# 'PD', # pandas-vet -# 'PGH', # pygrep-hooks -# 'PL', # Pylint -# 'TRY', # tryceratops -# 'FLY', # flynt -# 'NPY', # NumPy-specific rules -# 'AIR', # Airflow -# 'PERF', # Perflint -# 'FURB', # refurb 'LOG', # flake8-logging 'RUF', # Ruff-specific rules ] @@ -67,7 +19,6 @@ select = [ [format] quote-style = "single" exclude = [ - "**/tests/*", "pytest-embedded-jtag/pytest_embedded_jtag/_telnetlib/telnetlib.py" ] docstring-code-format = true From d8cac16d14b02d4bf4639d498905e158d9601ec1 Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Fri, 17 Jan 2025 13:32:06 +0100 Subject: [PATCH 2/2] ci: run pre-check for changed files only --- .github/workflows/check-pre-commit.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-pre-commit.yml b/.github/workflows/check-pre-commit.yml index d45fd63d..168bd18d 100644 --- a/.github/workflows/check-pre-commit.yml +++ b/.github/workflows/check-pre-commit.yml @@ -1,14 +1,18 @@ -name: Check pre-commit +name: pre-commit check on: pull_request: jobs: - check-pre-commit: + pre-commit: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 with: fetch-depth: 0 - uses: actions/setup-python@v5 + - id: changed-files + uses: tj-actions/changed-files@v45 - uses: pre-commit/action@v3.0.1 + with: + extra_args: --files ${{ steps.changed-files.outputs.all_changed_files }}