Fixed all tests with latest changes

jeeves/conftest.py

@@ -0,0 +1,27 @@
+from jeeves.core.actions.stub import (
+    StubSuccessAction,
+    StubNonSuccessAction,
+    StubParametersAction,
+    StubNoParametersAction,
+    StubUncaughtExceptionAction,
+)
+from jeeves.core.actions import PROVIDED_ACTIONS
+from jeeves.core.registry import ActionRegistry
+
+
+def pytest_runtest_setup(item):
+    for action in [
+        StubSuccessAction,
+        StubNonSuccessAction,
+        StubParametersAction,
+        StubNoParametersAction,
+        StubUncaughtExceptionAction,
+    ]:
+        PROVIDED_ACTIONS[action.id] = action
+
+    ActionRegistry.autodiscover()
+
+
+def pytest_runtest_teardown(item):
+    ActionRegistry.actions = {}
+#

jeeves/core/actions/base.py

@@ -1,3 +1,4 @@
+import copy
 import logging
 from abc import abstractmethod
 
@@ -14,7 +15,7 @@ class Action:
     def __init__(self, parameters=None):
         self.logger = logging.getLogger(self.__class__.__name__)
         self.parameters = self.Parameters(**(parameters or {}))
-        self.parsed_parameters = {}
+        self.parsed_parameters = copy.deepcopy(parameters)
 
     def parse_parameters(self, current_execution=None, **arguments):
         """
@@ -31,4 +32,4 @@ class Action:
         """
         Main method to override that handles the work for the defining action.
         """
-        pass
+        pass

jeeves/core/actions/stub.py

@@ -41,7 +41,7 @@ class StubParametersAction(StubSuccessAction):
     An empty Action that provide two configurable parameters.
     """
 
-    id = "sub/parameters"
+    id = "stub/parameters"
 
     class Parameters(Action.Parameters):
         mandatory: Text

jeeves/core/actions/tests/test_base.py

@@ -5,24 +5,18 @@ from jeeves.core.registry import ActionRegistry
 
 
 def test_action_with_empty_parameters_ok():
-    action = ActionRegistry.get_action_cls(
-        "jeeves.core.actions.stub:StubNoParametersAction"
-    )
+    action = ActionRegistry.get_action_cls("stub/no-parameters")
     action()
     action(parameters=None)
     action(parameters={})
 
 
 def test_action_with_parameters_ok():
-    action = ActionRegistry.get_action_cls(
-        "jeeves.core.actions.stub:StubParametersAction"
-    )
+    action = ActionRegistry.get_action_cls("stub/parameters")
     action(parameters=dict(mandatory="text", non_mandatory="text"))
 
 
 def test_action_with_parameters_ko():
-    action = ActionRegistry.get_action_cls(
-        "jeeves.core.actions.stub:StubParametersAction"
-    )
+    action = ActionRegistry.get_action_cls("stub/parameters")
     with pytest.raises(pydantic.ValidationError):
         action(parameters=dict(thisshould="fail"))

jeeves/core/executor.py

@@ -29,7 +29,7 @@ class Executor:
 
     def execute_step(self, step: ExecutionStep):
         try:
-            action = ActionRegistry.get_action_cls(step.task.type)(parameters=step.task.parameters)
+            action = ActionRegistry.get_action(action_id=step.task.type, parameters=step.task.parameters)
             action.parse_parameters(current_execution=self._execution, **self._arguments)
             step.result = action.execute(workspace=self._execution.workspace, arguments=self._arguments)
         except Exception as error:

jeeves/core/registry.py

@@ -50,7 +50,7 @@ class ActionRegistry(metaclass=Singleton):
 
         try:
             return cls.actions[action_id]
-        except IndexError as error:
+        except KeyError as error:
             raise cls.ActionDoesNotExist(f"Error importing action {action_id}: {error}")
 
     @classmethod

jeeves/core/tests/conftest.py

@@ -3,12 +3,6 @@ import os
 import pytest
 
 from jeeves.core.objects import Workspace
-from jeeves.core.registry import ActionRegistry
-
-
-@pytest.fixture(scope="session", autouse=True)
-def autoregister_actions():
-    ActionRegistry.autodiscover()
 
 
 @pytest.fixture

jeeves/core/tests/tasks/test_shell.py

@@ -5,9 +5,11 @@ from unittest import mock
 from subprocess import CompletedProcess
 
 from jeeves.core.objects import Task
+from jeeves.core.registry import ActionRegistry
+
 
 MOCK_DEFINITION = {
-    "type": f"jeeves.core.actions.shell:ScriptAction",
+    "type": "contrib/script",
     "name": "Say hello world in bash",
     "parameters": {"script": "#!/bin/bash\necho Hello World"},
 }
@@ -20,8 +22,9 @@ def get_completed_process(returncode=0, stdout=b"", **kwargs):
 
 @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process()))
 def test_script_bash_task_ok(workspace_obj):
-    task = Task.parse_obj(MOCK_DEFINITION).action
-    result = task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(MOCK_DEFINITION)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    result = action.execute(workspace=workspace_obj)
     assert result.success
 
 
@@ -29,8 +32,9 @@ def test_script_bash_task_ok(workspace_obj):
     "subprocess.run", mock.MagicMock(return_value=get_completed_process(returncode=1))
 )
 def test_script_bash_task_ko(workspace_obj):
-    task = Task.parse_obj(MOCK_DEFINITION).action
-    result = task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(MOCK_DEFINITION)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    result = action.execute(workspace=workspace_obj)
     assert not result.success
 
 
@@ -40,9 +44,10 @@ def test_script_no_shebang_defaults_to_bash_ok(workspace_obj):
     definition["parameters"]["script"] = definition["parameters"]["script"].strip(
         "#!/bin/bash"
     )
-    task = Task.parse_obj(definition).action
-    assert task._get_script().startswith(task.DEFAULT_SHEBANG)
-    result = task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(definition)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    assert action._get_script().startswith(action.DEFAULT_SHEBANG)
+    result = action.execute(workspace=workspace_obj)
     assert result.success
 
 
@@ -53,9 +58,10 @@ def test_script_with_other_shebang_ok(workspace_obj):
     definition = MOCK_DEFINITION.copy()
     py_script = f"#!{py_interpreter}\nprint('{expected_output}')"
     definition["parameters"]["script"] = py_script
-    task = Task.parse_obj(definition).action
-    assert task._get_script().startswith(f"#!{py_interpreter}")
-    result = task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(definition)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    assert action._get_script().startswith(f"#!{py_interpreter}")
+    result = action.execute(workspace=workspace_obj)
     assert result.success
 
 
@@ -77,8 +83,9 @@ def test_script_stdout_and_stderr_is_sent_to_result_ok(workspace_obj):
     )
     definition = MOCK_DEFINITION.copy()
     definition["parameters"]["script"] = script
-    task = Task.parse_obj(definition).action
-    result = task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(definition)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    result = action.execute(workspace=workspace_obj)
     assert "Hello" in result.output
     assert "World" in result.output
 
@@ -86,25 +93,27 @@ def test_script_stdout_and_stderr_is_sent_to_result_ok(workspace_obj):
 @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process()))
 def test_script_task_cleans_tempfile_ok(workspace_obj):
     """Make sure that the script is removed from the system after execution"""
-    task = Task.parse_obj(MOCK_DEFINITION).action
+    task = Task.parse_obj(MOCK_DEFINITION)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
     temp = tempfile.NamedTemporaryFile(mode="w", delete=False)
     with mock.patch(
         "tempfile.NamedTemporaryFile", mock.MagicMock(return_value=temp)
     ) as mocked:
-        task.execute(workspace=workspace_obj)
+        action.execute(workspace=workspace_obj)
         assert not os.path.isfile(mocked.return_value.name)
 
 
 @mock.patch("subprocess.run", mock.MagicMock(return_value=get_completed_process()))
 def test_script_task_sets_permissions_for_owner_only_ok(workspace_obj):
     """Make sure that the script have only read and execution permissions for owner"""
-    task = Task.parse_obj(MOCK_DEFINITION).action
+    task = Task.parse_obj(MOCK_DEFINITION)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
     temp = tempfile.NamedTemporaryFile(mode="w", delete=False)
     with mock.patch(
         "tempfile.NamedTemporaryFile", mock.MagicMock(return_value=temp)
     ) as mocked:
         with mock.patch("os.unlink"):
-            task.execute(workspace=workspace_obj)
+            action.execute(workspace=workspace_obj)
         stat = os.stat(mocked.return_value.name)
         assert oct(stat.st_mode).endswith("500")
         os.unlink(mocked.return_value.name)
@@ -112,8 +121,9 @@ def test_script_task_sets_permissions_for_owner_only_ok(workspace_obj):
 
 @mock.patch("subprocess.run")
 def test_script_task_appends_workspace_env_variable_ok(run_mock, workspace_obj):
-    """Make sure that the WORKSPACE_PATH environment variable is sent correctly """
+    """Make sure that the WORKSPACE_PATH environment variable is sent correctly"""
     run_mock.return_value = get_completed_process()
-    task = Task.parse_obj(MOCK_DEFINITION).action
-    task.execute(workspace=workspace_obj)
+    task = Task.parse_obj(MOCK_DEFINITION)
+    action = ActionRegistry.get_action(action_id=task.type, parameters=task.parameters)
+    action.execute(workspace=workspace_obj)
     assert run_mock.call_args[1]["env"] == {"WORKSPACE_PATH": workspace_obj.path}

jeeves/core/tests/test_executor.py

@@ -6,7 +6,7 @@ from jeeves.core.tests.factories import FlowFactory, TaskFactory
 
 
 def test_executor_success_task_ok():
-    task = TaskFactory(type="jeeves.core.actions.stub:StubSuccessAction")
+    task = TaskFactory(type="stub/success")
     flow = FlowFactory(tasks=[task])
     runner = Executor(flow)
     runner.start()
@@ -16,7 +16,7 @@ def test_executor_success_task_ok():
 
 
 def test_executor_non_success_task_ok():
-    task = TaskFactory(type="jeeves.core.actions.stub:StubNonSuccessAction")
+    task = TaskFactory(type="stub/non-success")
     flow = FlowFactory(tasks=[task])
     runner = Executor(flow)
     runner.start()
@@ -26,7 +26,7 @@ def test_executor_non_success_task_ok():
 
 
 def test_executor_uncaught_exception_in_task_ok():
-    task = TaskFactory(type="jeeves.core.actions.stub:StubUncaughtExceptionAction")
+    task = TaskFactory(type="stub/uncaught-exception")
     flow = FlowFactory(tasks=[task])
     runner = Executor(flow)
     runner.start()
@@ -37,16 +37,16 @@ def test_executor_uncaught_exception_in_task_ok():
 
 @mock.patch("jeeves.core.actions.stub.StubSuccessAction.execute")
 def test_executor_run_action_with_workpsace_ok(execute_mock):
-    task = TaskFactory(type="jeeves.core.actions.stub:StubSuccessAction")
+    task = TaskFactory(type="stub/success")
     flow = FlowFactory(tasks=[task])
     runner = Executor(flow)
     runner.start()
     assert execute_mock.called
-    execute_mock.assert_called_with(workspace=runner._execution.workspace)
+    execute_mock.assert_called_with(workspace=runner._execution.workspace, arguments={})
 
 
 def test_executor_cleans_workspace_after_ok():
-    task = TaskFactory(type="jeeves.core.actions.stub:StubSuccessAction")
+    task = TaskFactory(type="stub/success")
     flow = FlowFactory(tasks=[task])
     runner = Executor(flow)
     path = runner._execution.workspace.path

jeeves/core/tests/test_registry.py

@@ -6,7 +6,7 @@ from jeeves.core.actions.stub import StubSuccessAction
 
 
 def test_registry_get_action_cls_ok():
-    action = ActionRegistry.get_action_cls("jeeves.core.actions.stub:StubSuccessAction")
+    action = ActionRegistry.get_action_cls("stub/success")
     assert issubclass(action, Action) and not isinstance(action, Action)
 
 
@@ -16,12 +16,11 @@ def test_registry_get_action_cls_ko():
 
 
 def test_registry_get_action_ok():
-    action = ActionRegistry.get_action("jeeves.core.actions.stub:StubSuccessAction")
+    action = ActionRegistry.get_action("stub/success")
     assert issubclass(action.__class__, Action) and isinstance(action, Action)
 
 
 def test_registry_namespace_conflict_ok():
-
-    ActionRegistry.register_action(StubSuccessAction)
+    assert StubSuccessAction.id in ActionRegistry.actions
     with pytest.raises(ActionRegistry.ActionIDConflict):
-        ActionRegistry.register_action(StubSuccessAction)
+        ActionRegistry.register_action(StubSuccessAction.id, StubSuccessAction)