Merge pull request #333 from espressif/feat/support-add_target_as_mar…

…kers_with_amount

feat: support --add-target-as-marker-with-amount

.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/[email protected]
+        with:
+          extra_args: --files ${{ steps.changed-files.outputs.all_changed_files }}

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:

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])

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,30 +546,38 @@ 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
 
 
 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()