Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@zhenyu-ms
zhenyu-ms committed Oct 13, 2023
1 parent bbe57c3 commit fbf59da
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 100 deletions.
5 changes: 2 additions & 3 deletions doc/newsfragments/2626_changed.pattern_with_parts.rst
View file
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 5 additions & 7 deletions testplan/testing/filtering.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ class Pattern(Filter):
"""

MAX_LEVEL = 3
DELIMITER = ":"
ALL_MATCH = "*"

category = FilterCategory.PATTERN
Expand All @@ -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 (
Expand All @@ -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(
Expand Down Expand Up @@ -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

Expand Down
36 changes: 28 additions & 8 deletions testplan/testing/listing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
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 Any, List, Sequence, 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

Expand Down Expand Up @@ -71,6 +71,12 @@ class ExpandedNameLister(BaseLister):
DESCRIPTION = "List tests in readable format."

def format_instance(self, instance):
if hasattr(instance.cfg, "part") and isinstance(
instance.cfg.part, tuple
):
return TEST_PART_FORMAT_STRING.format(
instance.name, instance.cfg.part[0], instance.cfg.part[1]
)
return instance.name

def format_suite(self, instance, suite):
Expand Down Expand Up @@ -132,6 +138,12 @@ class ExpandedPatternLister(ExpandedNameLister):
DESCRIPTION = "List tests in `--patterns` / `--tags` compatible format."

def format_instance(self, instance):
if hasattr(instance.cfg, "part") and isinstance(
instance.cfg.part, tuple
):
return TEST_PART_FORMAT_STRING.format(
instance.name, instance.cfg.part[0], instance.cfg.part[1]
)
return instance.name

def apply_tag_label(self, pattern, obj):
Expand All @@ -143,17 +155,17 @@ 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(instance.name, suite)

pattern = "{}::{}".format(instance.name, suite.name)
pattern = "{}:{}".format(instance.name, 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(instance.name, suite, testcase)

pattern = "{}::{}::{}".format(instance.name, suite.name, testcase.name)
pattern = "{}:{}:{}".format(instance.name, suite.name, testcase.name)
return self.apply_tag_label(pattern, testcase)


Expand Down Expand Up @@ -215,6 +227,14 @@ class CountLister(BaseLister):

def get_output(self, instance):
test_context = instance.test_context
if hasattr(instance.cfg, "part") and isinstance(
instance.cfg.part, tuple
):
instance_name = TEST_PART_FORMAT_STRING.format(
instance.name, instance.cfg.part[0], instance.cfg.part[1]
)
else:
instance_name = instance.name
if test_context:
suites, testcase_lists = zip(*test_context)
total_testcases = sum(map(len, testcase_lists))
Expand All @@ -223,7 +243,7 @@ def get_output(self, instance):
" suite{num_suites_plural},"
" {num_testcases}"
" testcase{num_testcases_plural})".format(
instance_name=instance.name,
instance_name=instance_name,
num_suites=len(suites),
num_suites_plural="s" if len(suites) > 1 else "",
num_testcases=total_testcases,
Expand Down
20 changes: 9 additions & 11 deletions testplan/testing/multitest/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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],
)
Expand Down Expand Up @@ -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__,
Expand Down
8 changes: 4 additions & 4 deletions testplan/testing/multitest/parametrization.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testplan/testing/multitest/suite.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
82 changes: 16 additions & 66 deletions tests/functional/testplan/testing/multitest/test_multitest_parts.py
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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():
Expand Down

0 comments on commit fbf59da

Please sign in to comment.