Fix/save exception raised by resource hooks to step_results

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

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,
@@ -310,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)
 
@@ -345,12 +346,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 +526,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.

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

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",

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

@@ -1,6 +1,9 @@
+import re
 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 +58,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()],
@@ -159,11 +160,15 @@ 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.
+    ---
+    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)
@@ -172,20 +177,14 @@ 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 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
 
 
 def test_multi_parts_missing_parts():

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"