WX-1461 Remove womtool upgrade command (#7382)

broadinstitute · Mar 12, 2024 · 1049c4e · 1049c4e
1 parent 9d71ae2
commit 1049c4e
Show file tree

Hide file tree

Showing 44 changed files with 50 additions and 932 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## 87 Release Notes
 
+### `upgrade` command removed from Womtool
+
+Womtool previously supported a `womtool upgrade` command for upgrading draft-2 WDLs to 1.0. With WDL 1.1 soon to become the latest supported version, this functionality is retiring.
+
 ### Replacement of `gsutil` with `gcloud storage`
 
 In this release, all **localization** functionality on the GCP backend migrates to use the more modern and performant `gcloud storage`. With sufficiently powerful worker VMs, Cromwell can now localize at up to 1200 MB/s [0][1][2].

diff --git a/centaur/src/it/scala/centaur/AbstractCentaurTestCaseSpec.scala b/centaur/src/it/scala/centaur/AbstractCentaurTestCaseSpec.scala
@@ -76,59 +76,6 @@ abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String],
     runOrDont(testCase, tags, isIgnored, retries, runTestAndDeleteZippedImports())
   }
 
-  def executeWdlUpgradeTest(testCase: CentaurTestCase): Unit =
-    executeStandardTest(wdlUpgradeTestWdl(testCase))
-
-  private def wdlUpgradeTestWdl(testCase: CentaurTestCase): CentaurTestCase = {
-    import better.files.File
-    import womtool.WomtoolMain
-
-    // The suffix matters because WomGraphMaker.getBundle() uses it to choose the language factory
-    val rootWorkflowFile = File.newTemporaryFile(suffix = ".wdl").append(testCase.workflow.data.workflowContent.get)
-    val workingDir = File.newTemporaryDirectory()
-    val upgradedImportsDir = File.newTemporaryDirectory()
-    val rootWorkflowFilepath = workingDir / rootWorkflowFile.name
-
-    // Un-upgraded imports go into the working directory
-    testCase.workflow.data.zippedImports match {
-      case Some(importsZip: File) =>
-        importsZip.unzipTo(workingDir)
-      case None => ()
-    }
-
-    // Upgrade the imports and copy to main working dir (precludes transitive imports; no recursion yet)
-    workingDir.list.toList.map { file: File =>
-      val upgradedWdl = WomtoolMain.upgrade(file.pathAsString).stdout.get
-      upgradedImportsDir.createChild(file.name).append(upgradedWdl)
-    }
-
-    // Copy to working directory after we operate on the imports that are in it
-    rootWorkflowFile.copyTo(rootWorkflowFilepath)
-
-    val upgradeResult = WomtoolMain.upgrade(rootWorkflowFilepath.pathAsString)
-
-    upgradeResult.stderr match {
-      case Some(stderr) => println(stderr)
-      case _ => ()
-    }
-
-    val newCase = testCase.copy(
-      workflow = testCase.workflow.copy(
-        testName = testCase.workflow.testName + " (draft-2 to 1.0 upgrade)",
-        data = testCase.workflow.data.copy(
-          workflowContent = Option(upgradeResult.stdout.get), // this '.get' catches an error if upgrade fails
-          zippedImports = Option(upgradedImportsDir.zip())
-        )
-      )
-    )(cromwellTracker) // An empty zip appears to be completely harmless, so no special handling
-
-    rootWorkflowFile.delete(swallowIOExceptions = true)
-    upgradedImportsDir.delete(swallowIOExceptions = true)
-    workingDir.delete(swallowIOExceptions = true)
-
-    newCase
-  }
-
   private def runOrDont(testCase: CentaurTestCase,
                         tags: List[Tag],
                         ignore: Boolean,

diff --git a/centaur/src/it/scala/centaur/CentaurTestSuite.scala b/centaur/src/it/scala/centaur/CentaurTestSuite.scala
@@ -28,8 +28,6 @@ object CentaurTestSuite extends StrictLogging {
   logger.info(s"Cromwell under test configured with backends ${cromwellBackends.mkString(", ")}")
   logger.info(s"Unless overridden by workflow options file, tests use default backend: $defaultBackend")
 
-  def isWdlUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("wdl_upgrade")
-
   def isEngineUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("engine_upgrade")
 
   def isPapiUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("papi_upgrade")

diff --git a/centaur/src/it/scala/centaur/WdlUpgradeTestCaseSpec.scala b/centaur/src/it/scala/centaur/WdlUpgradeTestCaseSpec.scala
diff --git a/centaur/src/main/resources/standardTestCases/composedenginefunctions.test b/centaur/src/main/resources/standardTestCases/composedenginefunctions.test
@@ -1,6 +1,5 @@
 name: composedenginefunctions
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: composedenginefunctions/composedenginefunctions.wdl

diff --git a/centaur/src/main/resources/standardTestCases/conditionals.ifs_in_scatters.test b/centaur/src/main/resources/standardTestCases/conditionals.ifs_in_scatters.test
@@ -1,6 +1,6 @@
 name: ifs_in_scatters
 testFormat: workflowsuccess
-tags: [ conditionals, "wdl_upgrade" ]
+tags: [ conditionals ]
 
 files {
   workflow: conditionals_tests/ifs_in_scatters/ifs_in_scatters.wdl

diff --git a/centaur/src/main/resources/standardTestCases/conditionals.nested_lookups.test b/centaur/src/main/resources/standardTestCases/conditionals.nested_lookups.test
@@ -1,6 +1,6 @@
 name: nested_lookups
 testFormat: workflowsuccess
-tags: [ conditionals, "wdl_upgrade" ]
+tags: [ conditionals ]
 
 files {
   workflow: conditionals_tests/nested_lookups/nested_lookups.wdl

diff --git a/centaur/src/main/resources/standardTestCases/conditionals.scatters_in_ifs.test b/centaur/src/main/resources/standardTestCases/conditionals.scatters_in_ifs.test
@@ -1,6 +1,6 @@
 name: scatters_in_ifs
 testFormat: workflowsuccess
-tags: [ conditionals, "wdl_upgrade" ]
+tags: [ conditionals ]
 
 files {
   workflow: conditionals_tests/scatters_in_ifs/scatters_in_ifs.wdl

diff --git a/centaur/src/main/resources/standardTestCases/conditionals.simple_if.test b/centaur/src/main/resources/standardTestCases/conditionals.simple_if.test
@@ -1,6 +1,6 @@
 name: simple_if
 testFormat: workflowsuccess
-tags: [ conditionals, "wdl_upgrade" ]
+tags: [ conditionals ]
 
 files {
   workflow: conditionals_tests/simple_if/simple_if.wdl

diff --git a/centaur/src/main/resources/standardTestCases/conditionals.simple_if_workflow_outputs.test b/centaur/src/main/resources/standardTestCases/conditionals.simple_if_workflow_outputs.test
@@ -1,6 +1,6 @@
 name: simple_if_workflow_outputs
 testFormat: workflowsuccess
-tags: [ conditionals, "wdl_upgrade" ]
+tags: [ conditionals ]
 
 files {
   workflow: conditionals_tests/simple_if_workflow_outputs/simple_if_workflow_outputs.wdl

diff --git a/centaur/src/main/resources/standardTestCases/declarations.test b/centaur/src/main/resources/standardTestCases/declarations.test
@@ -2,7 +2,6 @@
 
 name: declarations
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: declarations/declarations.wdl

diff --git a/centaur/src/main/resources/standardTestCases/declarations_as_nodes.test b/centaur/src/main/resources/standardTestCases/declarations_as_nodes.test
@@ -1,6 +1,5 @@
 name: declarations_as_nodes
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: declarations_as_nodes/declarations_as_nodes.wdl

diff --git a/centaur/src/main/resources/standardTestCases/hello.test b/centaur/src/main/resources/standardTestCases/hello.test
@@ -1,6 +1,5 @@
 name: hello
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: hello/hello.wdl

diff --git a/centaur/src/main/resources/standardTestCases/object_access.test b/centaur/src/main/resources/standardTestCases/object_access.test
@@ -1,6 +1,5 @@
 name: object_access
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: object_access/object_access.wdl

diff --git a/centaur/src/main/resources/standardTestCases/scatterchain.test b/centaur/src/main/resources/standardTestCases/scatterchain.test
@@ -1,6 +1,5 @@
 name: scatterchain
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: scatter_chain/scatter_chain.wdl

diff --git a/centaur/src/main/resources/standardTestCases/scattergather.test b/centaur/src/main/resources/standardTestCases/scattergather.test
@@ -1,6 +1,5 @@
 name: scattergather
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: scattergather/scattergather.wdl

diff --git a/centaur/src/main/resources/standardTestCases/short_circuit.test b/centaur/src/main/resources/standardTestCases/short_circuit.test
@@ -2,7 +2,7 @@ name: short_circuit
 testFormat: workflowsuccess
 
 backends: [Local]
-tags: [localdockertest, "wdl_upgrade"]
+tags: [localdockertest]
 
 
 files {

diff --git a/centaur/src/main/resources/standardTestCases/square.test b/centaur/src/main/resources/standardTestCases/square.test
@@ -1,6 +1,5 @@
 name: square
 testFormat: workflowsuccess
-tags: [ "wdl_upgrade" ]
 
 files {
   workflow: square/square.wdl

diff --git a/centaur/src/main/resources/standardTestCases/string_interpolation.test b/centaur/src/main/resources/standardTestCases/string_interpolation.test
@@ -1,6 +1,5 @@
 name: string_interpolation
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: string_interpolation/string_interpolation.wdl

diff --git a/centaur/src/main/resources/standardTestCases/sub_workflow_hello_world.test b/centaur/src/main/resources/standardTestCases/sub_workflow_hello_world.test
@@ -1,6 +1,6 @@
 name: sub_workflow_hello_world
 testFormat: workflowsuccess
-tags: [subworkflow, "wdl_upgrade"]
+tags: [subworkflow]
 
 files {
   workflow: sub_workflow_hello_world/sub_workflow_hello_world.wdl

diff --git a/centaur/src/main/resources/standardTestCases/sub_workflow_var_refs.test b/centaur/src/main/resources/standardTestCases/sub_workflow_var_refs.test
@@ -1,6 +1,6 @@
 name: sub_workflow_var_refs
 testFormat: workflowsuccess
-tags: [subworkflow, "wdl_upgrade"]
+tags: [subworkflow]
 
 files {
   workflow: sub_workflow_var_refs/sub_workflow_var_refs.wdl

diff --git a/centaur/src/main/resources/standardTestCases/valid_labels.test b/centaur/src/main/resources/standardTestCases/valid_labels.test
@@ -1,6 +1,6 @@
 name: valid_labels
 testFormat: workflowsuccess
-tags: [ labels, "wdl_upgrade" ]
+tags: [ labels ]
 
 files {
   workflow: hello/hello.wdl

diff --git a/centaur/src/main/resources/standardTestCases/write_lines.test b/centaur/src/main/resources/standardTestCases/write_lines.test
@@ -1,6 +1,5 @@
 name: write_lines
 testFormat: workflowsuccess
-tags: ["wdl_upgrade"]
 
 files {
   workflow: write_lines/write_lines.wdl
@@ -13,4 +12,3 @@ metadata {
   "calls.write_lines.f2a.executionStatus": Done
   "outputs.write_lines.a2f_second.x": "a\nb\nc\nd"
 }
-
diff --git a/docs/developers/Centaur.md b/docs/developers/Centaur.md
@@ -133,7 +133,6 @@ our tests into groups depending on which configuration files they require. Below
 | PAPI Upgrade                  | `PapiUpgradeTestCaseSpec`   | `papiUpgradeTestCases`              |
 | PAPI Upgrade<br>New Workflows | `CentaurTestSuite`          | `papiUpgradeNewWorkflowsTestCases`  |
 | Azure Blob                    | `CentaurTestSuite      `    | `azureBlobTestCases`                |
-| WDL Upgrade                   | `WdlUpgradeTestCaseSpec`    | `standardTestCases`****             |
 | (other)                       | `CentaurTestSuite`          | `standardTestCases`                 |
 
 <small>
@@ -143,9 +142,7 @@ or `papi_v2beta_centaur_application.conf`
 \*\* Cromwell Config overrides
   ([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L213-L221))  
 \*\*\* Test Directory overrides
-  ([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L440-L449))  
-\*\*\*\* Test Directory only tests tagged with `wdl_upgrade`
-  ([47 link](https://github.com/broadinstitute/cromwell/blob/47/centaur/src/main/resources/standardTestCases/write_lines.test#L3))  
+  ([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L440-L449))
 </small>
 
 - Engine Upgrade: Retrieves the [Cromwell Version](https://github.com/broadinstitute/cromwell/blob/47/project/Version.scala#L8) then retrieves the previous jar/docker-image from DockerHub. Centaur starts with the prior version, then restarts with the compiled source code.

diff --git a/src/ci/bin/test.inc.sh b/src/ci/bin/test.inc.sh
@@ -169,7 +169,6 @@ cromwell::private::create_build_variables() {
     backend_type="${CROMWELL_BUILD_TYPE}"
     backend_type="${backend_type#centaurEngineUpgrade}"
     backend_type="${backend_type#centaurPapiUpgrade}"
-    backend_type="${backend_type#centaurWdlUpgrade}"
     backend_type="${backend_type#centaurHoricromtal}"
     backend_type="${backend_type#centaur}"
     backend_type="$(echo "${backend_type}" | sed 's/\([A-Z]\)/_\1/g' | tr '[:upper:]' '[:lower:]' | cut -c 2-)"

diff --git a/src/ci/bin/testCentaurWdlUpgradeLocal.sh b/src/ci/bin/testCentaurWdlUpgradeLocal.sh
diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/CallElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/CallElement.scala
@@ -7,4 +7,9 @@ final case class CallElement(callableReference: String,
                              body: Option[CallBodyElement],
                              override val sourceLocation: Option[SourceFileLocation]
 ) extends LanguageElement
-    with WorkflowGraphElement
+    with WorkflowGraphElement {
+  override def toString: String =
+    s"""Call "$callableReference${alias.map(alias => s" as $alias").getOrElse("")}${afters
+        .map(after => s" after $after")
+        .mkString}""""
+}
diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/DeclarationElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/DeclarationElement.scala
@@ -12,7 +12,9 @@ final case class IntermediateValueDeclarationElement(typeElement: TypeElement,
                                                      name: String,
                                                      expression: ExpressionElement
 ) extends WorkflowGraphElement
-    with TaskSectionElement
+    with TaskSectionElement {
+  override def toString: String = s"""Declaration "$name" ($typeElement)"""
+}
 
 object IntermediateValueDeclarationElement {
   def fromContent(content: DeclarationContent): IntermediateValueDeclarationElement =
@@ -24,7 +26,9 @@ object IntermediateValueDeclarationElement {
   */
 final case class OutputDeclarationElement(typeElement: TypeElement, name: String, expression: ExpressionElement)
     extends LanguageElement
-    with WorkflowGraphElement
+    with WorkflowGraphElement {
+  override def toString: String = s"""Output "$name" ($typeElement)"""
+}
 
 object OutputDeclarationElement {
   def fromContent(content: DeclarationContent): OutputDeclarationElement =
@@ -36,7 +40,9 @@ object OutputDeclarationElement {
   */
 final case class InputDeclarationElement(typeElement: TypeElement, name: String, expression: Option[ExpressionElement])
     extends LanguageElement
-    with WorkflowGraphElement
+    with WorkflowGraphElement {
+  override def toString: String = s"""Input "$name" ($typeElement)"""
+}
 
 object DeclarationElement {
   /* Custom unapply so that elsewhere we can do things like this, and otherwise treat all declarations the same:

diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/IfElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/IfElement.scala
@@ -1,4 +1,6 @@
 package wdl.model.draft3.elements
 
 final case class IfElement(conditionExpression: ExpressionElement, graphElements: Seq[WorkflowGraphElement])
-    extends WorkflowGraphElement
+    extends WorkflowGraphElement {
+  override def toString: String = s"""Condition "$conditionExpression""""
+}
diff --git a/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ScatterElement.scala b/wdl/model/draft3/src/main/scala/wdl/model/draft3/elements/ScatterElement.scala
@@ -23,4 +23,6 @@ final case class ScatterElement(scatterName: String,
   // Shorthand to only include certain members for hashing purposes
   // https://stackoverflow.com/a/31915429/818054
   override def hashCode(): Int = (scatterExpression, scatterVariableName, graphElements).##
+
+  override def toString: String = s"""Scatter "$scatterName" ($scatterExpression)"""
 }