diff --git a/src/ert/config/ert_script.py b/src/ert/config/ert_script.py index fac345392da..7824e6daa57 100644 --- a/src/ert/config/ert_script.py +++ b/src/ert/config/ert_script.py @@ -123,31 +123,13 @@ def initializeAndRun( arguments.append(arg_type(arg_value)) # type: ignore else: arguments.append(None) - + fixtures["workflow_args"] = arguments try: func_args = inspect.signature(self.run).parameters - for i, val in enumerate(func_args): - if val in fixtures: - if len(argument_values) == len(func_args): - # If the user has specified enough arguments - # we let them take precedent over fixtures - logger.warning( - f"Argument: {val} in fixtures: {list(fixtures)}, using user specified argument" - ) - else: - arguments.insert(i, fixtures[val]) - # Check if user have specified arguments that the length matches the signature, if they have provided - # *args, we skip the check - num_positional = len( - [p for p in func_args.values() if p.default is p.empty] - ) - if num_positional > len(arguments) and not any( - [p.kind == p.VAR_POSITIONAL for p in func_args.values()] - ): - raise UserWarning( - "Input arguments different from signature, " - f"user arguments: {argument_values}, fixtures: {list(fixtures)}" - ) + # If the user has specified *args, we skip injecting fixtures, and just + # pass the user configured arguments + if not any([p.kind == p.VAR_POSITIONAL for p in func_args.values()]): + arguments = self.insert_fixtures(func_args, fixtures) # Part of deprecation self._ert = fixtures.get("ert_config") self._ensemble = fixtures.get("ensemble") @@ -176,6 +158,21 @@ def initializeAndRun( # Need to have unique modules in case of identical object naming in scripts __module_count = 0 + def insert_fixtures(self, func_args, fixtures) -> List[Any]: + arguments = [] + errors = [] + for val in func_args: + if val in fixtures: + arguments.append(fixtures[val]) + else: + errors.append(val) + if errors: + raise ValueError( + f"Plugin: {self.__class__.__name__} misconfigured, arguments: {errors} " + f"not found in fixtures: {list(fixtures)}" + ) + return arguments + def output_stack_trace(self, error: str = "") -> None: stack_trace = error or "".join(traceback.format_exception(*sys.exc_info())) sys.stderr.write( diff --git a/src/ert/gui/tools/plugins/plugin.py b/src/ert/gui/tools/plugins/plugin.py index af356da1c83..5e725287587 100644 --- a/src/ert/gui/tools/plugins/plugin.py +++ b/src/ert/gui/tools/plugins/plugin.py @@ -40,10 +40,9 @@ def getArguments(self, fixtures: Dict[str, Any]) -> List[Any]: """ script = self.__loadPlugin() fixtures["parent"] = self.__parent_window - arguments = [] - for i, val in enumerate(inspect.signature(script.getArguments).parameters): - if val in fixtures: - arguments.insert(i, fixtures[val]) + func_args = inspect.signature(script.getArguments).parameters + arguments = script.insert_fixtures(func_args, fixtures) + # Part of deprecation script._ert = fixtures.get("ert_config") script._ensemble = fixtures.get("ensemble") diff --git a/tests/unit_tests/job_queue/test_ert_plugin.py b/tests/unit_tests/job_queue/test_ert_plugin.py index ac6a213642c..0bd498bd20f 100644 --- a/tests/unit_tests/job_queue/test_ert_plugin.py +++ b/tests/unit_tests/job_queue/test_ert_plugin.py @@ -1,4 +1,3 @@ -import logging from unittest.mock import MagicMock import pytest @@ -79,65 +78,65 @@ def run(self, ert_script): assert plugin.initializeAndRun([], [], {"ert_script": fixture_mock}) == fixture_mock -def test_plugin_with_fixtures_and_arguments(): +def test_plugin_with_missing_arguments(capsys): class FixturePlugin(ErtPlugin): - def run(self, arg_1, ert_script, arg_2, fixture_2): - return arg_1, ert_script, arg_2, fixture_2 + def run(self, arg_1, ert_script, fixture_2, arg_2="something"): + pass plugin = FixturePlugin() fixture_mock = MagicMock() fixture2_mock = MagicMock() - assert plugin.initializeAndRun( + plugin.initializeAndRun( [], [1, 2], {"ert_script": fixture_mock, "fixture_2": fixture2_mock} - ) == ("1", fixture_mock, "2", fixture2_mock) + ) + captured = capsys.readouterr() + assert plugin.hasFailed() + assert "FixturePlugin misconfigured" in captured.err + assert "['arg_1', 'arg_2'] not found in fixtures" in captured.err -def test_plugin_mismatch_arguments_and_fixtures(): +def test_plugin_with_fixtures_and_enough_arguments(): class FixturePlugin(ErtPlugin): - def run(self, arg_1, fixture, arg_2, arg_3): - return None + def run(self, workflow_args, ert_script): + return workflow_args, ert_script plugin = FixturePlugin() fixture_mock = MagicMock() - assert "Input arguments different from signature" in plugin.initializeAndRun( - [], [1, 2], {"fixture": fixture_mock} + assert plugin.initializeAndRun([], [1, 2, 3], {"ert_script": fixture_mock}) == ( + ["1", "2", "3"], + fixture_mock, ) -def test_plugin_with_fixtures_and_enough_arguments(caplog): +def test_plugin_with_default_arguments(capsys): class FixturePlugin(ErtPlugin): - def run(self, arg_1, ert_script, arg_2): - return arg_1, ert_script, arg_2 + def run(self, ert_script=None): + return ert_script plugin = FixturePlugin() fixture_mock = MagicMock() - with caplog.at_level(logging.WARNING): - assert plugin.initializeAndRun([], [1, 2, 3], {"ert_script": fixture_mock}) == ( - "1", - "2", - "3", - ) - assert "Argument: ert_script in fixtures" in caplog.messages[0] + assert ( + plugin.initializeAndRun([], [1, 2], {"ert_script": fixture_mock}) + == fixture_mock + ) -def test_plugin_with_fixtures_and_default_arguments(): +def test_plugin_with_args(): class FixturePlugin(ErtPlugin): - def run(self, arg_1, ert_script, arg_2, optional="something"): - return arg_1, ert_script, arg_2, optional + def run(self, *args): + return args plugin = FixturePlugin() fixture_mock = MagicMock() assert plugin.initializeAndRun([], [1, 2], {"ert_script": fixture_mock}) == ( "1", - fixture_mock, "2", - "something", ) -def test_plugin_with_args(): +def test_plugin_with_args_and_kwargs(): class FixturePlugin(ErtPlugin): - def run(self, *args): + def run(self, *args, **kwargs): return args plugin = FixturePlugin()