From 3ffc5add13778ed5176803d819980688d92bb74b Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Wed, 14 Feb 2024 18:20:36 -0800 Subject: [PATCH] Fix URL Map setUpClass errors not framed with `PSM Failed Test ...` --- framework/test_cases/base_testcase.py | 96 +++++++++++++++++++++------ framework/xds_url_map_testcase.py | 48 ++++++++------ 2 files changed, 101 insertions(+), 43 deletions(-) diff --git a/framework/test_cases/base_testcase.py b/framework/test_cases/base_testcase.py index f2dbcab4..d72a30b8 100644 --- a/framework/test_cases/base_testcase.py +++ b/framework/test_cases/base_testcase.py @@ -12,8 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. """Base test case used for xds test suites.""" - -from typing import Optional +import inspect +import traceback +from typing import Optional, Union import unittest from absl import logging @@ -21,6 +22,7 @@ class BaseTestCase(absltest.TestCase): + # @override def run(self, result: Optional[unittest.TestResult] = None) -> None: super().run(result) # TODO(sergiitk): should this method be returning result? See @@ -44,10 +46,8 @@ def run(self, result: Optional[unittest.TestResult] = None) -> None: self.test_name, f" | Errors count: {total_errors}" if total_errors > 1 else "", ) - if test_errors: - self._print_error_list(test_errors, is_unexpected_error=True) - if test_failures: - self._print_error_list(test_failures) + self._log_test_errors(test_errors, is_unexpected=True) + self._log_test_errors(test_failures) elif test_unexpected_successes: logging.error( "----- PSM Test Case UNEXPECTEDLY SUCCEEDED: %s -----\n", @@ -85,22 +85,74 @@ def test_name(self) -> str: """ return self.id().removeprefix("__main__.").split(" ", 1)[0] - def _print_error_list( - self, errors: list[str], is_unexpected_error: bool = False + def _log_test_errors(self, errors: list[str], is_unexpected: bool = False): + for err in errors: + self._log_framed_test_failure(self.test_name, err, is_unexpected) + + @classmethod + def _log_class_hook_failure(cls, error: Exception): + """ + Log error helper for failed unittest hooks, e.g. setUpClass. + + Normally we don't want to make external calls in setUpClass. + But when we do, we want to wrap them into try/except, and call + _log_class_hook_failure, so the error is logged in our standard format. + Don't forget to re-raise! + + Example: + @classmethod + def setUpClass(cls): + try: + # Making bad external calls that end up raising + raise OSError("Network bad!") + except Exception as error: # noqa pylint: disable=broad-except + cls._log_class_hook_failure(error) + raise + """ + caller: str + try: + caller_info: inspect.FrameInfo = inspect.stack()[1] + caller: str = caller_info.function + except (IndexError, AttributeError): + caller = "undefined_hook" + + fake_test_id = f"{cls.__name__}.{caller}" + # same cleanup as in self. test_name + test_name = fake_test_id.removeprefix("__main__.").split(" ", 1)[0] + logging.error("----- PSM Test Case FAILED: %s -----", caller) + cls._log_framed_test_failure(test_name, error, is_unexpected=True) + + @classmethod + def _log_framed_test_failure( + cls, + test_name: str, + error: Union[str, Exception], + is_unexpected: bool = False, ) -> None: + trace: str + if isinstance(error, Exception): + trace = cls._format_error_with_trace(error) + else: + trace = error + # FAILURE is an errors explicitly signalled using one of the # TestCase.assert*() methods, while ERROR means an unexpected exception. - fail_type: str = "ERROR" if is_unexpected_error else "FAILURE" - for err in errors: - logging.error( - "(%(fail_type)s) PSM Interop Test Failed: %(test_id)s" - "\n^^^^^" - "\n[%(test_id)s] PSM Failed Test Traceback BEGIN" - "\n%(error)s" - "[%(test_id)s] PSM Failed Test Traceback END\n", - { - "test_id": self.test_name, - "fail_type": fail_type, - "error": err, - }, - ) + fail_type: str = "ERROR" if is_unexpected else "FAILURE" + logging.error( + "(%(fail_type)s) PSM Interop Test Failed: %(test_id)s" + "\n^^^^^" + "\n[%(test_id)s] PSM Failed Test Traceback BEGIN" + "\n%(error)s" + "[%(test_id)s] PSM Failed Test Traceback END\n", + { + "test_id": test_name, + "fail_type": fail_type, + "error": trace, + }, + ) + + @classmethod + def _format_error_with_trace(cls, error: Exception) -> str: + return "".join( + traceback.TracebackException.from_exception(error).format() + ) diff --git a/framework/xds_url_map_testcase.py b/framework/xds_url_map_testcase.py index 25ef9ee9..c1abd85e 100644 --- a/framework/xds_url_map_testcase.py +++ b/framework/xds_url_map_testcase.py @@ -380,27 +380,33 @@ def setUpClass(cls): # whether setUpClass failed. cls.addClassCleanup(cls.cleanupAfterTests) - if not cls.started_test_cases: - # Create the GCP resource once before the first test start - GcpResourceManager().setup(cls.test_case_classes) - cls.started_test_cases.add(cls.__name__) - - # Create the test case's own client runner with it's own namespace, - # enables concurrent running with other test cases. - cls.test_client_runner = ( - GcpResourceManager().create_test_client_runner() - ) - # Start the client, and allow the test to override the initial RPC config. - rpc, metadata = cls.client_init_config( - rpc="UnaryCall,EmptyCall", metadata="" - ) - cls.test_client = cls.test_client_runner.run( - server_target=f"xds:///{cls.hostname()}", - rpc=rpc, - metadata=metadata, - qps=QPS.value, - print_response=True, - ) + # Normally we don't want to make external calls in setUpClass. + try: + if not cls.started_test_cases: + # Create the GCP resource once before the first test start + GcpResourceManager().setup(cls.test_case_classes) + cls.started_test_cases.add(cls.__name__) + + # Create the test case's own client runner with it's own namespace, + # enables concurrent running with other test cases. + cls.test_client_runner = ( + GcpResourceManager().create_test_client_runner() + ) + # Start the client, and allow the test to override the initial + # RPC config. + rpc, metadata = cls.client_init_config( + rpc="UnaryCall,EmptyCall", metadata="" + ) + cls.test_client = cls.test_client_runner.run( + server_target=f"xds:///{cls.hostname()}", + rpc=rpc, + metadata=metadata, + qps=QPS.value, + print_response=True, + ) + except Exception as error: # noqa pylint: disable=broad-except + cls._log_class_hook_failure(error) + raise @classmethod def cleanupAfterTests(cls):