From e47fba0b6a554ab03c2dfed3c2d0b47c5b2cc6f5 Mon Sep 17 00:00:00 2001 From: Zhenyu Yao <111329301+zhenyu-ms@users.noreply.github.com> Date: Thu, 20 Jun 2024 13:44:58 +0800 Subject: [PATCH] Fix/empty dict to dependencies indicating all drivers start concurrently (#1103) * fix dependencies behaviour at empty dict * add newsfrag * address review comments --- doc/en/multitest.rst | 7 ++- ...0_changed.test_dependencies_empty_dict.rst | 1 + testplan/testing/base.py | 3 +- testplan/testing/environment/base.py | 5 +- .../testplan/testing/test_environment.py | 58 +++++++++++++------ 5 files changed, 50 insertions(+), 24 deletions(-) create mode 100644 doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst diff --git a/doc/en/multitest.rst b/doc/en/multitest.rst index 4affc0cde..9ce6dee20 100644 --- a/doc/en/multitest.rst +++ b/doc/en/multitest.rst @@ -67,8 +67,11 @@ A MultiTest instance can be constructed from the following parameters: :py:class:`drivers `. Drivers on the value side should only start after drivers on the key side are fully started. When specified, Testplan will try to schedule more drivers - starting parallelly based on the dependencies. Click - :ref:`here ` for more information. + starting concurrently based on the dependencies. Drivers included in + ``environment`` while not presented in ``dependencies`` will be started + concurrently at the very beginning. Using empty dict here to instruct all + drivers to start concurrently. Click :ref:`here ` for more + information. * **Runtime Information**: The environment always contains a member called ``runtime_info`` which contains information about the current state of the diff --git a/doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst b/doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst new file mode 100644 index 000000000..c4eb10913 --- /dev/null +++ b/doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst @@ -0,0 +1 @@ +Users now can make all drivers of a certain ``Test`` start concurrently by passing an empty dict to ``dependencies`` argument. \ No newline at end of file diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 7412daeb1..8ef1b5624 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -440,8 +440,7 @@ def _set_dependencies(self) -> None: deps = parse_dependency(self.cfg.dependencies()) else: deps = self.cfg.dependencies - if deps: - self.resources.set_dependency(deps) + self.resources.set_dependency(deps) def _start_resource(self) -> None: if len(self.resources) == 0: diff --git a/testplan/testing/environment/base.py b/testplan/testing/environment/base.py index 7f7110377..fa1cff576 100644 --- a/testplan/testing/environment/base.py +++ b/testplan/testing/environment/base.py @@ -58,7 +58,10 @@ def __init__(self, parent: Optional["Test"] = None): self.__dict__["_rt_dependency"]: Optional[DriverDepGraph] = None self.__dict__["_pocketwatches"]: Dict[str, DriverPocketwatch] = dict() - def set_dependency(self, dependency: DriverDepGraph): + def set_dependency(self, dependency: Optional[DriverDepGraph]): + if dependency is None: + return + for d in dependency.vertices.values(): if ( d.uid() not in self._resources diff --git a/tests/functional/testplan/testing/test_environment.py b/tests/functional/testplan/testing/test_environment.py index 8afe40a33..70c12b8c2 100644 --- a/tests/functional/testplan/testing/test_environment.py +++ b/tests/functional/testplan/testing/test_environment.py @@ -55,40 +55,60 @@ def __missing__(self, key): return self[key] +def _assert_orig_dep(env): + assert env.__dict__["_orig_dependency"] is not None + + @skip_on_windows(reason="Bash files skipped on Windows.") @pytest.mark.parametrize( - "driver_dependencies", + "driver_dependencies, use_callable", [ - [("a", "b")], - [("a", "b"), ("b", "c"), ("a", "c")], - [("a", "b"), ("c", "d"), ("a", "d"), ("c", "b")], + ([], False), + ([], True), + ([("a", "b")], False), + ([("a", "b"), ("b", "c"), ("a", "c")], True), + ([("a", "b"), ("c", "d"), ("a", "d"), ("c", "b")], False), ], ) -def test_testing_environment(mockplan, driver_dependencies, named_temp_file): - +def test_testing_environment( + mockplan, named_temp_file, driver_dependencies, use_callable +): drivers = DriverGeneratorDict(named_temp_file) - dependencies = defaultdict(list) + for k in ["a", "b"]: + drivers[k] = MyDriver(named_temp_file, k) + predicates = list() - for side_a, side_b in driver_dependencies: - dependencies[drivers[side_a]].append(drivers[side_b]) - predicates.append( - lambda line_of: line_of[f"{drivers[side_a].name}_POST"] - < line_of[f"{drivers[side_b].name}_PRE"] - ) + if driver_dependencies is None: + dependencies = None + else: + dependencies = defaultdict(list) + for side_a, side_b in driver_dependencies: + dependencies[drivers[side_a]].append(drivers[side_b]) + predicates.append( + lambda line_of: line_of[f"{drivers[side_a].name}_POST"] + < line_of[f"{drivers[side_b].name}_PRE"] + ) - mockplan.add_resource(ProcessPool(name="I'm not local.")) mockplan.schedule( target=DummyTest( name="MyTest", binary=binary_path, - environment=list(drivers.values()), - dependencies=dependencies, + environment=lambda: list(drivers.values()) + if use_callable + else list(drivers.values()), + dependencies=lambda: dependencies + if use_callable + else dependencies, + after_start=_assert_orig_dep, ), - resource="I'm not local.", + resource=None, ) assert mockplan.run().success is True - assert "lifespan" in mockplan.result.report.entries[0].children[0].timer - assert "lifespan" in mockplan.result.report.entries[0].children[1].timer + + for i in range(len(drivers)): + assert ( + "lifespan" in mockplan.result.report.entries[0].children[i].timer + ) with open(named_temp_file, "r") as f: lines = f.read().splitlines()