From 03c610700cba30f65533c4c0dced34031c9d887f Mon Sep 17 00:00:00 2001
From: Dan Rose <dan@digilabs.io>
Date: Sat, 14 Mar 2020 18:35:55 -0500
Subject: [PATCH] Better exception handling in tests

1. Don't continue with testing if an extension crashes
2. Use logger.exception to format current traceback in an error
---
 colcon_core/task/python/test/__init__.py | 30 ++++++++++--------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/colcon_core/task/python/test/__init__.py b/colcon_core/task/python/test/__init__.py
index bd189e0e..592ec3cf 100644
--- a/colcon_core/task/python/test/__init__.py
+++ b/colcon_core/task/python/test/__init__.py
@@ -2,7 +2,6 @@
 # Licensed under the Apache License, Version 2.0
 
 import re
-import traceback
 
 from colcon_core.logging import colcon_logger
 from colcon_core.plugin_system import get_first_line_doc
@@ -27,7 +26,6 @@ def add_arguments(self, *, parser):  # noqa: D102
         add_python_testing_step_arguments(parser)
 
     async def test(self, *, additional_hooks=None):  # noqa: D102
-        pkg = self.context.pkg
         args = self.context.args
 
         logger.info(
@@ -36,8 +34,8 @@ async def test(self, *, additional_hooks=None):  # noqa: D102
         try:
             env = await get_command_environment(
                 'setup_py', args.build_base, self.context.dependencies)
-        except RuntimeError as e:
-            logger.error(str(e))
+        except RuntimeError:
+            logger.exception('Failed to get command environment')
             return 1
         setup_py_data = get_setup_data(self.context.pkg, env)
 
@@ -52,15 +50,13 @@ async def test(self, *, additional_hooks=None):  # noqa: D102
                     1, "test() by extension '{key}'".format_map(locals()))
                 try:
                     matched = extension.match(self.context, env, setup_py_data)
-                except Exception as e:  # noqa: F841
+                except Exception:
                     # catch exceptions raised in python testing step extension
-                    exc = traceback.format_exc()
-                    logger.error(
+                    logger.exception(
                         'Exception in Python testing step extension '
-                        "'{extension.STEP_TYPE}': {e}\n{exc}"
+                        "'{extension.STEP_TYPE}'"
                         .format_map(locals()))
-                    # skip failing extension, continue with next one
-                    continue
+                    return 1
                 if matched:
                     break
             else:
@@ -73,12 +69,11 @@ async def test(self, *, additional_hooks=None):  # noqa: D102
             1, "test.step() by extension '{key}'".format_map(locals()))
         try:
             return await extension.step(self.context, env, setup_py_data)
-        except Exception as e:  # noqa: F841
+        except Exception:
             # catch exceptions raised in python testing step extension
-            exc = traceback.format_exc()
-            logger.error(
+            logger.exception(
                 'Exception in Python testing step extension '
-                "'{extension.STEP_TYPE}': {e}\n{exc}".format_map(locals()))
+                "'{extension.STEP_TYPE}'".format_map(locals()))
             return 1
 
 
@@ -176,12 +171,11 @@ def add_python_testing_step_arguments(parser):
         try:
             retval = extension.add_arguments(parser=parser)
             assert retval is None, 'add_arguments() should return None'
-        except Exception as e:  # noqa: F841
+        except Exception:
             # catch exceptions raised in package selection extension
-            exc = traceback.format_exc()
-            logger.error(
+            logger.exception(
                 'Exception in Python testing step extension '
-                "'{extension.STEP_TYPE}': {e}\n{exc}".format_map(locals()))
+                "'{extension.STEP_TYPE}'".format_map(locals()))
             # skip failing extension, continue with next one