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 16, 2023
1 parent bbe57c3 commit b5ed5bd
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 144 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
38 changes: 27 additions & 11 deletions testplan/testing/listing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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__:
Expand All @@ -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)


Expand Down Expand Up @@ -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,
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
Loading

0 comments on commit b5ed5bd

Please sign in to comment.