Copilot commented on code in PR #7556:
URL: https://github.com/apache/texera/pull/7556#discussion_r3759441482
##########
amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala:
##########
@@ -79,12 +115,20 @@ class ExecutionResultServiceSpec
private val testUid: Integer = 9001
private var executionsDao: WorkflowExecutionsDao = _
private var testVid: Integer = _
+ private var testCuid: Integer = _
+
+ // AmberClient needs an ActorSystem to host its ClientActor. A bare one is
enough:
+ // the client is constructed over an empty PhysicalPlan, so its
InitializeRequest
+ // completes without an engine, and the subclass below never sends anything
to it.
+ private var system: ActorSystem = _
Review Comment:
`ActorSystem.terminate()` is asynchronous and isn’t awaited here, which can
leak threads/actors and cause flaky or hanging test runs. Also, the hard-coded
system name can collide when suites run in parallel in the same JVM (e.g., via
JMX/MBean registration). Consider using a unique name (suffix with
`System.nanoTime`/UUID) and awaiting termination (or using CoordinatedShutdown)
before `shutdownDB()`.
##########
amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala:
##########
@@ -79,12 +115,20 @@ class ExecutionResultServiceSpec
private val testUid: Integer = 9001
private var executionsDao: WorkflowExecutionsDao = _
private var testVid: Integer = _
+ private var testCuid: Integer = _
+
+ // AmberClient needs an ActorSystem to host its ClientActor. A bare one is
enough:
+ // the client is constructed over an empty PhysicalPlan, so its
InitializeRequest
+ // completes without an engine, and the subclass below never sends anything
to it.
+ private var system: ActorSystem = _
override protected def beforeAll(): Unit = {
initializeDBAndReplaceDSLContext()
+ system = ActorSystem("ExecutionResultServiceSpec")
}
override protected def afterAll(): Unit = {
+ system.terminate()
shutdownDB()
}
Review Comment:
`ActorSystem.terminate()` is asynchronous and isn’t awaited here, which can
leak threads/actors and cause flaky or hanging test runs. Also, the hard-coded
system name can collide when suites run in parallel in the same JVM (e.g., via
JMX/MBean registration). Consider using a unique name (suffix with
`System.nanoTime`/UUID) and awaiting termination (or using CoordinatedShutdown)
before `shutdownDB()`.
--
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]
