-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separating report_target application in interactive mode #1034
Changes from 15 commits
ac1663c
23a19a7
1e6b2e0
4ff5fde
6e1fc9f
42a51f6
592dae3
ebd9d93
4377f2d
e17965f
698462a
b9a7ae7
b6b9298
c2210ae
194229c
b46233a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Internal refactoring to enable faster startup in interactive mode. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
|
@@ -816,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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We checked and this was breaking testcase removal, the reloader would not handle the IndexError that is thrown by lists, we only continued here originally upon KeyError. |
||
continue | ||
multitest.entries[suite_index] = new_suite | ||
|
||
|
@@ -875,32 +810,12 @@ 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) | ||
|
||
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 +852,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) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -440,19 +440,6 @@ def get_test_context(self): | |
) < len(sorted_testcases): | ||
testcases_to_run = sorted_testcases | ||
|
||
if self.cfg.testcase_report_target: | ||
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: | ||
|
@@ -467,7 +454,7 @@ def get_test_context(self): | |
testcase_instance, None | ||
) | ||
if data is not None: | ||
testcase.__xfail__ = { | ||
testcase.__func__.__xfail__ = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a side effect from report_target, it was turning the bound method into a free function, but here we need to access now the func attribute. |
||
"reason": data["reason"], | ||
"strict": data["strict"], | ||
} | ||
|
@@ -901,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( | ||
|
@@ -960,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, pre_testcase, post_testcase | ||
self._run_testcase, | ||
testcase, | ||
testsuite, | ||
pre_testcase, | ||
post_testcase, | ||
) | ||
for testcase in execution_groups[exec_group] | ||
] | ||
|
@@ -1146,6 +1137,7 @@ def _run_case_related( | |
def _run_testcase( | ||
self, | ||
testcase, | ||
testsuite, | ||
pre_testcase: Callable, | ||
post_testcase: Callable, | ||
testcase_report: Optional[TestCaseReport] = None, | ||
|
@@ -1167,6 +1159,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( | ||
|
@@ -1337,7 +1339,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 | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All kinds of unused functions, we can restore from history if needed at some point.