Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WM-2296: Callback should supply fully qualified output names #7234

Merged
merged 2 commits into from
Oct 6, 2023
Merged

WM-2296: Callback should supply fully qualified output names #7234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bddc25
test
cjllanwarne Oct 5, 2023
f0e73a0
Update tests to assert FQN
cjllanwarne Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...rc/main/scala/cromwell/engine/workflow/lifecycle/finalization/WorkflowCallbackActor.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
}

private def performCallback(workflowId: WorkflowId, callbackUri: URI, terminalState: WorkflowState, outputs: CallOutputs, failures: List[String]): Future[Done] = {
val callbackPostBody = CallbackMessage(workflowId.toString, terminalState.toString, outputs.outputs.map(entry => (entry._1.name, entry._2)), failures)
val callbackPostBody = CallbackMessage(workflowId.toString, terminalState.toString, outputs.outputs.map(entry => (entry._1.identifier.fullyQualifiedName.value, entry._2)), failures)

Check warning on line 158 in engine/src/main/scala/cromwell/engine/workflow/lifecycle/finalization/WorkflowCallbackActor.scala

View check run for this annotation

Codecov / codecov/patch

engine/src/main/scala/cromwell/engine/workflow/lifecycle/finalization/WorkflowCallbackActor.scala#L158 

Added line #L158 was not covered by tests
for {
entity <- Marshal(callbackPostBody).to[RequestEntity]
headers <- makeHeaders
Expand Down
22 changes: 13 additions & 9 deletions ...est/scala/cromwell/engine/workflow/lifecycle/finalization/WorkflowCallbackActorSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
package cromwell.engine.workflow.lifecycle.finalization

import akka.testkit._
import akka.http.scaladsl.client.RequestBuilding.Post
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.model.{HttpResponse, StatusCodes}
import akka.testkit.TestProbe
import akka.testkit.{TestProbe, _}
import common.mock.MockSugar
import cromwell.core.retry.SimpleExponentialBackoff
import org.mockito.Mockito._
import cromwell.core.{CallOutputs, TestKitSuite, WorkflowFailed, WorkflowId, WorkflowSucceeded}
import cromwell.core._
import cromwell.engine.workflow.lifecycle.finalization.WorkflowCallbackActor.PerformCallbackCommand
import cromwell.engine.workflow.lifecycle.finalization.WorkflowCallbackJsonSupport._
import cromwell.services.metadata.MetadataService.PutMetadataAction
import cromwell.services.metadata.{MetadataEvent, MetadataKey, MetadataValue}
import cromwell.util.{GracefulShutdownHelper, WomMocks}
import cromwell.util.GracefulShutdownHelper
import org.mockito.Mockito._
import org.scalatest.flatspec.AnyFlatSpecLike
import org.scalatest.matchers.should.Matchers
import wom.graph.GraphNodePort.GraphNodeOutputPort
import wom.graph.WomIdentifier
import wom.types.WomStringType
import wom.values.WomString

import java.net.URI
import java.time.Instant
import scala.concurrent.duration._
import scala.concurrent.Future
import scala.concurrent.duration._

class WorkflowCallbackActorSpec
extends TestKitSuite with AnyFlatSpecLike with Matchers with MockSugar {
Expand All @@ -36,7 +38,9 @@ class WorkflowCallbackActorSpec
private val deathWatch = TestProbe("deathWatch")
private val mockUri = new URI("http://example.com")
private val basicConfig = WorkflowCallbackConfig.empty.copy(enabled = true).copy(retryBackoff = SimpleExponentialBackoff(100.millis, 200.millis, 1.1))
private val basicOutputs = WomMocks.mockOutputExpectations(List("foo" -> WomString("bar")).toMap)
private val basicOutputs = CallOutputs(Map(
GraphNodeOutputPort(WomIdentifier("foo", "wf.foo"), WomStringType, null) -> WomString("bar")
))

private val httpSuccess = Future.successful(HttpResponse.apply(StatusCodes.OK))
private val httpFailure = Future.successful(HttpResponse.apply(StatusCodes.GatewayTimeout))
Expand Down Expand Up @@ -64,7 +68,7 @@ class WorkflowCallbackActorSpec
val expectedPostBody = CallbackMessage(
workflowId.toString,
WorkflowSucceeded.toString,
basicOutputs.outputs.map(entry => (entry._1.name, entry._2)),
Map(("wf.foo", WomString("bar"))),
List.empty
)
val expectedRequest = Post(mockUri.toString, expectedPostBody)
Expand Down Expand Up @@ -114,7 +118,7 @@ class WorkflowCallbackActorSpec
val expectedPostBody = CallbackMessage(
workflowId.toString,
WorkflowSucceeded.toString,
basicOutputs.outputs.map(entry => (entry._1.name, entry._2)),
Map(("wf.foo", WomString("bar"))),
List.empty
)
val expectedRequest = Post(mockUri.toString, expectedPostBody)
Expand Down
Loading