Copilot commented on code in PR #6085:
URL: https://github.com/apache/texera/pull/6085#discussion_r3522565975
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/udf/java/JavaUDFOpDescSpec.scala:
##########
@@ -106,4 +106,74 @@ class JavaUDFOpDescSpec extends AnyFlatSpec with Matchers {
j.workers shouldBe 4
j.retainInputColumns shouldBe true
}
+
+ "JavaUDFOpDesc.getPhysicalOp" should
+ "build a parallelizable one-to-many op when workers > 1" in {
+ val d = new JavaUDFOpDesc
+ d.code = "return t;"
+ d.workers = 2
+ val physical = d.getPhysicalOp(workflowId, executionId)
+ physical.parallelizable shouldBe true
+ physical.isOneToManyOp shouldBe true
+ physical.suggestedWorkerNum shouldBe Some(2)
+ physical.opExecInitInfo match {
+ case OpExecWithCode(code, language) =>
+ code shouldBe "return t;"
+ language shouldBe "java"
+ case other => fail(s"expected OpExecWithCode, got $other")
+ }
+ }
+
+ "JavaUDFOpDesc.operatorInfo" should
+ "derive input ports from a configured inputPorts list" in {
+ val d = new JavaUDFOpDesc
+ d.inputPorts = List(
+ PortDescription(
+ portID = "0",
+ displayName = "left",
+ disallowMultiInputs = true,
+ isDynamicPort = false,
+ partitionRequirement = null,
+ dependencies = List.empty
+ )
+ )
+ val info = d.operatorInfo
+ info.inputPorts should have length 1
+ info.inputPorts.head.displayName shouldBe "left"
+ // also drives the getPhysicalOp inputPorts != null partitionRequirement
branch
+ val physical = d.getPhysicalOp(workflowId, executionId)
+ physical.inputPorts.keySet shouldBe info.inputPorts.map(_.id).toSet
+ }
+
+ it should "derive output ports from a configured outputPorts list" in {
+ val d = new JavaUDFOpDesc
+ d.outputPorts = List(
+ PortDescription(
+ portID = "0",
+ displayName = "result",
+ disallowMultiInputs = false,
+ isDynamicPort = false,
+ partitionRequirement = null,
+ dependencies = List.empty
+ )
Review Comment:
Avoid `partitionRequirement = null` in this PortDescription; provide a
concrete PartitionInfo (e.g., UnknownPartition) so the test data matches real
operator configuration and won’t hide null-handling issues.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/udf/java/JavaUDFOpDescSpec.scala:
##########
@@ -106,4 +106,74 @@ class JavaUDFOpDescSpec extends AnyFlatSpec with Matchers {
j.workers shouldBe 4
j.retainInputColumns shouldBe true
}
+
+ "JavaUDFOpDesc.getPhysicalOp" should
+ "build a parallelizable one-to-many op when workers > 1" in {
+ val d = new JavaUDFOpDesc
+ d.code = "return t;"
+ d.workers = 2
+ val physical = d.getPhysicalOp(workflowId, executionId)
+ physical.parallelizable shouldBe true
+ physical.isOneToManyOp shouldBe true
+ physical.suggestedWorkerNum shouldBe Some(2)
+ physical.opExecInitInfo match {
+ case OpExecWithCode(code, language) =>
+ code shouldBe "return t;"
+ language shouldBe "java"
+ case other => fail(s"expected OpExecWithCode, got $other")
+ }
+ }
+
+ "JavaUDFOpDesc.operatorInfo" should
+ "derive input ports from a configured inputPorts list" in {
+ val d = new JavaUDFOpDesc
+ d.inputPorts = List(
+ PortDescription(
+ portID = "0",
+ displayName = "left",
+ disallowMultiInputs = true,
+ isDynamicPort = false,
+ partitionRequirement = null,
+ dependencies = List.empty
+ )
+ )
+ val info = d.operatorInfo
+ info.inputPorts should have length 1
+ info.inputPorts.head.displayName shouldBe "left"
+ // also drives the getPhysicalOp inputPorts != null partitionRequirement
branch
+ val physical = d.getPhysicalOp(workflowId, executionId)
+ physical.inputPorts.keySet shouldBe info.inputPorts.map(_.id).toSet
+ }
Review Comment:
This test constructs a PortDescription with `partitionRequirement = null`,
but PortDescription.partitionRequirement is a non-optional PartitionInfo. Using
null here weakens the test and can mask NPEs in future code paths; use a
concrete PartitionInfo and assert the resulting physical op’s
partitionRequirement mapping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
