diff --git a/changes/214.bugfix.rst b/changes/214.bugfix.rst new file mode 100644 index 00000000..3f2dc2f2 --- /dev/null +++ b/changes/214.bugfix.rst @@ -0,0 +1 @@ +Fix pre-hooks by wrapping hook results in a tuple. diff --git a/src/stpipe/step.py b/src/stpipe/step.py index fac6849d..e45d569c 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -465,7 +465,7 @@ def run(self, *args): for pre_hook in self._pre_hooks: hook_results = pre_hook.run(*hook_args) if hook_results is not None: - hook_args = hook_results + hook_args = (hook_results,) args = hook_args self._reference_files_used = [] diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 3196f27a..c5db9570 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,4 +1,5 @@ import asdf +import pytest from stpipe.pipeline import Pipeline from stpipe.step import Step @@ -98,13 +99,14 @@ def hook_function(input_data): return input_data -def test_hook_as_step_class(caplog, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_step_class(hook_type, caplog, disable_crds_steppars): """Test an imported Step subclass can be a hook""" model = FakeDataModel() steps = { "cancelnoise": { - "post_hooks": [ + hook_type: [ HookStep, ] } @@ -115,13 +117,14 @@ def test_hook_as_step_class(caplog, disable_crds_steppars): assert "cancelnoise.myhook" in caplog.text -def test_hook_as_step_instance(caplog, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_step_instance(hook_type, caplog, disable_crds_steppars): """Test an imported Step subclass instance with parameters can be a hook""" model = FakeDataModel() steps = { "shovelpixels": { - "post_hooks": [ + hook_type: [ HookStep(param1="foo", param2=3), ] } @@ -131,13 +134,16 @@ def test_hook_as_step_instance(caplog, disable_crds_steppars): assert "Running HookStep with foo and 3" in caplog.text -def test_hook_as_string_of_importable_step_class(caplog, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_string_of_importable_step_class( + hook_type, caplog, disable_crds_steppars +): """Test a string of a fully-qualified path to Step subclass can be a hook""" model = FakeDataModel() steps = { "shovelpixels": { - "post_hooks": [ + hook_type: [ "test_hooks.HookStep", ] } @@ -147,13 +153,14 @@ def test_hook_as_string_of_importable_step_class(caplog, disable_crds_steppars): assert "Running HookStep" in caplog.text -def test_hook_as_string_of_step_instance(caplog, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_string_of_step_instance(hook_type, caplog, disable_crds_steppars): """Test a string of a fully-qualified Step instance w/params""" model = FakeDataModel() steps = { "shovelpixels": { - "post_hooks": [ + hook_type: [ "test_hooks.HookStep(param1='foo', param2=2)", ] } @@ -163,14 +170,17 @@ def test_hook_as_string_of_step_instance(caplog, disable_crds_steppars): assert "Running HookStep with foo and 2" in caplog.text -def test_hook_as_string_of_importable_function(caplog, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_string_of_importable_function( + hook_type, caplog, disable_crds_steppars +): """Test a string of a fully-qualified function path can be a hook""" data_id = 42 model = FakeDataModel(data_id) steps = { "shovelpixels": { - "post_hooks": [ + hook_type: [ "test_hooks.hook_function", ] } @@ -180,14 +190,15 @@ def test_hook_as_string_of_importable_function(caplog, disable_crds_steppars): assert f"Running hook_function on data {data_id}" in caplog.text -def test_hook_as_systemcall(caplog, tmp_cwd, disable_crds_steppars): +@pytest.mark.parametrize("hook_type", ["pre_hooks", "post_hooks"]) +def test_hook_as_systemcall(hook_type, caplog, tmp_cwd, disable_crds_steppars): """Test a string of a terminal command""" model = FakeDataModel() # Run post_hook CLI scripts steps = { "shovelpixels": { - "post_hooks": [ + hook_type: [ "asdftool info {0}", ] } @@ -196,7 +207,4 @@ def test_hook_as_systemcall(caplog, tmp_cwd, disable_crds_steppars): # Logs from fitsinfo assert "SystemCall instance created" in caplog.text - assert ( - "Spawning 'asdftool info stpipe.MyPipeline.shovelpixels.post_hook0" - in caplog.text - ) + assert "Spawning 'asdftool info stpipe.MyPipeline.shovelpixels" in caplog.text