diff --git a/CHANGELOG.md b/CHANGELOG.md index 258c2644d..3611c4c8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Viash 0.x.x (yyyy-MM-dd): TODO Add title -TODO add summary +## NEW FEATURES + +* `Nextflow` runner: specifying a non-existent argument as a hashmap key for `fromState` and `toState` now raises an error (PR #793). # Viash 0.9.1 (2024-12-16): Enhanced nextflow support and Scala 3 update diff --git a/src/main/resources/io/viash/runners/nextflow/workflowFactory/processWorkflowArgs.nf b/src/main/resources/io/viash/runners/nextflow/workflowFactory/processWorkflowArgs.nf index 7645d7ca3..ce3bf7af1 100644 --- a/src/main/resources/io/viash/runners/nextflow/workflowFactory/processWorkflowArgs.nf +++ b/src/main/resources/io/viash/runners/nextflow/workflowFactory/processWorkflowArgs.nf @@ -117,12 +117,16 @@ def _processFromState(fromState, key_, config_) { assert fromState.values().every{it instanceof CharSequence} : "Error in module '$key_': fromState is a Map, but not all values are Strings" assert fromState.keySet().every{it instanceof CharSequence} : "Error in module '$key_': fromState is a Map, but not all keys are Strings" def fromStateMap = fromState.clone() - def requiredInputNames = meta.config.allArguments.findAll{it.required && it.direction == "Input"}.collect{it.plainName} + def allArgumentNames = config_.allArguments.collect{it.plainName} + def requiredInputNames = config_.allArguments.findAll{it.required && it.direction == "Input"}.collect{it.plainName} // turn the map into a closure to be used later on fromState = { it -> def state = it[1] assert state instanceof Map : "Error in module '$key_': the state is not a Map" def data = fromStateMap.collectMany{newkey, origkey -> + if (!allArgumentNames.contains(newkey)) { + throw new Exception("Error processing fromState for '$key_': invalid argument '$newkey'") + } // check whether newkey corresponds to a required argument if (state.containsKey(origkey)) { [[newkey, state[origkey]]] @@ -161,6 +165,7 @@ def _processToState(toState, key_, config_) { assert toState.values().every{it instanceof CharSequence} : "Error in module '$key_': toState is a Map, but not all values are Strings" assert toState.keySet().every{it instanceof CharSequence} : "Error in module '$key_': toState is a Map, but not all keys are Strings" def toStateMap = toState.clone() + def allArgumentNames = config_.allArguments.collect{it.plainName} def requiredOutputNames = config_.allArguments.findAll{it.required && it.direction == "Output"}.collect{it.plainName} // turn the map into a closure to be used later on toState = { it -> @@ -169,6 +174,9 @@ def _processToState(toState, key_, config_) { assert output instanceof Map : "Error in module '$key_': the output is not a Map" assert state instanceof Map : "Error in module '$key_': the state is not a Map" def extraEntries = toStateMap.collectMany{newkey, origkey -> + if (!allArgumentNames.contains(origkey)) { + throw new Exception("Error processing toState for '$key_': invalid argument '$origkey'") + } // check whether newkey corresponds to a required argument if (output.containsKey(origkey)) { [[newkey, output[origkey]]] diff --git a/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/config.vsh.yaml b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/config.vsh.yaml new file mode 100644 index 000000000..8d1f411a7 --- /dev/null +++ b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/config.vsh.yaml @@ -0,0 +1,12 @@ +name: invalid_fromstate_argument +namespace: test_wfs +resources: + - type: nextflow_script + path: main.nf + entrypoint: base + # TODO: make absolute when the ns build uses the right CWD + - path: ../../../resources +dependencies: + - name: sub_workflow +platforms: + - type: nextflow \ No newline at end of file diff --git a/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/main.nf b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/main.nf new file mode 100644 index 000000000..ed9aac891 --- /dev/null +++ b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_fromstate_argument/main.nf @@ -0,0 +1,33 @@ +workflow base { + take: input_ch + main: + + // generate list from 0 to 1000 + ch = Channel.fromList(0..1000) + | map { num -> + // create temporary file + def file = tempFile() + file.write("num: $num") + + ["num$num", [ file: file ], ["num": num]] + } + | sub_workflow.run( + fromState: [ + "file": "file", + "thisargumentdoesnotexist": "file", // this should raise + ], + toState: {id, output, state -> + def newState = [ + "step1_output": output.output, + "num": state.num, + "file": state.file, + "thisargumentdoesnotexist": "foo" + ] + return newState + } + ) + + + emit: + input_ch +} diff --git a/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/config.vsh.yaml b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/config.vsh.yaml new file mode 100644 index 000000000..788b6a3e8 --- /dev/null +++ b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/config.vsh.yaml @@ -0,0 +1,12 @@ +name: invalid_tostate_argument +namespace: test_wfs +resources: + - type: nextflow_script + path: main.nf + entrypoint: base + # TODO: make absolute when the ns build uses the right CWD + - path: ../../../resources +dependencies: + - name: sub_workflow +platforms: + - type: nextflow \ No newline at end of file diff --git a/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/main.nf b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/main.nf new file mode 100644 index 000000000..b2bf8cdfd --- /dev/null +++ b/src/test/resources/testnextflowvdsl3/src/test_wfs/invalid_tostate_argument/main.nf @@ -0,0 +1,28 @@ +workflow base { + take: input_ch + main: + + // generate list from 0 to 1000 + ch = Channel.fromList(0..1000) + | map { num -> + // create temporary file + def file = tempFile() + file.write("num: $num") + + ["num$num", [ file: file ], ["num": num]] + } + | sub_workflow.run( + fromState: [ + "file": "file", + ], + toState: [ + "step1_output": "output", + "file": "file", + "newkey": "thisargumentdoesnotexist" // This should raise + ] + ) + + + emit: + input_ch +} diff --git a/src/test/scala/io/viash/runners/nextflow/NextflowScriptTest.scala b/src/test/scala/io/viash/runners/nextflow/NextflowScriptTest.scala index 2e0a770cb..1d057c8b9 100644 --- a/src/test/scala/io/viash/runners/nextflow/NextflowScriptTest.scala +++ b/src/test/scala/io/viash/runners/nextflow/NextflowScriptTest.scala @@ -201,6 +201,33 @@ class NextflowScriptTest extends AnyFunSuite with BeforeAndAfterAll { } + test("Test invalid argument in fromState map", DockerTest, NextflowTest) { + val (exitCode, stdOut, stdErr) = NextflowTestHelper.run( + mainScript = "target/nextflow/test_wfs/invalid_fromstate_argument/main.nf", + args = List( + "--publish_dir", "output" + ), + cwd = tempFolFile + ) + + assert(exitCode == 1, s"\nexit code was $exitCode\nStd output:\n$stdOut\nStd error:\n$stdErr") + assert(stdOut.contains("Error processing fromState for 'sub_workflow': invalid argument 'thisargumentdoesnotexist'")) + } + + test("Test invalid argument in toState map", DockerTest, NextflowTest) { + val (exitCode, stdOut, stdErr) = NextflowTestHelper.run( + mainScript = "target/nextflow/test_wfs/invalid_tostate_argument/main.nf", + args = List( + "--publish_dir", "output" + ), + cwd = tempFolFile + ) + + assert(exitCode == 1, s"\nexit code was $exitCode\nStd output:\n$stdOut\nStd error:\n$stdErr") + assert(stdOut.contains("Error processing toState for 'sub_workflow': invalid argument 'thisargumentdoesnotexist'")) + } + + test("Run multiple output channels standalone", NextflowTest) { val (exitCode, stdOut, stdErr) = NextflowTestHelper.run( mainScript = "target/nextflow/multiple_emit_channels/main.nf",