From ac1663c0e744d55ed31f739f4bb3364cc27a5648 Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Tue, 19 Dec 2023 07:59:14 +0000 Subject: [PATCH 1/6] Separating report_target application in interactive mode. --- ...mprove_interactive_startup_performance.rst | 1 + testplan/testing/base.py | 2 ++ testplan/testing/multitest/base.py | 24 ++++++++++++++++--- 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 doc/newsfragments/2788_changed.improve_interactive_startup_performance.rst diff --git a/doc/newsfragments/2788_changed.improve_interactive_startup_performance.rst b/doc/newsfragments/2788_changed.improve_interactive_startup_performance.rst new file mode 100644 index 000000000..eb79fb84a --- /dev/null +++ b/doc/newsfragments/2788_changed.improve_interactive_startup_performance.rst @@ -0,0 +1 @@ +Internal refactoring to enable faster startup in interactive mode. \ No newline at end of file diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 86472b97d..1a1b0437a 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -244,6 +244,8 @@ def stdout_style(self): @property def test_context(self): + # NOTE: there is probably a better way to ask safely if something + # is interactive if ( getattr(self, "parent", None) and hasattr(self.parent, "cfg") diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index d07e20346..34d6a6f9b 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -397,7 +397,11 @@ def get_test_context(self): ) < len(sorted_testcases): testcases_to_run = sorted_testcases - if self.cfg.testcase_report_target: + if self.cfg.testcase_report_target and not ( + getattr(self, "parent", None) + and hasattr(self.parent, "cfg") + and self.parent.cfg.interactive_port is not None + ): testcases_to_run = [ report_target( func=testcase, @@ -517,7 +521,14 @@ def run_testcases_iter( if shallow_report is None: testcases = [ - testcase + report_target( + func=testcase, + ref_func=getattr( + testsuite, + getattr(testcase, "_parametrization_template", ""), + None, + ), + ) for testcase in testcases if test_filter.filter( test=self, suite=testsuite, case=testcase @@ -527,7 +538,14 @@ def run_testcases_iter( if testsuite.name not in test_targets: continue testcases = [ - testcase + report_target( + func=testcase, + ref_func=getattr( + testsuite, + getattr(testcase, "_parametrization_template", ""), + None, + ), + ) for testcase in testcases if testcase.name in test_targets[testsuite.name] ] From 4377f2d06085ff9843b0c5d8cefd2d5583583bfd Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Thu, 18 Jan 2024 12:18:54 +0000 Subject: [PATCH 2/6] Move report_target decoration to the testcase runner. --- testplan/testing/multitest/base.py | 52 +++++++++--------------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index 1ca67f25d..3684764e4 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -440,23 +440,6 @@ def get_test_context(self): ) < len(sorted_testcases): testcases_to_run = sorted_testcases - if self.cfg.testcase_report_target and not ( - getattr(self, "parent", None) - and hasattr(self.parent, "cfg") - and self.parent.cfg.interactive_port is not None - ): - testcases_to_run = [ - report_target( - func=testcase, - ref_func=getattr( - suite, - getattr(testcase, "_parametrization_template", ""), - None, - ), - ) - for testcase in testcases_to_run - ] - if testcases_to_run: if hasattr(self.cfg, "xfail_tests") and self.cfg.xfail_tests: for testcase in testcases_to_run: @@ -575,14 +558,7 @@ def run_testcases_iter( if shallow_report is None: testcases = [ - report_target( - func=testcase, - ref_func=getattr( - testsuite, - getattr(testcase, "_parametrization_template", ""), - None, - ), - ) + testcase for testcase in testcases if test_filter.filter( test=self, suite=testsuite, case=testcase @@ -592,14 +568,7 @@ def run_testcases_iter( if testsuite.name not in test_targets: continue testcases = [ - report_target( - func=testcase, - ref_func=getattr( - testsuite, - getattr(testcase, "_parametrization_template", ""), - None, - ), - ) + testcase for testcase in testcases if testcase.name in test_targets[testsuite.name] ] @@ -919,7 +888,7 @@ def _run_serial_testcases(self, testsuite, testcases): break testcase_report = self._run_testcase( - testcase, pre_testcase, post_testcase + testcase, testsuite, pre_testcase, post_testcase ) param_template = getattr( @@ -978,7 +947,7 @@ def _run_parallel_testcases(self, testsuite, execution_groups): self.logger.debug('Running execution group "%s"', exec_group) results = [ self._thread_pool.submit( - self._run_testcase, testcase, pre_testcase, post_testcase + self._run_testcase, testcase, testsuite, pre_testcase, post_testcase ) for testcase in execution_groups[exec_group] ] @@ -1164,6 +1133,7 @@ def _run_case_related( def _run_testcase( self, testcase, + testsuite, pre_testcase: Callable, post_testcase: Callable, testcase_report: Optional[TestCaseReport] = None, @@ -1185,6 +1155,16 @@ def _run_testcase( testcase_name=testcase.name, testcase_report=testcase_report ) + if self.cfg.testcase_report_target: + testcase = report_target( + func=testcase, + ref_func=getattr( + testsuite, + getattr(testcase, "_parametrization_template", ""), + None, + ), + ) + # specially handle skipped testcases if hasattr(testcase, "__should_skip__"): with compose_contexts( @@ -1355,7 +1335,7 @@ def _run_testcases_iter(self, testsuite, testcases): ] testcase_report = self._run_testcase( - testcase, pre_testcase, post_testcase + testcase, testsuite, pre_testcase, post_testcase ) yield testcase_report, parent_uids From e17965fdc395e08ed3cc750300079decdd1bc6e5 Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Thu, 18 Jan 2024 12:19:18 +0000 Subject: [PATCH 3/6] Remove unused functionalities of interactive mode. --- testplan/runnable/interactive/base.py | 94 ---------------- .../runnable/interactive/test_interactive.py | 101 +++++++----------- 2 files changed, 41 insertions(+), 154 deletions(-) diff --git a/testplan/runnable/interactive/base.py b/testplan/runnable/interactive/base.py index e6c258ded..1456ac623 100644 --- a/testplan/runnable/interactive/base.py +++ b/testplan/runnable/interactive/base.py @@ -726,71 +726,6 @@ def all_tests_operation(self, operation, await_results=True): else: raise ValueError("Unknown operation: {}".format(operation)) - def create_new_environment(self, env_uid, env_type="local_environment"): - """Dynamically create an environment maker object.""" - if env_uid in self._created_environments: - raise RuntimeError( - "Environment {} already exists.".format(env_uid) - ) - - if env_type == "local_environment": - from testplan.environment import LocalEnvironment - - env_class = LocalEnvironment - else: - raise ValueError("Unknown environment type: {}".format(env_type)) - - self._created_environments[env_uid] = env_class(env_uid) - - def add_environment_resource( - self, env_uid, target_class_name, source_file=None, **kwargs - ): - """ - Add a resource to existing environment or to environment maker object. - """ - final_kwargs = {} - compiled = re.compile(r"_ctx_(.+)_ctx_(.+)") - context_params = {} - for key, value in kwargs.items(): - if key.startswith("_ctx_"): - matched = compiled.match(key) - if not matched or key.count("_ctx_") != 2: - raise ValueError("Invalid key: {}".format(key)) - target_key, ctx_key = matched.groups() - if target_key not in context_params: - context_params[target_key] = {} - context_params[target_key][ctx_key] = value - else: - final_kwargs[key] = value - if context_params: - from testplan.common.utils.context import context - - for key in context_params: - final_kwargs[key] = context(**context_params[key]) - - if source_file is None: # Invoke class loader - resource = self._resource_loader.load( - target_class_name, final_kwargs - ) - try: - self.get_environment(env_uid).add(resource) - except: - self._created_environments[env_uid].add_resource(resource) - else: - raise Exception("Add from source file is not yet supported.") - - def reload_environment_resource( - self, env_uid, target_class_name, source_file=None, **kwargs - ): - # Placeholder for function to delele an existing and registering a new - # environment resource with probably altered source code. - # This should access the already added Environment to plan. - pass - - def add_created_environment(self, env_uid): - """Add an environment from the created environment maker instance.""" - self.target.add_environment(self._created_environments[env_uid]) - def reload(self, rebuild_dependencies=False): """Reload test suites.""" if self._reloader is None: @@ -880,27 +815,6 @@ def _initial_report(self): return report - def _run_all_test_operations(self, test_run_generator): - """Run all test operations.""" - return [ - self._run_test_operation(operation, args, kwargs) - for operation, args, kwargs in test_run_generator - ] - - def _run_test_operation(self, test_operation, args, kwargs): - """Run a test operation and update our report tree with the results.""" - result = test_operation(*args, **kwargs) - - if isinstance(result, TestGroupReport): - self.logger.debug("Merge test result: %s", result) - with self.report_mutex: - self.report[result.uid].merge(result) - elif result is not None: - self.logger.debug( - "Discarding result from test operation: %s", result - ) - return result - def _auto_start_environment(self, test_uid): """Start environment if required.""" env_status = self.report[test_uid].env_status @@ -937,14 +851,6 @@ def _set_env_status(self, test_uid, new_status): ) self.report[test_uid].env_status = new_status - def _log_env_errors(self, test_uid, error_messages): - """Log errors during environment start/stop for a given test.""" - test_report = self.report[test_uid] - with self.report_mutex: - for errmsg in error_messages: - test_report.logger.error(errmsg) - test_report.status_override = Status.ERROR - def _clear_env_errors(self, test_uid): """Remove error logs about environment start/stop for a given test.""" test = self.test(test_uid) diff --git a/tests/functional/testplan/runnable/interactive/test_interactive.py b/tests/functional/testplan/runnable/interactive/test_interactive.py index 96abb3e77..57d9dc42c 100644 --- a/tests/functional/testplan/runnable/interactive/test_interactive.py +++ b/tests/functional/testplan/runnable/interactive/test_interactive.py @@ -232,70 +232,51 @@ def test_top_level_environment(): wait_for_interactive_start(plan) assert len(plan.resources.environments.envs) == 1 + env_uid = "env1" - # Create an environment using serializable arguments. - # That is mandatory for HTTP usage. - plan.interactive.create_new_environment("env2") - plan.interactive.add_environment_resource( - "env2", "TCPServer", name="server" + env = plan.interactive.get_environment(env_uid) + assert isinstance(env, entity.Environment) + resources = [res.uid() for res in env] + assert resources == ["server", "client"] + for resource in env: + assert resource.status == resource.STATUS.NONE + plan.interactive.start_environment(env_uid) # START + + # INSPECT THE CONTEXT WHEN STARTED + env_context = plan.interactive.get_environment_context(env_uid) + for resource in [res.uid() for res in env]: + res_context = plan.interactive.environment_resource_context( + env_uid, resource_uid=resource + ) + assert env_context[resource] == res_context + assert isinstance(res_context["host"], str) + assert isinstance(res_context["port"], int) + assert res_context["port"] > 0 + + # CUSTOM RESOURCE OPERATIONS + plan.interactive.environment_resource_operation( + env_uid, "server", "accept_connection" ) - plan.interactive.add_environment_resource( - "env2", - "TCPClient", - name="client", - _ctx_host_ctx_driver="server", - _ctx_host_ctx_value="{{host}}", - _ctx_port_ctx_driver="server", - _ctx_port_ctx_value="{{port}}", + plan.interactive.environment_resource_operation( + env_uid, "client", "send_text", msg="hello" ) - plan.interactive.add_created_environment("env2") - - assert len(plan.resources.environments.envs) == 2 - - for env_uid in ("env1", "env2"): - env = plan.interactive.get_environment(env_uid) - assert isinstance(env, entity.Environment) - resources = [res.uid() for res in env] - assert resources == ["server", "client"] - for resource in env: - assert resource.status == resource.STATUS.NONE - plan.interactive.start_environment(env_uid) # START - - # INSPECT THE CONTEXT WHEN STARTED - env_context = plan.interactive.get_environment_context(env_uid) - for resource in [res.uid() for res in env]: - res_context = plan.interactive.environment_resource_context( - env_uid, resource_uid=resource - ) - assert env_context[resource] == res_context - assert isinstance(res_context["host"], str) - assert isinstance(res_context["port"], int) - assert res_context["port"] > 0 - - # CUSTOM RESOURCE OPERATIONS - plan.interactive.environment_resource_operation( - env_uid, "server", "accept_connection" - ) - plan.interactive.environment_resource_operation( - env_uid, "client", "send_text", msg="hello" - ) - received = plan.interactive.environment_resource_operation( - env_uid, "server", "receive_text" - ) - assert received == "hello" - plan.interactive.environment_resource_operation( - env_uid, "server", "send_text", msg="worlds" - ) - received = plan.interactive.environment_resource_operation( - env_uid, "client", "receive_text" - ) - assert received == "worlds" + received = plan.interactive.environment_resource_operation( + env_uid, "server", "receive_text" + ) + assert received == "hello" + plan.interactive.environment_resource_operation( + env_uid, "server", "send_text", msg="worlds" + ) + received = plan.interactive.environment_resource_operation( + env_uid, "client", "receive_text" + ) + assert received == "worlds" - for resource in env: - assert resource.status == resource.STATUS.STARTED - plan.interactive.stop_environment(env_uid) # STOP - for resource in env: - assert resource.status == resource.STATUS.STOPPED + for resource in env: + assert resource.status == resource.STATUS.STARTED + plan.interactive.stop_environment(env_uid) # STOP + for resource in env: + assert resource.status == resource.STATUS.STOPPED def put_request(url, data): From 698462a24342e88565f30d422ed8232167612e66 Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Thu, 18 Jan 2024 12:21:26 +0000 Subject: [PATCH 4/6] Make it black. --- testplan/testing/multitest/base.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index 3684764e4..6da696b7d 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -947,7 +947,11 @@ def _run_parallel_testcases(self, testsuite, execution_groups): self.logger.debug('Running execution group "%s"', exec_group) results = [ self._thread_pool.submit( - self._run_testcase, testcase, testsuite, pre_testcase, post_testcase + self._run_testcase, + testcase, + testsuite, + pre_testcase, + post_testcase, ) for testcase in execution_groups[exec_group] ] From b9a7ae7e38819ad2903206d2005483f994fbfb74 Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Thu, 18 Jan 2024 14:02:57 +0000 Subject: [PATCH 5/6] Fix report target side effect inside test context generation. --- testplan/testing/multitest/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testplan/testing/multitest/base.py b/testplan/testing/multitest/base.py index 6da696b7d..a92788960 100644 --- a/testplan/testing/multitest/base.py +++ b/testplan/testing/multitest/base.py @@ -454,7 +454,7 @@ def get_test_context(self): testcase_instance, None ) if data is not None: - testcase.__xfail__ = { + testcase.__func__.__xfail__ = { "reason": data["reason"], "strict": data["strict"], } From c2210aecbb9daab30e04a4aef2a8e10e803e9176 Mon Sep 17 00:00:00 2001 From: Marton Szikszai Date: Tue, 23 Jan 2024 08:40:54 +0000 Subject: [PATCH 6/6] Fixed an issue with testcase removal. Unified handling of test context between interactive and batch execution modes. --- testplan/runnable/interactive/base.py | 5 +-- testplan/testing/base.py | 44 ++++++++------------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/testplan/runnable/interactive/base.py b/testplan/runnable/interactive/base.py index 1456ac623..bf5d9ef13 100644 --- a/testplan/runnable/interactive/base.py +++ b/testplan/runnable/interactive/base.py @@ -751,13 +751,13 @@ def reload_report(self): ].entries[param_index] = case[ param_case.uid ] - except KeyError: + except (KeyError, IndexError): continue else: new_report[multitest.uid][suite.uid].entries[ case_index ] = suite[case.uid] - except KeyError: + except (KeyError, IndexError): continue multitest.entries[suite_index] = new_suite @@ -810,6 +810,7 @@ def _initial_report(self): for test_uid in self.all_tests(): test = self.test(test_uid) + test.reset_context() test_report = test.dry_run().report report.append(test_report) diff --git a/testplan/testing/base.py b/testplan/testing/base.py index 28c8c89e4..83f332529 100644 --- a/testplan/testing/base.py +++ b/testplan/testing/base.py @@ -116,39 +116,25 @@ class Test(Runnable): customize functionality. :param name: Test instance name, often used as uid of test entity. - :type name: ``str`` :param description: Description of test instance. - :type description: ``str`` :param environment: List of - :py:class:`drivers ` to + :py:class:`drivers ` to be started and made available on tests execution. Can also take a callable that returns the list of drivers. - :type environment: ``list`` or ``callable`` :param dependencies: driver start-up dependencies as a directed graph, e.g {server1: (client1, client2)} indicates server1 shall start before client1 and client2. Can also take a callable that returns a dict. - :type dependencies: ``dict`` or ``callable`` :param initial_context: key: value pairs that will be made available as context for drivers in environment. Can also take a callbale that returns a dict. - :type initial_context: ``dict`` or ``callable`` :param test_filter: Class with test filtering logic. - :type test_filter: :py:class:`~testplan.testing.filtering.BaseFilter` :param test_sorter: Class with tests sorting logic. - :type test_sorter: :py:class:`~testplan.testing.ordering.BaseSorter` :param before_start: Callable to execute before starting the environment. - :type before_start: ``callable`` taking an environment argument. :param after_start: Callable to execute after starting the environment. - :type after_start: ``callable`` taking an environment argument. :param before_stop: Callable to execute before stopping the environment. - :type before_stop: ``callable`` taking environment and a result arguments. :param after_stop: Callable to execute after stopping the environment. - :type after_stop: ``callable`` taking environment and a result arguments. :param stdout_style: Console output style. - :type stdout_style: :py:class:`~testplan.report.testing.styles.Style` :param tags: User defined tag value. - :type tags: ``string``, ``iterable`` of ``string``, or a ``dict`` with - ``string`` keys and ``string`` or ``iterable`` of ``string`` as values. Also inherits all :py:class:`~testplan.common.entity.base.Runnable` options. @@ -194,10 +180,10 @@ def __init__( self._init_test_report() self._env_built = False - def __str__(self): + def __str__(self) -> str: return "{}[{}]".format(self.__class__.__name__, self.name) - def _new_test_report(self): + def _new_test_report(self) -> TestGroupReport: return TestGroupReport( name=self.cfg.name, description=self.cfg.description, @@ -206,10 +192,10 @@ def _new_test_report(self): env_status=ResourceStatus.STOPPED, ) - def _init_test_report(self): + def _init_test_report(self) -> TestGroupReport: self.result.report = self._new_test_report() - def get_tags_index(self): + def get_tags_index(self) -> Union[str, Iterable[str], Dict]: """ Return the tag index that will be used for filtering. By default this is equal to the native tags for this object. @@ -219,7 +205,7 @@ def get_tags_index(self): """ return self.cfg.tags or {} - def get_filter_levels(self): + def get_filter_levels(self) -> List[filtering.FilterLevel]: if not self.filter_levels: raise ValueError( "`filter_levels` is not defined by {}".format(self) @@ -227,16 +213,16 @@ def get_filter_levels(self): return self.filter_levels @property - def name(self): + def name(self) -> str: """Instance name.""" return self.cfg.name @property - def description(self): + def description(self) -> str: return self.cfg.description @property - def report(self): + def report(self) -> TestGroupReport: """Shortcut for the test report.""" return self.result.report @@ -247,19 +233,13 @@ def stdout_style(self): @property def test_context(self): - # NOTE: there is probably a better way to ask safely if something - # is interactive - if ( - getattr(self, "parent", None) - and hasattr(self.parent, "cfg") - and self.parent.cfg.interactive_port is not None - ): - return self.get_test_context() - if self._test_context is None: self._test_context = self.get_test_context() return self._test_context + def reset_context(self) -> None: + self._test_context = None + def get_test_context(self): raise NotImplementedError