fix: fix DSL lightweight validation logic (#323)

* fix: no IndexError exception when detecting parameters with no default values
* fix: detect components with no executable or interpreter during light tests

Resolves https://github.ibm.com/st4sd/st4sd-runtime-core/issues/322
Signed-off-by: Vassilis Vassiladis <[email protected]>

python/experiment/model/frontends/dsl.py

@@ -1322,12 +1322,17 @@ def enter(
             template_location=template_location,
         )
 
-        for p in template.signature.parameters:
+        for i in range(len(template.signature.parameters)):
+            p = template.signature.parameters[i] # VV: type hints ...
+
             if p.name in parameters:
                 continue
             if "default" not in p.model_fields_set:
-                errors.append(ValueError(f"The parameter {p.name} in location {'/'.join(location)} "
-                                         f"does not have a value or default"))
+                errors.append(experiment.model.errors.DSLInvalidFieldError(
+                    scope_entry.dsl_location() + ["signature", "parameters", i],
+                    ValueError(f"The parameter {p.name} in location {'/'.join(location)} does not have a value or default")
+                ))
+
         if errors:
             dsl_error = experiment.model.errors.DSLInvalidError([])
             for e in errors:
@@ -2490,8 +2495,8 @@ def no_param_refs_in_parameter_defaults(
     ) -> typing.List[experiment.model.errors.DSLInvalidFieldError]:
         errors = []
 
-        for idx in range(len(template.signature.parameters)):
-            param = comp.signature.parameters[idx]
+        for i in range(len(template.signature.parameters)):
+            param = template.signature.parameters[i]
 
             if not param.default:
                 continue
@@ -2501,7 +2506,7 @@ def no_param_refs_in_parameter_defaults(
             for r in refs:
                 errors.append(
                     experiment.model.errors.DSLInvalidFieldError(
-                        location=location + ["signature", "parameters", idx, "default"],
+                        location=location + ["signature", "parameters", i, "default"],
                         underlying_error=ValueError(
                             f'Reference to parameter/variable "{r}"'
                         )
@@ -2510,6 +2515,21 @@ def no_param_refs_in_parameter_defaults(
 
         return errors
 
+    def detect_missing_fields(
+            comp: Component,
+            index: int
+    ) -> typing.List[experiment.model.errors.DSLInvalidFieldError]:
+        comp_location = ["components", index]
+        errors = []
+
+        if not comp.command.executable and not comp.command.interpreter:
+            errors.append(experiment.model.errors.DSLInvalidFieldError(
+                comp_location + ["command"],
+                ValueError("The component does not specify command.executable or command.interpreter"))
+            )
+
+        return errors
+
     def detect_unknown_variables(
         comp: Component,
         index: int
@@ -2587,6 +2607,7 @@ class Field:
         # VV: enumerate messes up typehints
         comp = namespace.components[idx]
         errors_acc.extend(detect_unknown_variables(comp, idx))
+        errors_acc.extend(detect_missing_fields(comp, idx))
 
     for idx in range(len(namespace.workflows)):
         # VV: enumerate messes up typehints

tests/test_dsl.py

@@ -1362,4 +1362,105 @@ def test_lightweight_validation_parameter_referencing_variable():
             "loc": ["components", 0, "signature", "parameters", 0, "default"],
             "msg": 'Reference to parameter/variable "something"'
         }
+    ]
+
+
+def test_lightweight_validation_missing_exec():
+    dsl = yaml.safe_load("""
+        components:
+        - signature:
+            name: normalize
+            parameters:
+            - name: input
+            - name: parameter
+        - command:
+            arguments: serious science stuff here
+            executable: echo
+          signature:
+            name: simulation-a
+            parameters:
+            - name: data
+        - command:
+            arguments: '%(input)s'
+            executable: echo
+          signature:
+            description: null
+            name: simulation-b
+            parameters:
+            - name: input
+        entrypoint:
+          entry-instance: simulate-and-report
+          execute:
+          - args: {}
+            target: <entry-instance>
+        workflows:
+        - execute:
+          - args:
+              data: '%(some-data)s'
+            target: <simulation>
+          - args:
+              input: <simulation/second-stage>:ref
+              some-parameter: '%(some-parameter)s'
+            target: <analysis>
+          signature:
+            description: You can type text here to help other people use your workflow. For
+              example you can explain what kind of inputs it expects, where people can go
+              to find more information about this etc.
+            name: simulate-and-report
+            parameters:
+            - name: some-data
+            - name: some-parameter
+          steps:
+            analysis: analysis
+            simulation: simulation
+        - execute:
+          - args:
+              input: '%(input)s'
+              parameter: '%(some-parameter)s'
+            target: <normalize>
+          signature:
+            description: null
+            name: analysis
+            parameters:
+            - name: input
+            - name: some-parameter
+          steps:
+            normalize: normalize
+        - execute:
+          - args:
+              data: '%(data)s'
+            target: <first-stage>
+          - args:
+              input: <first-stage>:ref
+            target: <second-stage>
+          signature:
+            description: null
+            name: simulation
+            parameters:
+            - name: data
+          steps:
+            first-stage: simulation-a
+            second-stage: simulation-b
+            """)
+
+    namespace = experiment.model.frontends.dsl.Namespace(**dsl)
+
+    with pytest.raises(experiment.model.errors.DSLInvalidError) as e:
+        experiment.model.frontends.dsl.lightweight_validate(namespace)
+
+    errors = e.value.errors()
+
+    assert errors == [
+        {
+            "loc": ["components", 0, "command"],
+            "msg": 'The component does not specify command.executable or command.interpreter'
+        },
+        {
+            "loc": ["workflows", 0, "signature", "parameters", 0],
+            "msg": 'The parameter some-data in location entry-instance does not have a value or default'
+        },
+        {
+            "loc": ["workflows", 0, "signature", "parameters", 1],
+            "msg": 'The parameter some-parameter in location entry-instance does not have a value or default'
+        }
     ]