Change default behaviour of run_tool

ctapipe/core/tests/test_traits.py

@@ -534,7 +534,7 @@ class SomeComponent(TelescopeComponent):
     assert config["SomeComponent"]["tel_param1"] == [("type", "*", 6.0)]
 
 
-def test_telescope_parameter_from_cli(mock_subarray):
+def test_telescope_parameter_from_cli(tmp_path, mock_subarray):
     """
     Test we can pass single default for telescope components via cli
     see #1559
@@ -569,15 +569,15 @@ def setup(self):
             tool,
             [
                 "--SomeComponent.path",
-                "test.h5",
+                f"{tmp_path}/test.h5",
                 "--SomeComponent.val",
                 "2.0",
                 "--SomeComponent.flag",
                 "False",
             ],
         )
         assert result == 0
-        assert tool.comp.path == [("type", "*", pathlib.Path("test.h5").absolute())]
+        assert tool.comp.path == [("type", "*", tmp_path / "test.h5")]
         assert tool.comp.val == [("type", "*", 2.0)]
         assert tool.comp.flag == [("type", "*", False)]
 

ctapipe/core/tool.py

@@ -7,6 +7,7 @@
 import textwrap
 from abc import abstractmethod
 from inspect import cleandoc
+from tempfile import mkdtemp
 from typing import Union
 
 import yaml
@@ -333,6 +334,9 @@ def run(self, argv=None, raises=False):
         argv: list(str)
             command-line arguments, or None to get them
             from sys.argv automatically
+
+        raises : bool
+            Whether to raise Exceptions (to test them) or not.
         """
 
         # return codes are taken from:
@@ -539,17 +543,28 @@ def commented(text, indent_level=2, width=70):
     return "\n".join(lines)
 
 
-def run_tool(tool: Tool, argv=None, cwd=None, raises=False):
+def run_tool(tool: Tool, argv=None, cwd=None, raises=True):
     """
-    Utility run a certain tool in a python session without exitinig
+    Utility run a certain tool in a python session without exiting.
+
+    Parameters
+    ----------
+    argv : List[str]
+        List of command line arguments for the tool.
+    cwd : str or pathlib.Path
+        Path to a temporary working directory. If none, a new (random)
+        temporary directeory gets created.
+    raises : bool
+        If true, raises Exceptions from running tools, to test them.
+        If false, tools can return a non-zero exit code.
 
     Returns
     -------
     exit_code: int
         The return code of the tool, 0 indicates success, everything else an error
     """
     current_cwd = pathlib.Path().absolute()
-    cwd = pathlib.Path(cwd) if cwd is not None else current_cwd
+    cwd = pathlib.Path(cwd) if cwd is not None else mkdtemp()
     try:
         # switch to cwd for running and back after
         os.chdir(cwd)

ctapipe/tools/tests/test_process.py

@@ -136,6 +136,7 @@ def test_stage_1_dl1(tmp_path, dl1_image_file, dl1_parameters_file):
             "--overwrite",
         ],
         cwd=tmp_path,
+        raises=False,
     )
     assert ret == 1
 

ctapipe/tools/tests/test_tools.py

@@ -99,6 +99,7 @@ def test_display_dl1(tmp_path, dl1_image_file, dl1_parameters_file):
     ret = run_tool(
         DisplayDL1Calib(),
         argv=[f"--input={dl1_parameters_file}", "--max-events=1", "--telescope=11"],
+        raises=False,
     )
     assert ret == 1
     assert run_tool(DisplayDL1Calib(), ["--help-all"]) == 0

docs/changes/2175.api.rst

@@ -0,0 +1,12 @@
+Change default behaviour of `run_rool`:
+
+1. The default value of `raises` is now `True`. That means, when using
+   `run_tool`, the Exceptions raised by a Tool will be re-raised. The raised
+   exceptions can be tested for their type and content.
+   If the Tool must fail and only the non-zero error case is important to test,
+   set `raises=False` (as it was before).
+
+2. If the `cwd` parameter is `None` (as per default), now a temporary directory
+   is used instead of the directory, where `run_tool` is called (typically via
+   pytest). This way, log-files and other output files don't clutter your
+   working space.