[WX-1468] Implement returnCodes runtime attribute (#7389)

Co-authored-by: Ryan Saperstein <[email protected]>
Co-authored-by: Ryan Saperstein <[email protected]>

backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala

@@ -748,10 +748,10 @@ trait StandardAsyncExecutionActor
     RuntimeAttributesValidation.extract(FailOnStderrValidation.instance, validatedRuntimeAttributes)
 
   /**
-    * Returns the behavior for continuing on the return code, obtained by converting `returnCodeContents` to an Int.
-    *
-    * @return the behavior for continuing on the return code.
-    */
+   * Returns the behavior for continuing on the return code, obtained by converting `returnCodeContents` to an Int.
+   *
+   * @return the behavior for continuing on the return code.
+   */
   lazy val continueOnReturnCode: ContinueOnReturnCode =
     RuntimeAttributesValidation.extract(ContinueOnReturnCodeValidation.instance, validatedRuntimeAttributes)
 

backend/src/main/scala/cromwell/backend/validation/ContinueOnReturnCodeValidation.scala

@@ -3,8 +3,8 @@ package cromwell.backend.validation
 import cats.data.Validated.{Invalid, Valid}
 import cats.implicits._
 import com.typesafe.config.Config
-import cromwell.backend.validation.RuntimeAttributesValidation._
 import common.validation.ErrorOr._
+import cromwell.backend.validation.RuntimeAttributesValidation.validateInt
 import wom.RuntimeAttributesKeys
 import wom.types._
 import wom.values._
@@ -23,8 +23,11 @@ import scala.util.Try
   * `default` a validation with the default value specified by the reference.conf file.
   */
 object ContinueOnReturnCodeValidation {
-  lazy val instance: RuntimeAttributesValidation[ContinueOnReturnCode] = new ContinueOnReturnCodeValidation
-  def default(runtimeConfig: Option[Config]): RuntimeAttributesValidation[ContinueOnReturnCode] =
+  lazy val instance: RuntimeAttributesValidation[ContinueOnReturnCode] =
+    new ContinueOnReturnCodeValidation
+  def default(
+    runtimeConfig: Option[Config]
+  ): RuntimeAttributesValidation[ContinueOnReturnCode] =
     instance.withDefault(configDefaultWdlValue(runtimeConfig) getOrElse WomInteger(0))
   def configDefaultWdlValue(runtimeConfig: Option[Config]): Option[WomValue] =
     instance.configDefaultWomValue(runtimeConfig)
@@ -39,6 +42,7 @@ class ContinueOnReturnCodeValidation extends RuntimeAttributesValidation[Continu
   override def validateValue: PartialFunction[WomValue, ErrorOr[ContinueOnReturnCode]] = {
     case WomBoolean(value) => ContinueOnReturnCodeFlag(value).validNel
     case WomString(value) if Try(value.toBoolean).isSuccess => ContinueOnReturnCodeFlag(value.toBoolean).validNel
+    case WomString(value) if value.equals("*") => ContinueOnReturnCodeFlag(true).validNel
     case WomString(value) if Try(value.toInt).isSuccess => ContinueOnReturnCodeSet(Set(value.toInt)).validNel
     case WomInteger(value) => ContinueOnReturnCodeSet(Set(value)).validNel
     case value @ WomArray(_, seq) =>
@@ -47,22 +51,25 @@ class ContinueOnReturnCodeValidation extends RuntimeAttributesValidation[Continu
         case Valid(ints) => ContinueOnReturnCodeSet(ints.toSet).validNel
         case Invalid(_) => invalidValueFailure(value)
       }
+    case value => invalidValueFailure(value)
   }
 
   override def validateExpression: PartialFunction[WomValue, Boolean] = {
     case WomBoolean(_) => true
     case WomString(value) if Try(value.toInt).isSuccess => true
     case WomString(value) if Try(value.toBoolean).isSuccess => true
+    case WomString(value) if value.equals("*") => true
     case WomInteger(_) => true
     case WomArray(WomArrayType(WomStringType), elements) =>
       elements forall { value =>
         Try(value.valueString.toInt).isSuccess
       }
     case WomArray(WomArrayType(WomIntegerType), _) => true
+    case _ => false
   }
 
-  override protected def missingValueMessage: String = s"Expecting $key" +
-    " runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]"
+  override protected def missingValueMessage: String = "Expecting returnCodes/continueOnReturnCode" +
+    " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]."
 
   override def usedInCallCaching: Boolean = true
 }

backend/src/test/scala/cromwell/backend/BackendWorkflowInitializationActorSpec.scala

@@ -62,6 +62,11 @@ class BackendWorkflowInitializationActorSpec
       "read_int(\"bad file\")"
     )
 
+    val starRow = Table(
+      "value",
+      "*"
+    )
+
     val invalidWdlValueRows = Table(
       "womValue",
       WomString(""),
@@ -201,6 +206,21 @@ class BackendWorkflowInitializationActorSpec
       // NOTE: expressions are never valid to validate
     }
 
+    forAll(starRow) { value =>
+      val womValue = WomString(value)
+      val result = true
+      testContinueOnReturnCode(Option(womValue)) should be(result)
+      ContinueOnReturnCodeValidation.default(optionalConfig).validateOptionalWomValue(Option(womValue)) should be(
+        result
+      )
+      val valid =
+        ContinueOnReturnCodeValidation
+          .default(optionalConfig)
+          .validate(Map(RuntimeAttributesKeys.ContinueOnReturnCodeKey -> womValue))
+      valid.isValid should be(result)
+      valid.toEither.toOption.get should be(ContinueOnReturnCodeFlag(true))
+    }
+
     forAll(invalidWdlValueRows) { womValue =>
       val result = false
       testContinueOnReturnCode(Option(womValue)) should be(result)
@@ -213,7 +233,8 @@ class BackendWorkflowInitializationActorSpec
           .validate(Map(RuntimeAttributesKeys.ContinueOnReturnCodeKey -> womValue))
       valid.isValid should be(result)
       valid.toEither.swap.toOption.get.toList should contain theSameElementsAs List(
-        "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]"
+        "Expecting returnCodes/continueOnReturnCode" +
+          " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]."
       )
     }
 

backend/src/test/scala/cromwell/backend/standard/StandardValidatedRuntimeAttributesBuilderSpec.scala

@@ -136,7 +136,8 @@ class StandardValidatedRuntimeAttributesBuilderSpec
       val runtimeAttributes = Map("continueOnReturnCode" -> WomString("value"))
       assertRuntimeAttributesFailedCreation(
         runtimeAttributes,
-        "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]"
+        "Expecting returnCodes/continueOnReturnCode" +
+          " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]."
       )
     }
 
@@ -152,7 +153,6 @@ class StandardValidatedRuntimeAttributesBuilderSpec
                                                 workflowOptions = workflowOptions
       )
     }
-
   }
 
   val defaultLogger: Logger = LoggerFactory.getLogger(classOf[StandardValidatedRuntimeAttributesBuilderSpec])
@@ -188,7 +188,7 @@ class StandardValidatedRuntimeAttributesBuilderSpec
     docker should be(expectedRuntimeAttributes(DockerKey).asInstanceOf[Option[String]])
     failOnStderr should be(expectedRuntimeAttributes(FailOnStderrKey).asInstanceOf[Boolean])
     continueOnReturnCode should be(
-      expectedRuntimeAttributes(ContinueOnReturnCodeKey).asInstanceOf[ContinueOnReturnCode]
+      expectedRuntimeAttributes(ContinueOnReturnCodeKey)
     )
     ()
   }

backend/src/test/scala/cromwell/backend/validation/ContinueOnReturnCodeSpec.scala

@@ -37,5 +37,13 @@ class ContinueOnReturnCodeSpec extends AnyWordSpecLike with CromwellTimeoutSpec
         ContinueOnReturnCodeSet(set).continueFor(returnCode) should be(expectedContinue)
       }
     }
+
+    "continue on expected return code string" in {
+      val flagTests = Table(("string", "returnCode", "expectedContinue"), ("*", 0, true), ("*", 1, true))
+
+      forAll(flagTests) { (flag, returnCode, expectedContinue) =>
+        ContinueOnReturnCodeFlag(flag == "*").continueFor(returnCode) should be(expectedContinue)
+      }
+    }
   }
 }

backend/src/test/scala/cromwell/backend/validation/RuntimeAttributesValidationSpec.scala

@@ -184,7 +184,8 @@ class RuntimeAttributesValidationSpec
         case Valid(_) => fail("A failure was expected.")
         case Invalid(e) =>
           assert(
-            e.head == "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]"
+            e.head == "Expecting returnCodes/continueOnReturnCode" +
+              " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]."
           )
       }
     }
@@ -212,7 +213,8 @@ class RuntimeAttributesValidationSpec
         case Valid(_) => fail("A failure was expected.")
         case Invalid(e) =>
           assert(
-            e.head == "Expecting continueOnReturnCode runtime attribute to be either a Boolean, a String 'true' or 'false', or an Array[Int]"
+            e.head == "Expecting returnCodes/continueOnReturnCode" +
+              " runtime attribute to be either a String '*', 'true', or 'false', a Boolean, or an Array[Int]."
           )
       }
     }
@@ -228,6 +230,18 @@ class RuntimeAttributesValidationSpec
       }
     }
 
+    "return success when tries to validate a valid returnCodes string entry" in {
+      val returnCodesValue = Some(WomString("*"))
+      val result = RuntimeAttributesValidation.validateContinueOnReturnCode(
+        returnCodesValue,
+        "Failed to get return code mandatory key from runtime attributes".invalidNel
+      )
+      result match {
+        case Valid(x) => assert(x == ContinueOnReturnCodeFlag(true))
+        case Invalid(e) => fail(e.toList.mkString(" "))
+      }
+    }
+
     "return success when tries to validate a valid Integer memory entry" in {
       val expectedGb = 1
       val memoryValue = Some(WomInteger(1 << 30))

centaur/src/main/resources/standardTestCases/failing_return_code.test

@@ -0,0 +1,11 @@
+name: failing_return_code
+testFormat: workflowfailure
+
+files {
+  workflow: failing_return_code/failing_return_code.wdl
+}
+
+metadata {
+  workflowName: FailingReturnCode
+  status: Failed
+}

centaur/src/main/resources/standardTestCases/failing_return_code/failing_return_code.wdl

@@ -0,0 +1,20 @@
+version development-1.1
+
+workflow FailingReturnCode {
+    call FailingReturnCodeSet
+}
+
+task FailingReturnCodeSet {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 1
+    >>>
+
+    runtime {
+        returnCodes: [0, 5, 10]
+        docker: "ubuntu:latest"
+    }
+}

centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code.test

@@ -0,0 +1,11 @@
+name: invalid_return_codes_and_continue_on_return_code
+testFormat: workflowfailure
+
+files {
+  workflow: invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl
+}
+
+metadata {
+  workflowName: InvalidReturnCodeAndContinueOnReturnCode
+  status: Failed
+}

centaur/src/main/resources/standardTestCases/invalid_return_codes_and_continue_on_return_code/invalid_return_codes_and_continue_on_return_code.wdl

@@ -0,0 +1,21 @@
+version development-1.1
+
+workflow InvalidReturnCodeAndContinueOnReturnCode {
+    call InvalidReturnCodeContinueOnReturnCode
+}
+
+task InvalidReturnCodeContinueOnReturnCode {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 1
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: [5, 10, 15]
+        continueOnReturnCode: [1]
+    }
+}

centaur/src/main/resources/standardTestCases/return_codes.test

@@ -0,0 +1,11 @@
+name: return_codes
+testFormat: workflowsuccess
+
+files {
+  workflow: return_codes/return_codes.wdl
+}
+
+metadata {
+  workflowName: ReturnCodeValidation
+  status: Succeeded
+}

centaur/src/main/resources/standardTestCases/return_codes/return_codes.wdl

@@ -0,0 +1,69 @@
+version development-1.1
+
+workflow ReturnCodeValidation {
+    call ReturnCodeSet1
+    call ReturnCodeSet2
+    call ReturnCodeSet3
+    call ReturnCodeString
+}
+
+task ReturnCodeSet1 {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 1
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: [1]
+    }
+}
+
+task ReturnCodeSet2 {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 200
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: [1, 123, 200]
+    }
+}
+
+task ReturnCodeSet3 {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 10
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: 10
+    }
+}
+
+
+task ReturnCodeString {
+    meta {
+        volatile: true
+    }
+
+    command <<<
+        exit 500
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: "*"
+    }
+}

centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version.test

@@ -0,0 +1,11 @@
+name: return_codes_invalid_on_old_wdl_version
+testFormat: workflowfailure
+
+files {
+  workflow: return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl
+}
+
+metadata {
+  workflowName: ReturnCodesInvalidOnOldWdl
+  status: Failed
+}

centaur/src/main/resources/standardTestCases/return_codes_invalid_on_old_wdl_version/return_codes_invalid_on_old_wdl_version.wdl

@@ -0,0 +1,18 @@
+workflow ReturnCodesInvalidOnOldWdl {
+        call ReturnCodesInvalidOnOldWdlTask
+}
+
+task ReturnCodesInvalidOnOldWdlTask {
+    meta {
+        volatile: "true"
+    }
+
+    command <<<
+        exit 5
+    >>>
+
+    runtime {
+        docker: "ubuntu:latest"
+        returnCodes: [5, 10, 15]
+    }
+}