From dbe773acffb66d3400ca7662451e9546f6559b8d Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:17:33 +0800 Subject: [PATCH 1/3] fix exc saving in step_results of resource hook steps --- testplan/testing/base.py | 14 +++++++++++--- testplan/testing/multitest/base.py | 17 +++-------------- testplan/testing/py_test.py | 10 +++++----- .../testing/multitest/test_pre_post_steps.py | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 205cb7aef..375891ffc 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -662,7 +662,7 @@ def _create_case_or_override( return case_report def _run_resource_hook( - self, hook: Callable, hook_name: str, suite_name: str + self, hook: Optional[Callable], hook_name: str, suite_name: str ) -> None: # TODO: env or env, result signature is mandatory not an "if" """ @@ -698,7 +698,11 @@ def _run_resource_hook( interface.check_signature(hook, ["env"]) hook_args = (runtime_env,) with compose_contexts(*self._get_hook_context(case_report)): - hook(*hook_args) + try: + res = hook(*hook_args) + except Exception as e: + res = e + raise case_report.extend(case_result.serialized_entries) case_report.attachments.extend(case_result.attachments) @@ -710,8 +714,12 @@ def _run_resource_hook( self._xfail(pattern, case_report) case_report.runtime_status = RuntimeStatus.FINISHED + if isinstance(res, Exception): + raise res + return res + def _dry_run_resource_hook( - self, hook: Callable, hook_name: str, suite_name: str + self, hook: Optional[Callable], hook_name: str, suite_name: str ) -> None: if not hook: return diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index 814e67b14..f31fbe0ec 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -224,9 +224,6 @@ def get_options(cls): and tup[1] > 1, ), ), - config.ConfigOption("multi_part_uid", default=None): Or( - None, lambda x: callable(x) - ), config.ConfigOption("fix_spec_path", default=None): Or( None, And(str, os.path.exists) ), @@ -261,9 +258,6 @@ class MultiTest(testing_base.Test): :param part: Execute only a part of the total testcases. MultiTest needs to know which part of the total it is. Only works with Multitest. :type part: ``tuple`` of (``int``, ``int``) - :param multi_part_uid: Custom function to overwrite the uid of test entity - if `part` attribute is defined, otherwise use default implementation. - :type multi_part_uid: ``callable`` :type result: :py:class:`~testplan.testing.multitest.result.result.Result` :param fix_spec_path: Path of fix specification file. :type fix_spec_path: ``NoneType`` or ``str``. @@ -296,7 +290,6 @@ def __init__( thread_pool_size=0, max_thread_pool_size=10, part=None, - multi_part_uid=None, before_start=None, after_start=None, before_stop=None, @@ -345,12 +338,8 @@ def uid(self): A Multitest part instance should not have the same uid as its name. """ if self.cfg.part: - return ( - self.cfg.multi_part_uid(self.cfg.name, self.cfg.part) - if self.cfg.multi_part_uid - else TEST_PART_PATTERN_FORMAT_STRING.format( - self.cfg.name, self.cfg.part[0], self.cfg.part[1] - ) + return TEST_PART_PATTERN_FORMAT_STRING.format( + self.cfg.name, self.cfg.part[0], self.cfg.part[1] ) else: return self.cfg.name @@ -529,7 +518,7 @@ def run_testcases_iter( self, testsuite_pattern: str = "*", testcase_pattern: str = "*", - shallow_report: Dict = None, + shallow_report: Optional[Dict] = None, ) -> Generator: """ Run all testcases and yield testcase reports. diff --git a/testplan/testing/py_test.py b/testplan/testing/py_test.py index 820a230ca..2067badbb 100644 --- a/testplan/testing/py_test.py +++ b/testplan/testing/py_test.py @@ -27,7 +27,7 @@ from testplan.testing.multitest.entries.stdout.base import ( registry as stdout_registry, ) -from testplan.testing.result import Result as MultiTestResult +from testplan.testing.result import Result # Regex for parsing suite and case name and case parameters _CASE_REGEX = re.compile( @@ -49,9 +49,9 @@ def get_options(cls): "target": Or(str, [str]), ConfigOption("select", default=""): str, ConfigOption("extra_args", default=None): Or([str], None), - ConfigOption( - "result", default=MultiTestResult - ): validation.is_subclass(MultiTestResult), + ConfigOption("result", default=Result): validation.is_subclass( + Result + ), } @@ -85,7 +85,7 @@ def __init__( description=None, select="", extra_args=None, - result=MultiTestResult, + result=Result, **options ): options.update(self.filter_locals(locals())) diff --git a/tests/functional/testplan/testing/multitest/test_pre_post_steps.py b/tests/functional/testplan/testing/multitest/test_pre_post_steps.py index 3d8a98825..328dd0b83 100644 --- a/tests/functional/testplan/testing/multitest/test_pre_post_steps.py +++ b/tests/functional/testplan/testing/multitest/test_pre_post_steps.py @@ -200,3 +200,20 @@ def test_no_pre_post_steps(mockplan): ) check_report(expected_report, mockplan.report) + + +def test_before_start_error_skip_remaining(mockplan): + multitest = MultiTest( + name="MyMultiTest", + suites=[MySuite()], + before_start=lambda env: 1 / 0, + ) + mockplan.add(multitest) + mockplan.run() + + mt_rpt = mockplan.report.entries[0] + # only before start report exists + assert len(mt_rpt.entries) == 1 + assert mt_rpt.entries[0].entries[0].status == Status.ERROR + assert len(mt_rpt.entries[0].entries[0].logs) == 1 + assert mt_rpt.entries[0].entries[0].logs[0]["levelname"] == "ERROR" From c8c79b6a566287fa1c02b15d4614b29af7c58af2 Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Wed, 8 Jan 2025 18:18:49 +0800 Subject: [PATCH 2/3] fix tests --- testplan/testing/multitest/base.py | 8 ++++++ .../multitest/test_error_handler_hook.py | 1 + .../testing/multitest/test_multitest_parts.py | 27 ++++++++----------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index f31fbe0ec..0fc8e3ad3 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -303,6 +303,14 @@ def __init__( ): self._tags_index = None + if "multi_part_uid" in options: + # might be replaced by multi_part_name_func + warnings.warn( + "MultiTest uid can no longer be customised, please remove ``multi_part_uid`` argument.", + DeprecationWarning, + ) + del options["multi_part_uid"] + options.update(self.filter_locals(locals())) super(MultiTest, self).__init__(**options) diff --git a/tests/functional/testplan/testing/multitest/test_error_handler_hook.py b/tests/functional/testplan/testing/multitest/test_error_handler_hook.py index 717cd9929..b33d2ccce 100644 --- a/tests/functional/testplan/testing/multitest/test_error_handler_hook.py +++ b/tests/functional/testplan/testing/multitest/test_error_handler_hook.py @@ -157,6 +157,7 @@ def test_multitest_hook_failure(mockplan): TestGroupReport( name="MyMultitest", category=ReportCategories.MULTITEST, + status_override=Status.ERROR, entries=[ TestGroupReport( name="Environment Start", diff --git a/tests/functional/testplan/testing/multitest/test_multitest_parts.py b/tests/functional/testplan/testing/multitest/test_multitest_parts.py index a0c7b42f1..e13e7c24c 100644 --- a/tests/functional/testplan/testing/multitest/test_multitest_parts.py +++ b/tests/functional/testplan/testing/multitest/test_multitest_parts.py @@ -1,6 +1,8 @@ from itertools import chain, cycle, repeat from operator import eq +import pytest + from testplan import TestplanMock from testplan.report import Status from testplan.runners.pools.base import Pool as ThreadPool @@ -55,9 +57,7 @@ def get_mtest(part_tuple=None): def get_mtest_with_custom_uid(part_tuple=None): - # XXX: abolish multi_part_uid, may rename it to multi_part_report_name? - # XXX: or we may still accept customised uids, but we need to rewrite - # XXX: current filters + # NOTE: multi_part_uid is noop now return MultiTest( name="MTest", suites=[Suite1(), Suite2()], @@ -163,6 +163,9 @@ def test_multi_parts_duplicate_part(): """ Execute MultiTest parts with a part of MultiTest has been scheduled twice and automatically be filtered out. + --- + since multi_part_uid has no functional effect now, original test is invalid + preserved for simple backward compatibility test, i.e. no exception raised """ plan = TestplanMock(name="plan", merge_scheduled_parts=True) pool = ThreadPool(name="MyThreadPool", size=2) @@ -172,20 +175,12 @@ def test_multi_parts_duplicate_part(): task = Task(target=get_mtest_with_custom_uid(part_tuple=(idx, 3))) plan.schedule(task, resource="MyThreadPool") - task = Task(target=get_mtest_with_custom_uid(part_tuple=(1, 3))) - plan.schedule(task, resource="MyThreadPool") - - assert len(plan._tests) == 4 - - assert plan.run().run is False + with pytest.raises(ValueError): + task = Task(target=get_mtest_with_custom_uid(part_tuple=(1, 3))) + plan.schedule(task, resource="MyThreadPool") - assert len(plan.report.entries) == 5 # one placeholder report & 4 siblings - assert len(plan.report.entries[0].entries) == 0 # already cleared - assert plan.report.status == Status.ERROR # Testplan result - assert ( - "duplicate MultiTest parts had been scheduled" - in plan.report.entries[0].logs[0]["message"] - ) + assert len(plan._tests) == 3 + assert plan.run().run is True def test_multi_parts_missing_parts(): From aed2e7ac42694203a3ec48c3d36527c65d240b6a Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Fri, 10 Jan 2025 11:10:17 +0800 Subject: [PATCH 3/3] update test --- .../testplan/testing/multitest/test_multitest_parts.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/functional/testplan/testing/multitest/test_multitest_parts.py b/tests/functional/testplan/testing/multitest/test_multitest_parts.py index e13e7c24c..933c07237 100644 --- a/tests/functional/testplan/testing/multitest/test_multitest_parts.py +++ b/tests/functional/testplan/testing/multitest/test_multitest_parts.py @@ -1,3 +1,4 @@ +import re from itertools import chain, cycle, repeat from operator import eq @@ -159,7 +160,7 @@ def test_multi_parts_incorrect_schedule(): ) -def test_multi_parts_duplicate_part(): +def test_multi_parts_duplicate_part(mocker): """ Execute MultiTest parts with a part of MultiTest has been scheduled twice and automatically be filtered out. @@ -167,6 +168,7 @@ def test_multi_parts_duplicate_part(): since multi_part_uid has no functional effect now, original test is invalid preserved for simple backward compatibility test, i.e. no exception raised """ + mock_warn = mocker.patch("warnings.warn") plan = TestplanMock(name="plan", merge_scheduled_parts=True) pool = ThreadPool(name="MyThreadPool", size=2) plan.add_resource(pool) @@ -179,6 +181,8 @@ def test_multi_parts_duplicate_part(): task = Task(target=get_mtest_with_custom_uid(part_tuple=(1, 3))) plan.schedule(task, resource="MyThreadPool") + assert mock_warn.call_count == 4 + assert re.search(r"remove.*multi_part_uid", mock_warn.call_args[0][0]) assert len(plan._tests) == 3 assert plan.run().run is True