Fix!/fix certain naming issues, better patterns for multitests with parts

doc/newsfragments/2626_changed.pattern_with_parts.rst

@@ -1,5 +1,4 @@
 Filtering Patterns now are easier to use with MultiTest with parts.
 
-    * Both patterns with part specified or not could be used to filter MultiTest with parts.
-    * Applying patterns won't cause certain testcase appear in some different part now.
-    * Part feature has been tuned to generate more even parts in term of number of testcases.
+    * Support filtering MultiTest with both part-specified patterns and part-unspecified patterns.
+    * Applying filtering patterns will not change testcase shuffling - the same testcase will always end up in the same part of a MultiTest.

testplan/testing/filtering.py

@@ -261,7 +261,6 @@ class Pattern(Filter):
     """
 
     MAX_LEVEL = 3
-    DELIMITER = ":"
     ALL_MATCH = "*"
 
     category = FilterCategory.PATTERN
@@ -270,7 +269,6 @@ def __init__(self, pattern, match_definition=False):
         self.pattern = pattern
         self.match_definition = match_definition
         self.parse_pattern(pattern)
-        # self.test_pattern, self.suite_pattern, self.case_pattern = patterns
 
     def __eq__(self, other):
         return (
@@ -283,8 +281,10 @@ def __repr__(self):
         return '{}(pattern="{}")'.format(self.__class__.__name__, self.pattern)
 
     def parse_pattern(self, pattern: str) -> List[str]:
-        # ":" would be used as delimiter
-        patterns = [s for s in pattern.split(self.DELIMITER) if s]
+        # ":" or "::" can be used as delimiter
+        patterns = (
+            pattern.split("::") if "::" in pattern else pattern.split(":")
+        )
 
         if len(patterns) > self.MAX_LEVEL:
             raise ValueError(
@@ -332,9 +332,7 @@ def parse_pattern(self, pattern: str) -> List[str]:
     def filter_test(self, test: "Test"):
         if isinstance(self.test_pattern, tuple):
             if not hasattr(test.cfg, "part"):
-                raise ValueError(
-                    f"Invalid pattern, Part feature not implemented for {type(test).__qualname__}."
-                )
+                return False
 
             name_p, (cur_part_p, ttl_part_p) = self.test_pattern
 

testplan/testing/listing.py

@@ -7,16 +7,19 @@
 from argparse import Action, ArgumentParser, Namespace
 from enum import Enum
 from os import PathLike
-from typing import List, Union, Sequence, Any, Tuple
+from typing import TYPE_CHECKING, List, Tuple, Union
 from urllib.parse import urlparse
 
-from testplan.common.utils.parser import ArgMixin
 from testplan.common.utils.logger import TESTPLAN_LOGGER
-
+from testplan.common.utils.parser import ArgMixin
 from testplan.testing import tagging
+from testplan.testing.common import TEST_PART_FORMAT_STRING
 from testplan.testing.multitest import MultiTest
 from testplan.testing.multitest.test_metadata import TestPlanMetadata
 
+if TYPE_CHECKING:
+    from testplan.testing.base import Test
+
 INDENT = " "
 MAX_TESTCASES = 25
 
@@ -51,6 +54,18 @@ def get_output(self, instance):
         raise NotImplementedError
 
 
+def test_name(test_instance: "Test") -> str:
+    if hasattr(test_instance.cfg, "part") and isinstance(
+        test_instance.cfg.part, tuple
+    ):
+        return TEST_PART_FORMAT_STRING.format(
+            test_instance.name,
+            test_instance.cfg.part[0],
+            test_instance.cfg.part[1],
+        )
+    return test_instance.name
+
+
 class ExpandedNameLister(BaseLister):
     """
     Lists names of the items within the test context:
@@ -71,7 +86,7 @@ class ExpandedNameLister(BaseLister):
     DESCRIPTION = "List tests in readable format."
 
     def format_instance(self, instance):
-        return instance.name
+        return test_name(instance)
 
     def format_suite(self, instance, suite):
         return suite if isinstance(suite, str) else suite.name
@@ -132,7 +147,7 @@ class ExpandedPatternLister(ExpandedNameLister):
     DESCRIPTION = "List tests in `--patterns` / `--tags` compatible format."
 
     def format_instance(self, instance):
-        return instance.name
+        return test_name(instance)
 
     def apply_tag_label(self, pattern, obj):
         if obj.__tags__:
@@ -143,17 +158,18 @@ def apply_tag_label(self, pattern, obj):
 
     def format_suite(self, instance, suite):
         if not isinstance(instance, MultiTest):
-            return "{}::{}".format(instance.name, suite)
+            return "{}:{}".format(test_name(instance), suite)
 
-        pattern = "{}::{}".format(instance.name, suite.name)
+        pattern = "{}:{}".format(test_name(instance), suite.name)
         return self.apply_tag_label(pattern, suite)
 
     def format_testcase(self, instance, suite, testcase):
-
         if not isinstance(instance, MultiTest):
-            return "{}::{}::{}".format(instance.name, suite, testcase)
+            return "{}:{}:{}".format(test_name(instance), suite, testcase)
 
-        pattern = "{}::{}::{}".format(instance.name, suite.name, testcase.name)
+        pattern = "{}:{}:{}".format(
+            test_name(instance), suite.name, testcase.name
+        )
         return self.apply_tag_label(pattern, testcase)
 
 
@@ -223,7 +239,7 @@ def get_output(self, instance):
                 " suite{num_suites_plural},"
                 " {num_testcases}"
                 " testcase{num_testcases_plural})".format(
-                    instance_name=instance.name,
+                    instance_name=test_name(instance),
                     num_suites=len(suites),
                     num_suites_plural="s" if len(suites) > 1 else "",
                     num_testcases=total_testcases,

testplan/testing/multitest/base.py

@@ -376,26 +376,23 @@ def get_test_context(self):
         ctx = []
         sorted_suites = self.cfg.test_sorter.sorted_testsuites(self.cfg.suites)
 
-        g_offset = 0
         for suite in sorted_suites:
             testcases = suite.get_testcases()
 
-            if self.cfg.part and self.cfg.part[1] > 1:
-                offset = len(testcases)
-                testcases = [
-                    testcase
-                    for (idx, testcase) in enumerate(testcases)
-                    if (idx + g_offset) % self.cfg.part[1] == self.cfg.part[0]
-                ]
-                g_offset += offset
-
             sorted_testcases = (
                 testcases
                 if getattr(suite, "strict_order", False)
                 or not hasattr(self.cfg, "test_sorter")
                 else self.cfg.test_sorter.sorted_testcases(suite, testcases)
             )
 
+            if self.cfg.part:
+                sorted_testcases = [
+                    testcase
+                    for (idx, testcase) in enumerate(sorted_testcases)
+                    if (idx) % self.cfg.part[1] == self.cfg.part[0]
+                ]
+
             testcases_to_run = [
                 case
                 for case in sorted_testcases
@@ -724,7 +721,7 @@ def stop_test_resources(self):
 
     def get_metadata(self) -> TestMetadata:
         return TestMetadata(
-            name=self.name,
+            name=self.uid(),
             description=self.cfg.description,
             test_suites=[get_suite_metadata(suite) for suite in self.suites],
         )
@@ -859,6 +856,7 @@ def _new_parametrized_group_report(self, param_template, param_method):
         return TestGroupReport(
             name=param_method.name,
             description=strings.get_docstring(param_method),
+            instance_name=param_template,
             uid=param_template,
             category=ReportCategories.PARAMETRIZATION,
             tags=param_method.__tags__,

testplan/testing/multitest/parametrization.py

@@ -279,13 +279,13 @@ def _parametrization_name_func_wrapper(func_name, kwargs):
     generated_name = parametrization_name_func(func_name, kwargs)
 
     if not PYTHON_VARIABLE_REGEX.match(generated_name):
-        # Generated method name is not a valid Python attribute name,
-        # i.e. "{func_name}__1", "{func_name}__2" will be used.
+        # Generated method name is not a valid Python attribute name.
+        # Index suffixed names, e.g. "{func_name}__1", "{func_name}__2", will be used.
         return func_name
 
     if len(generated_name) > MAX_METHOD_NAME_LENGTH:
-        # Generated method name is a bit too long. Simple index suffixes
-        # e.g. "{func_name}__1", "{func_name}__2" will be used.
+        # Generated method name is a bit too long.
+        # Index suffixed names, e.g. "{func_name}__1", "{func_name}__2", will be used.
         return func_name
 
     return generated_name

testplan/testing/multitest/suite.py

@@ -364,7 +364,7 @@ def _ensure_unique_generated_testcase_names(names, functions):
     # Functions should have different __name__ attributes after the step above
     names = list(itertools.chain(names, (func.__name__ for func in functions)))
     if len(set(names)) != len(names):
-        raise RuntimeError("Duplicate testcase __name__ found.")
+        raise RuntimeError("Internal error, duplicate case name found.")
 
 
 def _testsuite(klass):

tests/functional/testplan/testing/multitest/test_multitest_parts.py

@@ -1,11 +1,11 @@
 import itertools
 
+from testplan.testing.multitest import MultiTest, testsuite, testcase
+
 from testplan import TestplanMock
-from testplan.common.utils.testing import check_report_context
-from testplan.report import Status
 from testplan.runners.pools.base import Pool as ThreadPool
 from testplan.runners.pools.tasks import Task
-from testplan.testing.multitest import MultiTest, testcase, testsuite
+from testplan.report import Status
 
 
 @testsuite
@@ -76,69 +76,19 @@ def test_multi_parts_not_merged():
 
     assert plan.run().run is True
 
-    check_report_context(
-        plan.report,
-        [
-            (
-                "MTest - part(0/3)",
-                [
-                    (
-                        "Suite1",
-                        [
-                            (
-                                "test_true",
-                                [
-                                    "test_true <val=0>",
-                                    "test_true <val=3>",
-                                    "test_true <val=6>",
-                                    "test_true <val=9>",
-                                ],
-                            )
-                        ],
-                    ),
-                    ("Suite2", [("test_false", ["test_false <val=''>"])]),
-                ],
-            ),
-            (
-                "MTest - part(1/3)",
-                [
-                    (
-                        "Suite1",
-                        [
-                            (
-                                "test_true",
-                                [
-                                    "test_true <val=1>",
-                                    "test_true <val=4>",
-                                    "test_true <val=7>",
-                                ],
-                            )
-                        ],
-                    ),
-                    ("Suite2", [("test_false", ["test_false <val=False>"])]),
-                ],
-            ),
-            (
-                "MTest - part(2/3)",
-                [
-                    (
-                        "Suite1",
-                        [
-                            (
-                                "test_true",
-                                [
-                                    "test_true <val=2>",
-                                    "test_true <val=5>",
-                                    "test_true <val=8>",
-                                ],
-                            )
-                        ],
-                    ),
-                    ("Suite2", [("test_false", ["test_false <val=None>"])]),
-                ],
-            ),
-        ],
-    )
+    assert len(plan.report.entries) == 3
+    assert plan.report.entries[0].name == "MTest - part(0/3)"
+    assert plan.report.entries[1].name == "MTest - part(1/3)"
+    assert plan.report.entries[2].name == "MTest - part(2/3)"
+    assert len(plan.report.entries[0].entries) == 2  # 2 suites
+    assert plan.report.entries[0].entries[0].name == "Suite1"
+    assert plan.report.entries[0].entries[1].name == "Suite2"
+    assert len(plan.report.entries[0].entries[0].entries) == 1  # param group
+    assert plan.report.entries[0].entries[0].entries[0].name == "test_true"
+    assert len(plan.report.entries[0].entries[1].entries) == 1  # param group
+    assert plan.report.entries[0].entries[1].entries[0].name == "test_false"
+    assert len(plan.report.entries[0].entries[0].entries[0].entries) == 4
+    assert len(plan.report.entries[0].entries[1].entries[0].entries) == 1
 
 
 def test_multi_parts_merged():