diff --git a/README.md b/README.md index 050f716..06dcb08 100644 --- a/README.md +++ b/README.md @@ -112,8 +112,9 @@ details on how Open Job Description's Jobs are run within Sessions. |`--step-name`|string|yes| The name of the Step to run in a local Session. |`--step-name Step1`| |`--environment`|paths|no| Path to a file containing Environment Template definitions. Can be provided multiple times. |`--environment /path/to/env.template1.json --environment /path/to/env.template2.yaml`| |`--job-param`, `-p`|string, path|no| The values for the job template's parameters. Can be provided as key-value pairs, inline JSON string, or as path(s) to a JSON or YAML document. If provided more than once then the given values are combined in the order that they appear. |`--job-param MyParam=5`, `-p file://parameter_file.json`, `-p '{"MyParam": "5"}'`| -|`--task-params`, `-tp`|string, path|no| A list of key-value pairs representing a Task parameter set for the Step, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. If present, the Session will run one Task per parameter set supplied with `--task-params`. Can be specified multiple times. |`--task-params PingCount=20 PingDelay=30`, `-tp file://parameter_set_file.json`| -|`--maximum-tasks`|integer|no| A maximum number of Tasks to run from this Step. Unless present, the Session will run all Tasks defined in the Step's parameter space, or one Task per `--task-params` argument. |`--maximum-tasks 5`| +|`--task-params`, `-tp`|string|no| Instructs the command to run a single task in a Session with the given value for one of the task parameters. The option must be provided once for each task parameter defined for the Step, with each instance providing the value for a different task parameter. Mutually exclusive with `--tasks` and `--maximum-tasks`. |`-tp MyParam=5 -tp MyOtherParam=Foo`| +|`--tasks`|string, path|no| Instructs the command to run one or more tasks for the Step in a Session. The argument must be either the filename of a JSON or YAML file containing an array of maps from task parameter name to value; or an inlined JSON string of the same. Mutually exclusive with `--task-param/-tp` and `--maximum-tasks`. |`--tasks '[{"MyParam": 5}]'`, `--tasks file://parameter_set_file.json`| +|`--maximum-tasks`|integer|no| A maximum number of Tasks to run from this Step. Unless present, the Session will run all Tasks defined in the Step's parameter space or the Task(s) selected by the `--task-params` or `--tasks` arguments. Mutually exclusive with `--task-param/-tp` and `--tasks`. |`--maximum-tasks 5`| |`--run-dependencies`|flag|no| If present, runs all of a Step's dependencies in the Session prior to the Step itself. |`--run-dependencies`| |`--path-mapping-rules`|string, path|no| The path mapping rules to apply to the template. Should be a JSON-formatted list of Open Job Description path mapping rules, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. |`--path-mapping-rules [{"source_os": "Windows", "source_path": "C:\test", "destination_path": "/mnt/test"}]`, `--path-mapping-rules file://rules_file.json`| |`--output`|string|no| How to display the results of the command. Allowed values are `human-readable` (default), `json`, and `yaml`. |`--output json`, `--output yaml`| @@ -122,9 +123,8 @@ details on how Open Job Description's Jobs are run within Sessions. ```sh $ openjd-cli run /path/to/job.template.json --step Step1 \ --job-param PingServer=amazon.com \ - --task-params PingCount=20 PingDelay=30 \ - --task-params file://some_task_parameter_set.json - --maximum-tasks 5 + --task-params PingCount=20 \ + --task-params PingDelay=30 # ... Task logs accompanied by timestamps ... @@ -134,7 +134,7 @@ Session ended successfully Job: MyJob Step: Step1 Duration: 1.0 seconds -Tasks run: 5 +Tasks run: 1 ``` diff --git a/src/openjd/cli/_common/__init__.py b/src/openjd/cli/_common/__init__.py index ef7be5b..5ac95b6 100644 --- a/src/openjd/cli/_common/__init__.py +++ b/src/openjd/cli/_common/__init__.py @@ -12,7 +12,7 @@ from ._job_from_template import ( job_from_template, get_job_params, - get_task_params, + get_params_from_file, ) from ._validation_utils import ( get_doc_type, @@ -25,7 +25,7 @@ __all__ = [ "get_doc_type", "get_job_params", - "get_task_params", + "get_params_from_file", "read_template", "read_job_template", "read_environment_template", diff --git a/src/openjd/cli/_common/_job_from_template.py b/src/openjd/cli/_common/_job_from_template.py index 1ae9044..971fd11 100644 --- a/src/openjd/cli/_common/_job_from_template.py +++ b/src/openjd/cli/_common/_job_from_template.py @@ -91,7 +91,7 @@ def get_job_params(parameter_args: list[str]) -> dict: parameter_dict.update(parameters) # Case 3: Provided argument is a Key=Value string - elif regex_match := re.match("^(.+)=(.*)$", arg): + elif regex_match := re.match("^([^=]+)=(.*)$", arg): parameter_dict.update({regex_match[1]: regex_match[2]}) else: @@ -102,81 +102,6 @@ def get_job_params(parameter_args: list[str]) -> dict: return parameter_dict -def get_task_params(arguments: list[list[str]]) -> list[dict[str, str]]: - """ - Retrieves Task parameter sets from user-provided command line arguments. - Each argument may be a list of Task parameters that forms - the parameter set, or a file containing the Task parameter set(s) to use. - - For example, the arguments `["Param1=1 Param2=String1", "Param1=2 Param2=String2"]` will produce the following output: - ``` - [ - { - "Param1": "1", - "Param2": "String1" - }, - { - "Param1": "2", - "Param2": "String2" - } - ] - ``` - - Returns: A list of dictionaries, with each dictionary representing a - Task parameter set. All values are represented as strings regardless - of the parameter's defined type (types are resolved later by the - `sessions` module). - - Raises: RuntimeError if filepaths can't be resolved or if arguments - can't be serialized into dictionary objects. - """ - all_parameter_sets: list[dict] = [] - - error_list: list[str] = [] - for arg_list in arguments: - # Case 1: Provided argument is a filepath - if len(arg_list) == 1 and arg_list[0].startswith("file://"): - filename = arg_list[0] - # Raises: RuntimeError - file_parameters = get_params_from_file(filename) - # If the file contains a dictionary, add it as-is - if isinstance(file_parameters, dict): - all_parameter_sets.append(file_parameters) - - # If not, the file is a list; check if the list only contains dictionaries, - # with a proper error message if not - elif not all([isinstance(entry, dict) for entry in file_parameters]): - error_list.append( - f"'{filename.removeprefix('file://')}' contains non-dictionary entries: {[entry for entry in file_parameters if not isinstance(entry, dict)]}" - ) - - # If not, all entries are dictionaries; add them to the parameter sets - else: - all_parameter_sets.extend(file_parameters) - - # Case 2: Provided argument is a list of Key=Value strings - else: - parameter_set: dict = {} - - for kvp in arg_list: - regex_match = re.match("(.+)=(.+)", kvp.strip()) - if not regex_match: - error_list.append(f"'{kvp}' should be in the format 'Key=Value'") - else: - parameter_set.update({regex_match[1]: regex_match[2]}) - - if parameter_set: - all_parameter_sets.append(parameter_set) - - if error_list: - error_msg = "Found the following errors collecting Task parameters:" - for error in error_list: - error_msg += f"\n- {error}" - raise RuntimeError(error_msg) - - return all_parameter_sets - - def job_from_template( template: JobTemplate, parameter_args: list[str] | None, diff --git a/src/openjd/cli/_run/_run_command.py b/src/openjd/cli/_run/_run_command.py index c7a047d..89d20ec 100644 --- a/src/openjd/cli/_run/_run_command.py +++ b/src/openjd/cli/_run/_run_command.py @@ -5,16 +5,23 @@ from pathlib import Path import json from typing import Optional +import re from ._local_session._session_manager import LocalSession, LogEntry from .._common import ( OpenJDCliResult, generate_job, - get_task_params, + get_params_from_file, print_cli_result, read_environment_template, ) -from openjd.model import DecodeValidationError, EnvironmentTemplate, Job, Step +from openjd.model import ( + DecodeValidationError, + EnvironmentTemplate, + Job, + Step, + StepParameterSpaceIterator, +) from openjd.sessions import PathMappingRule @@ -52,23 +59,42 @@ def add_run_arguments(run_parser: ArgumentParser): metavar="STEP_NAME", help="The name of the Step in the Job to run Tasks from.", ) - run_parser.add_argument( - "--task-params", + group = run_parser.add_mutually_exclusive_group() + group.add_argument( + "--task-param", "-tp", action="append", - nargs="*", type=str, - metavar=("PARAM1=VALUE1 PARAM2=VALUE2"), - help="Use these Task parameter sets to run the provided Step. Can be provided as a list of key-value pairs, or as a path to a JSON/YAML document prefixed with 'file://'. \ - Each non-file argument represents a single Task parameter set, as a list of Key=Value strings, to run a Task with. \ - Sessions will run one Task per non-file argument, and any Tasks defined in 'file://'-prefixed JSON or YAML documents.", + dest="task_params", + metavar="PARAM=VALUE", + help=( + "This argument instructs the command to run a single task in a Session with the given value for one of the task parameters " + "defined for the Step. The option must be provided once for each task parameter defined for the Step, with each instance " + "providing the value for a different task parameter. Mutually exclusive with --tasks and --maximum-tasks." + ), ) - run_parser.add_argument( + group.add_argument( + "--tasks", + action="store", + type=str, + dest="tasks", + metavar='file://tasks.json OR file://tasks.yaml OR [{"Param": "Value1", ...}, {"Param": "Value2", ...}]', + help=( + "This argument instructs the command to run one or more tasks for the Step in a Session. The argument must be either " + "the filename of a JSON or YAML file containing an array of maps from task parameter name to value; or an inlined " + "JSON string of the same. Mutually exclusive with --task-param/-tp and --maximum-tasks." + ), + ) + group.add_argument( "--maximum-tasks", action="store", type=int, default=-1, - help="The maximum number of Task parameter sets to run this Step with. If unset, the Session will run all of the Step's defined Tasks, or one Task per Task parameter set provided by '--task-params'.", + help=( + "This argument instructs the command to run at most this many Tasks for the Step in the Session. If neither this " + "argument, --task-param/-tp, nor --tasks are provided then the Session will run all of the selected Step's Tasks " + "in the Session. Mutually exclusive with --task-param/-tp and --tasks." + ), ) run_parser.add_argument( "--run-dependencies", @@ -131,6 +157,139 @@ def _collect_required_steps(step_map: dict[str, Step], step: Step) -> list[Step] return required_steps +def _process_task_params(arguments: list[str]) -> dict[str, str]: + """ + Retrieves a single Task parameter set from the user-provided --task-param option. + + Args: + argument (list[str]): Each item is the definition of a single task parameter's + value for the task that is expected to be of the form "ParamName=Value" (we + do validate that the form has been used in this function). + + Returns: A dictionary representing the task parameter set for a single task. All + values are represented as strings regardless of the parameter's defined type + (types are resolved later by the `sessions` module). + + Raises: + RuntimeError if any arguments do not match the required pattern + """ + parameter_set = dict[str, str]() + + error_list: list[str] = [] + for arg in arguments: + arg = arg.lstrip() + if regex_match := re.match("([^=]+)=(.+)", arg): + param, value = regex_match[1], regex_match[2] + if parameter_set.get(param) is not None: + error_list.append(f"Task parameter '{param}' has been defined more than once.") + else: + parameter_set[param] = value + pass + else: + error_list.append( + f"Task parameter '{arg}' defined incorrectly. Expected '=' format." + ) + + if error_list: + error_msg = "Found the following errors collecting Task parameters:" + for error in error_list: + error_msg += f"\n- {error}" + raise RuntimeError(error_msg) + + return parameter_set + + +def _process_tasks(argument: str) -> list[dict[str, str]]: + """ + Retrieves a list of parameter sets from the user-provided --tasks argument on the command-line. + + Args: + argument (str): The definition of the collection of task parameter sets to run in the Session. + Correct user-input must of one of the following forms (we validate that here): + - file://.[json|yaml] + - The file contains a JSON/YAML document that defines an array of parameter sets. Each + parameter set is defined as a mapping from parameter name to parameter value. + - + - The string contains a JSON document that defines an array of parameter sets. Each + parameter set is defined as a mapping from parameter name to parameter value. + + Returns: + list[dict[str,str]]: Each dictionary representing the task parameter set for a single task. + All values are represented as strings regardless of the parameter's defined type + (types are resolved later by the `sessions` module). + + Raises: + RuntimeError if any arguments do not match the required pattern, or fail to parse + """ + argument = argument.strip() + if argument.startswith("file://"): + # Raises: RuntimeError + parameter_sets = get_params_from_file(argument) + else: + try: + parameter_sets = json.loads(argument) + except (json.JSONDecodeError, TypeError): + raise RuntimeError( + "--task argument must be a JSON encoded list of maps or a string with the file:// prefix." + ) + + # Ensure that the type is what we expected -- a list[dict[str,str]] + if not isinstance(parameter_sets, list): + raise RuntimeError( + "--task argument must be a list of maps from string to string when decoded." + ) + for item in parameter_sets: + if not isinstance(item, dict): + raise RuntimeError( + "--task argument must be a list of maps from string to string when decoded." + ) + for param, value in item.items(): + if not isinstance(value, (str, int, float)): + raise RuntimeError( + "--task argument must be a list of maps from string to string when decoded." + ) + item[param] = str(value) + + return parameter_sets + + +def _validate_task_params(step: Step, task_params: list[dict[str, str]]) -> None: + # For each task parameter set, verify: + # 1) There are no parameters defined that don't exist in the template. + # 2) That all parameters that are defined in the Step are defined in the parameter set. + # 3) [TODO] That the given parameter set is actually in the parameter space of the Step. + # - We need openjd.model.StepParameterSpaceIterator to have a membership test first to be able to do + # this last check. + + # Collect the names of all of the task parameters defined in the step. + if step.parameterSpace is not None: + parameter_space = StepParameterSpaceIterator(space=step.parameterSpace) + task_parameter_names: set[str] = set(parameter_space.names) + else: + task_parameter_names = set[str]() + + error_list = list[str]() + for i, parameter_set in enumerate(task_params): + defined_params = set(parameter_set.keys()) + if defined_params == task_parameter_names: + continue + extra_names = defined_params.difference(task_parameter_names) + missing_names = task_parameter_names.difference(defined_params) + if extra_names: + error_list.append( + f"Task {i} defines unknown parameters: {', '.join(sorted(extra_names))}" + ) + if missing_names: + error_list.append( + f"Task {i} is missing values for parameters: {', '.join(sorted(missing_names))}" + ) + + if error_list: + error_msg = "Errors defining task parameter values:\n - " + error_msg += "\n - ".join(error_list) + raise RuntimeError(error_msg) + + def _run_local_session( *, job: Job, @@ -240,31 +399,36 @@ def do_run(args: Namespace) -> OpenJDCliResult: try: # Raises: RuntimeError - sample_job = generate_job(args) + the_job = generate_job(args) + + # Map Step names to Step objects so they can be easily accessed + step_map = {step.name: step for step in the_job.steps} - task_params: list[dict] = [] + if args.step not in step_map: + raise RuntimeError( + f"No Step with name '{args.step}' is defined in the given Job Template." + ) + + task_params: list[dict[str, str]] = [] if args.task_params: - task_params = get_task_params(args.task_params) + task_params = [_process_task_params(args.task_params)] + elif args.tasks: + task_params = _process_tasks(args.tasks) + + print(step_map[args.step]) + _validate_task_params(step_map[args.step], task_params) except RuntimeError as rte: return OpenJDCliResult(status="error", message=str(rte)) - # Map Step names to Step objects so they can be easily accessed - step_map = {step.name: step for step in sample_job.steps} - - if args.step in step_map: - return _run_local_session( - job=sample_job, - step_map=step_map, - step=step_map[args.step], - task_parameter_values=task_params, - maximum_tasks=args.maximum_tasks, - environments=environments, - path_mapping_rules=path_mapping_rules, - should_run_dependencies=(args.run_dependencies), - should_print_logs=(args.output == "human-readable"), - ) - - return OpenJDCliResult( - status="error", message=f"Step '{args.step}' does not exist in Job '{sample_job.name}'." + return _run_local_session( + job=the_job, + step_map=step_map, + step=step_map[args.step], + task_parameter_values=task_params, + maximum_tasks=args.maximum_tasks, + environments=environments, + path_mapping_rules=path_mapping_rules, + should_run_dependencies=(args.run_dependencies), + should_print_logs=(args.output == "human-readable"), ) diff --git a/test/openjd/cli/test_common.py b/test/openjd/cli/test_common.py index 0922c15..f43a8f9 100644 --- a/test/openjd/cli/test_common.py +++ b/test/openjd/cli/test_common.py @@ -19,7 +19,6 @@ from openjd.cli._common import ( generate_job, get_job_params, - get_task_params, read_template, read_job_template, read_environment_template, @@ -183,6 +182,7 @@ def test_read_environment_template_parsingerror(tempfile_extension: str, file_co "mock_param_args,expected_param_values", [ pytest.param(MOCK_PARAM_ARGUMENTS, MOCK_PARAM_VALUES, id="Params from key-value pair"), + pytest.param(["MyParam=One=Two"], {"MyParam": "One=Two"}, id="Param value with = in it"), pytest.param(["file://TEMPDIR/params.json"], MOCK_PARAM_VALUES, id="Params from file"), pytest.param( [json.dumps({"MyParam": "5"})], {"MyParam": "5"}, id="Params from json string" @@ -424,142 +424,6 @@ def test_job_from_template_error( assert expected_error in str(rte.value) -@pytest.mark.parametrize( - "parameter_set_lists,file_contents,num_expected_parameter_sets", - [ - pytest.param([["Param1=Value"], ["Param1=Value2"]], "", 2, id="Single KVPs"), - pytest.param( - [["Param1=A", "Param2=1"], ["Param1=B", "Param2=2"]], - "", - 2, - id="Lists of KVPs", - ), - pytest.param( - [["file://TEMPDIR/some-file.json"]], - '{"Param1": "A", "Param2": 1}', - 1, - id="File with single dictionary", - ), - pytest.param( - [["file://TEMPDIR/some-file.json"], ["file://TEMPDIR/actually-the-same-contents.json"]], - '{"Param1": "A", "Param2": 1}', - 2, - id="Multiple files with single dictionary", - ), - pytest.param( - [["file://TEMPDIR/some-file.json"]], - '[{"Param1": "A", "Param2": 1}, {"Param1": "B", "Param2": 2}]', - 2, - id="File with list of dictionaries", - ), - pytest.param( - [ - ["Param1=A", "Param2=1"], - ["Param1=B", "Param2=2"], - ["file://TEMPDIR/some-file.json"], - ], - '[{"Param1": "E", "Param2": 5}]', - 3, - id="Combination of formats", - ), - ], -) -def test_get_task_params_success( - parameter_set_lists: list[list[str]], - file_contents: str, - num_expected_parameter_sets: int, -): - """ - Test that Task parameters can be resolved from differently-formatted arguments. - """ - - # For each file argument, we create a file in a temporary directory to reference - with tempfile.TemporaryDirectory() as temp_dir: - # No nice way around it; argument format demands a nested for-loop - for parameter_list in parameter_set_lists: - for i, file_arg in enumerate(parameter_list): - if file_arg.startswith("file://TEMPDIR/"): - param_file = open( - os.path.join(temp_dir, file_arg.removeprefix("file://TEMPDIR/")), "x" - ) - param_file.write(file_contents) - param_file.close() - - parameter_list[i] = file_arg.replace("TEMPDIR", temp_dir) - - task_params = get_task_params(parameter_set_lists) - - assert len(task_params) == num_expected_parameter_sets - - -@pytest.mark.parametrize( - "parameter_set_lists,file_contents,expected_errors", - [ - pytest.param( - [["badformat"]], - "", - ["should be in the format 'Key=Value'"], - id="Badly-formed KVP", - ), - pytest.param( - [["keyvalue", "Key=Value", "valuekey"]], - "", - [ - "'keyvalue' should be in the format 'Key=Value'", - "'valuekey' should be in the format 'Key=Value'", - ], - id="Multiple badly-formed KVPs in same set", - ), - pytest.param( - [ - ["Key=Value", "Key2=Value2"], - ["keyvalue", "valuekey"], - ["Key3=Value3", "anotherbadparam"], - ], - "", - [ - "'keyvalue' should be in the format 'Key=Value'", - "'valuekey' should be in the format 'Key=Value'", - "'anotherbadparam' should be in the format 'Key=Value'", - ], - id="Badly-formed KVPs in multiple sets", - ), - pytest.param( - [["file://TEMPDIR/some-file.json"]], - '[["a list"], 5, "a string"]', - ["contains non-dictionary entries: [['a list'], 5, 'a string']"], - id="File containing non-dictionary parameters", - ), - ], -) -def test_get_task_params_error( - parameter_set_lists: list[list[str]], - file_contents: str, - expected_errors: list[str], -): - """ - Test that incorrectly-formatted Task parameters throw the expected errors. - """ - - # Create all files in a temporary directory so they are automatically cleaned up - with tempfile.TemporaryDirectory() as temp_dir: - for parameter_list in parameter_set_lists: - for i, file_arg in enumerate(parameter_list): - if file_arg.startswith("file://TEMPDIR/"): - param_file = open( - os.path.join(temp_dir, file_arg.removeprefix("file://TEMPDIR/")), "x" - ) - param_file.write(file_contents) - param_file.close() - - parameter_list[i] = file_arg.replace("TEMPDIR", temp_dir) - - with pytest.raises(RuntimeError) as rte: - get_task_params(parameter_set_lists) - - assert all([msg in str(rte.value) for msg in expected_errors]) - - @pytest.mark.parametrize( "template_dict,param_list,expected_param_list", [ diff --git a/test/openjd/cli/test_run_command.py b/test/openjd/cli/test_run_command.py index 12ba1d9..04be9be 100644 --- a/test/openjd/cli/test_run_command.py +++ b/test/openjd/cli/test_run_command.py @@ -6,7 +6,7 @@ import tempfile import re import os -from typing import Any +from typing import Any, Optional import pytest from unittest.mock import Mock, patch @@ -16,9 +16,13 @@ OpenJDRunResult, do_run, _run_local_session, + _process_task_params, + _process_tasks, + _validate_task_params, ) from openjd.cli._run._local_session._session_manager import LocalSession from openjd.sessions import PathMappingRule, PathFormat, Session +from openjd.model import decode_job_template, create_job, ParameterValue, ParameterValueType TEST_RUN_JOB_TEMPLATE_BASIC = { @@ -182,7 +186,7 @@ TEST_RUN_JOB_TEMPLATE_BASIC, [], # Env Templates "First", # step name - [["Foo=1 Bar=Bar1"]], # Task params + ["Foo=1", "Bar=Bar1"], # Task params True, # run_dependencies re.compile( r"J1 Enter.*J2 Enter.*FirstS Enter.*J=Jvalue.*Foo=1. Bar=Bar1.*FirstS Exit.*J2 Exit.*J1 Exit" @@ -242,7 +246,7 @@ def test_do_run_success( job_template: dict[str, Any], env_templates: list[dict[str, Any]], step_name: str, - task_params: list[list[str]], + task_params: list[str], run_dependencies: bool, expected_output: re.Pattern[str], expected_not_in_output: str, @@ -273,6 +277,7 @@ def test_do_run_success( step=step_name, job_params=["J=Jvalue"], task_params=task_params, + tasks=None, maximum_tasks=-1, run_dependencies=run_dependencies, path_mapping_rules=None, @@ -360,6 +365,7 @@ def test_do_run_path_mapping_rules(caplog: pytest.LogCaptureFixture): step="TestStep", job_params=[r"TestPath=/home/test" if os.name == "posix" else r"TestPath=c:\test"], task_params=None, + tasks=None, run_dependencies=False, output="human-readable", path_mapping_rules="file://" + temp_rules.name, @@ -403,6 +409,7 @@ def test_do_run_nonexistent_step(capsys: pytest.CaptureFixture): step="FakeStep", job_params=None, task_params=None, + tasks=None, maximum_tasks=-1, run_dependencies=False, path_mapping_rules=None, @@ -411,7 +418,10 @@ def test_do_run_nonexistent_step(capsys: pytest.CaptureFixture): ) with pytest.raises(SystemExit): do_run(mock_args) - assert "Step 'FakeStep' does not exist" in capsys.readouterr().out + assert ( + "No Step with name 'FakeStep' is defined in the given Job Template." + in capsys.readouterr().out + ) Path(temp_template.name).unlink() @@ -544,3 +554,220 @@ def test_run_local_session_failed( assert response.status == "error" assert expected_error in response.message + + +class TestProcessTaskParams: + """Testing that we properly handle the values of the --task-param/-tp + command-line argument""" + + @pytest.mark.parametrize( + "given, expected", + [ + pytest.param(["Foo=1"], {"Foo": "1"}, id="simple single"), + pytest.param(["Foo=One=Two"], {"Foo": "One=Two"}, id="value containing an = sign"), + pytest.param([" Foo=1 "], {"Foo": "1 "}, id="bracketting whitespace"), + pytest.param(["Foo = 1"], {"Foo ": " 1"}, id="internal whitespace"), + pytest.param( + ["Foo=1", "Bar=Buz"], {"Foo": "1", "Bar": "Buz"}, id="multiple parameters" + ), + ], + ) + def test_success(self, given: list[str], expected: dict[str, str]) -> None: + # WHEN + result = _process_task_params(given) + + # THEN + assert result == expected + + @pytest.mark.parametrize( + "given, expected_error", + [ + pytest.param( + ["Foo1"], "Task parameter 'Foo1' defined incorrectly.", id="regex mismatch" + ), + pytest.param( + ["Foo=1", "Foo=2"], + "Task parameter 'Foo' has been defined more than once.", + id="duplicate definition", + ), + ], + ) + def test_error(self, given: list[str], expected_error: str) -> None: + # WHEN + with pytest.raises(RuntimeError, match=expected_error): + _process_task_params(given) + + +class TestProcessTasks: + """Testing that we properly handle the value of the --tasks command-line argument.""" + + @pytest.mark.parametrize( + "given, file_contents, expected", + [ + pytest.param( + "file://TEMPDIR/some-file.json", + '[{"Param1": "A", "Param2": 1}]', + [{"Param1": "A", "Param2": "1"}], + id="json file; one task", + ), + pytest.param( + "file://TEMPDIR/some-file.yaml", + '- Param1: "A"\n Param2: 1\n', + [{"Param1": "A", "Param2": "1"}], + id="yaml file", + ), + pytest.param( + "file://TEMPDIR/some-file.json", + '[{"Param1": "A", "Param2": 1},{"Param1": "B", "Param2": 2}]', + [{"Param1": "A", "Param2": "1"}, {"Param1": "B", "Param2": "2"}], + id="json file; two tasks", + ), + pytest.param( + '[{"Param1": "A", "Param2": 1}]', + None, + [{"Param1": "A", "Param2": "1"}], + id="inline json; one task", + ), + pytest.param( + '[{"Param1": "A", "Param2": 1},{"Param1": "B", "Param2": 2}]', + None, + [{"Param1": "A", "Param2": "1"}, {"Param1": "B", "Param2": "2"}], + id="inline json; two tasks", + ), + pytest.param('[{"Param": "A"}]', None, [{"Param": "A"}], id="param value str->str"), + pytest.param('[{"Param": 12}]', None, [{"Param": "12"}], id="param value int->str"), + pytest.param( + '[{"Param": 12.2}]', None, [{"Param": "12.2"}], id="param value float->str" + ), + ], + ) + def test_success( + self, given: str, file_contents: Optional[str], expected: dict[str, str] + ) -> None: + # GIVEN + with tempfile.TemporaryDirectory() as temp_dir: + if given.startswith("file://TEMPDIR"): + assert file_contents is not None + filename = os.path.join(temp_dir, given.removeprefix("file://TEMPDIR/")) + with open(filename, "w") as param_file: + param_file.write(file_contents) + given = "file://" + filename + + # WHEN + result = _process_tasks(given) + + # THEN + assert result == expected + + @pytest.mark.parametrize( + "given, file_contents, expected_error", + [ + pytest.param( + "file://TEMPDIR/some-file.json", + "}not json", + "Parameter file.+is formatted incorrectly", + id="not json", + ), + pytest.param( + "file://TEMPDIR/some-file.yaml", + "}not yaml", + "Parameter file.+is formatted incorrectly", + id="not yaml", + ), + pytest.param( + '{"Param": "A"}', + None, + "argument must be a list of maps from string to string when decoded", + id="not a list", + ), + pytest.param( + "[1,2,3]", + None, + "argument must be a list of maps from string to string when decoded", + id="not a list of dicts", + ), + pytest.param( + '[{"Param": [1,2]}]', + None, + "argument must be a list of maps from string to string when decoded", + id="value not scalar", + ), + ], + ) + def test_error(self, given: str, file_contents: Optional[str], expected_error: str) -> None: + # GIVEN + with tempfile.TemporaryDirectory() as temp_dir: + if given.startswith("file://TEMPDIR"): + assert file_contents is not None + filename = os.path.join(temp_dir, given.removeprefix("file://TEMPDIR/")) + with open(filename, "w") as param_file: + param_file.write(file_contents) + given = "file://" + filename + + with pytest.raises(RuntimeError, match=expected_error): + _process_tasks(given) + + +class TestValidateTaskParams: + + @pytest.mark.parametrize( + "given", + [ + pytest.param([{"Foo": "1", "Bar": "Bar1"}], id="one task, all params defined"), + pytest.param( + [{"Foo": "1", "Bar": "Bar1"}, {"Foo": "1", "Bar": "Bar1"}], id="two tasks" + ), + ], + ) + def test_success(self, given: list[dict[str, str]]) -> None: + # GIVEN + job_template = decode_job_template(template=TEST_RUN_JOB_TEMPLATE_BASIC) + job = create_job( + job_template=job_template, + job_parameter_values={ + "J": ParameterValue(type=ParameterValueType.STRING, value="Jvalue") + }, + ) + step = job.steps[0] + + # THEN + # Does not raise + _validate_task_params(step, given) + + @pytest.mark.parametrize( + "given, expected_error", + [ + pytest.param( + [{"Bar": "Bar1"}], "Task 0 is missing values for parameters: Foo", id="missing Foo" + ), + pytest.param( + [{"Bar": "Bar1"}, {"Foo": "1"}], + "Task 0 is missing values for parameters: Foo.*\n.*Task 1 is missing values for parameters: Bar", + id="missing Foo & Bar; separate tasks", + ), + pytest.param( + [{"Foo": "1", "Bar": "Bar1", "Baz": "wut"}], + "Task 0 defines unknown parameters: Baz", + id="extra parameter", + ), + pytest.param( + [{"Bar": "Bar1", "Baz": "wut"}], + "Task 0 defines unknown parameters: Baz.*\n.*Task 0 is missing values for parameters: Foo", + id="missing & extra parameter", + ), + ], + ) + def test_errors(self, given: list[dict[str, str]], expected_error: str) -> None: + # GIVEN + job_template = decode_job_template(template=TEST_RUN_JOB_TEMPLATE_BASIC) + job = create_job( + job_template=job_template, + job_parameter_values={ + "J": ParameterValue(type=ParameterValueType.STRING, value="Jvalue") + }, + ) + step = job.steps[0] + + # THEN + with pytest.raises(RuntimeError, match=expected_error): + _validate_task_params(step, given) diff --git a/test/openjd/test_main.py b/test/openjd/test_main.py index adc7baa..1114883 100644 --- a/test/openjd/test_main.py +++ b/test/openjd/test_main.py @@ -106,14 +106,46 @@ def test_cli_summary_success(mock_summary: Mock, mock_args: list): "-tp", "taskparam=paramvalue", "--run-dependencies", + "--path-mapping-rules", + '[{"source_os": "someOS", "source_path": "some\path", "destination_path": "some/new/path"}]', + "--output", + "json", + ], + id="With all optional arguments (-tp)", + ), + pytest.param( + [ + "some-template.json", + "--step", + "step1", + "-p", + "jobparam=paramvalue", + "--tasks", + '[{"taskparam": "paramvalue"}]', + "--run-dependencies", + "--path-mapping-rules", + '[{"source_os": "someOS", "source_path": "some\path", "destination_path": "some/new/path"}]', + "--output", + "json", + ], + id="With all optional arguments (--tasks)", + ), + pytest.param( + [ + "some-template.json", + "--step", + "step1", + "-p", + "jobparam=paramvalue", "--maximum-tasks", "1", + "--run-dependencies", "--path-mapping-rules", '[{"source_os": "someOS", "source_path": "some\path", "destination_path": "some/new/path"}]', "--output", "json", ], - id="With all optional arguments", + id="With all optional arguments (--maximum-tasks)", ), ], ) @@ -154,6 +186,18 @@ def test_cli_schema_success(mock_schema: Mock): pytest.param(["check"], id="Not enough arguments"), pytest.param(["summary", "template.json", "--job-param"], id="Missing argument value"), pytest.param(["summary", "template.json", "notarealarg"], id="Unexpected argument"), + pytest.param( + ["run", "somefile.json", "-tp", "Foo=Bar", "--tasks", '[{"Foo": "Bar"}]'], + id="-tp/--tasks mutually exclusive", + ), + pytest.param( + ["run", "somefile.json", "-tp", "Foo=Bar", "--maximum-tasks", "1"], + id="-tp/--maximum-tasks mutually exclusive", + ), + pytest.param( + ["run", "somefile.json", "--tasks", '[{"Foo": "Bar"}]', "--maximum-tasks", "1"], + id="--tasks/--maximum-tasks mutually exclusive", + ), ], ) def test_cli_argument_errors(mock_args: list):