From 88beaed6f090a37623e114c9a18e2325a0f48e52 Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Tue, 18 Jun 2024 19:06:54 +0800 Subject: [PATCH 1/3] fix dependencies behaviour at empty dict --- testplan/testing/base.py | 7 ++- .../testplan/testing/test_environment.py | 62 +++++++++++++------ 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 7412daeb1..1f6a3cb9f 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -112,7 +112,7 @@ def get_options(cls): [Or(Resource, RemoteDriver)], validate_func() ), ConfigOption("dependencies", default=None): Or( - Use(parse_dependency), validate_func() + None, Use(parse_dependency), validate_func() ), ConfigOption("initial_context", default={}): Or( dict, validate_func() @@ -437,10 +437,11 @@ def _build_environment(self) -> None: def _set_dependencies(self) -> None: if callable(self.cfg.dependencies): - deps = parse_dependency(self.cfg.dependencies()) + ret = self.cfg.dependencies() + deps = parse_dependency(ret) if ret is not None else None else: deps = self.cfg.dependencies - if deps: + if deps is not None: self.resources.set_dependency(deps) def _start_resource(self) -> None: diff --git a/tests/functional/testplan/testing/test_environment.py b/tests/functional/testplan/testing/test_environment.py index 8afe40a33..09252dad8 100644 --- a/tests/functional/testplan/testing/test_environment.py +++ b/tests/functional/testplan/testing/test_environment.py @@ -55,40 +55,64 @@ def __missing__(self, key): return self[key] +def _assert_orig_dep(env): + if env["is_set"] is True: + assert env.__dict__["_orig_dependency"] is not None + else: + assert env.__dict__["_orig_dependency"] is None + + @skip_on_windows(reason="Bash files skipped on Windows.") @pytest.mark.parametrize( - "driver_dependencies", + "driver_dependencies, is_lift, is_set", [ - [("a", "b")], - [("a", "b"), ("b", "c"), ("a", "c")], - [("a", "b"), ("c", "d"), ("a", "d"), ("c", "b")], + (None, False, False), + (None, True, False), + ([], False, True), + ([], True, True), + ([("a", "b")], False, True), + ([("a", "b"), ("b", "c"), ("a", "c")], True, True), + ([("a", "b"), ("c", "d"), ("a", "d"), ("c", "b")], False, True), ], ) -def test_testing_environment(mockplan, driver_dependencies, named_temp_file): - +def test_testing_environment( + mockplan, named_temp_file, driver_dependencies, is_lift, is_set +): 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, + initial_context={"is_set": is_set}, + environment=lambda: list(drivers.values()) + if is_lift + else list(drivers.values()), + dependencies=lambda: dependencies if is_lift 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() From cb77cda70e0fe1055f42967890e145628ec54c2c Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:12:58 +0800 Subject: [PATCH 2/3] add newsfrag --- doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/newsfragments/2950_changed.test_dependencies_empty_dict.rst 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 From 512b09d8b865555306325a336087edea94c998bf Mon Sep 17 00:00:00 2001 From: zhenyu-ms <111329301+zhenyu-ms@users.noreply.github.com> Date: Thu, 20 Jun 2024 11:41:51 +0800 Subject: [PATCH 3/3] address review comments --- doc/en/multitest.rst | 7 +++-- testplan/testing/base.py | 8 ++---- testplan/testing/environment/base.py | 5 +++- .../testplan/testing/test_environment.py | 28 ++++++++----------- 4 files changed, 24 insertions(+), 24 deletions(-) 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/testplan/testing/base.py b/testplan/testing/base.py index 1f6a3cb9f..8ef1b5624 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -112,7 +112,7 @@ def get_options(cls): [Or(Resource, RemoteDriver)], validate_func() ), ConfigOption("dependencies", default=None): Or( - None, Use(parse_dependency), validate_func() + Use(parse_dependency), validate_func() ), ConfigOption("initial_context", default={}): Or( dict, validate_func() @@ -437,12 +437,10 @@ def _build_environment(self) -> None: def _set_dependencies(self) -> None: if callable(self.cfg.dependencies): - ret = self.cfg.dependencies() - deps = parse_dependency(ret) if ret is not None else None + deps = parse_dependency(self.cfg.dependencies()) else: deps = self.cfg.dependencies - if deps is not None: - 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 09252dad8..70c12b8c2 100644 --- a/tests/functional/testplan/testing/test_environment.py +++ b/tests/functional/testplan/testing/test_environment.py @@ -56,27 +56,22 @@ def __missing__(self, key): def _assert_orig_dep(env): - if env["is_set"] is True: - assert env.__dict__["_orig_dependency"] is not None - else: - assert env.__dict__["_orig_dependency"] is None + assert env.__dict__["_orig_dependency"] is not None @skip_on_windows(reason="Bash files skipped on Windows.") @pytest.mark.parametrize( - "driver_dependencies, is_lift, is_set", + "driver_dependencies, use_callable", [ - (None, False, False), - (None, True, False), - ([], False, True), - ([], True, True), - ([("a", "b")], False, True), - ([("a", "b"), ("b", "c"), ("a", "c")], True, True), - ([("a", "b"), ("c", "d"), ("a", "d"), ("c", "b")], False, True), + ([], 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, named_temp_file, driver_dependencies, is_lift, is_set + mockplan, named_temp_file, driver_dependencies, use_callable ): drivers = DriverGeneratorDict(named_temp_file) for k in ["a", "b"]: @@ -98,11 +93,12 @@ def test_testing_environment( target=DummyTest( name="MyTest", binary=binary_path, - initial_context={"is_set": is_set}, environment=lambda: list(drivers.values()) - if is_lift + if use_callable else list(drivers.values()), - dependencies=lambda: dependencies if is_lift else dependencies, + dependencies=lambda: dependencies + if use_callable + else dependencies, after_start=_assert_orig_dep, ), resource=None,