Copilot commented on code in PR #7441:
URL: https://github.com/apache/texera/pull/7441#discussion_r3743438759
##########
amber/src/test/scala/org/apache/texera/web/service/ExecutionConsoleServiceSpec.scala:
##########
@@ -20,17 +20,57 @@
package org.apache.texera.web.service
import com.google.protobuf.timestamp.Timestamp
+import com.twitter.util.{Future => TwitterFuture}
+import io.reactivex.rxjava3.disposables.Disposable
+import org.apache.pekko.actor.ActorSystem
+import org.apache.pekko.testkit.TestKit
+import org.apache.texera.amber.core.workflow.{PhysicalPlan, WorkflowContext}
+import
org.apache.texera.amber.engine.architecture.coordinator.CoordinatorConfig
import org.apache.texera.amber.engine.architecture.rpc.controlcommands.{
ConsoleMessage,
- ConsoleMessageType
+ ConsoleMessageType,
+ DebugCommandRequest => AmberDebugCommandRequest
}
+import
org.apache.texera.amber.engine.architecture.rpc.controlreturns.EmptyReturn
+import
org.apache.texera.amber.engine.architecture.rpc.coordinatorservice.CoordinatorServiceFs2Grpc
+import org.apache.texera.amber.engine.common.client.AmberClient
import
org.apache.texera.amber.engine.common.executionruntimestate.ExecutionConsoleStore
-import org.scalatest.flatspec.AnyFlatSpec
+import org.apache.texera.web.WebsocketInput
+import org.apache.texera.web.model.websocket.event.TexeraWebSocketEvent
+import org.apache.texera.web.model.websocket.event.python.ConsoleUpdateEvent
+import org.apache.texera.web.model.websocket.request.python.DebugCommandRequest
+import org.apache.texera.web.storage.ExecutionStateStore
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.flatspec.AnyFlatSpecLike
import org.scalatest.matchers.should.Matchers
import java.time.Instant
-
-class ExecutionConsoleServiceSpec extends AnyFlatSpec with Matchers {
+import scala.collection.mutable.ListBuffer
+import scala.reflect.ClassTag
+
+/**
+ * The `ConsoleMessageProcessor` object is covered by the first half of this
suite. The service
+ * class around it was entirely uncovered: it owns the console diff that
decides what the frontend
+ * is told, the worker-to-operator keying that decides where a message lands,
and the websocket
+ * handlers behind the debugger.
+ *
+ * Everything here runs on an empty-plan AmberClient with a mocked
coordinator, so no engine,
+ * database or Iceberg storage is involved. The Iceberg-backed writer path
(`getOrCreateWriter` and
+ * the commit loop on execution state) is `private` and storage-bound, and is
left uncovered rather
+ * than padded with a no-throw test.
Review Comment:
The suite Scaladoc claims "no ... database ... is involved", but these tests
call `consoleCallback` / `wsInput.onNext`, which routes through
`ExecutionConsoleService.addConsoleMessage` and schedules a background task
that calls `WorkflowExecutionsResource.insertOperatorExecutions` +
`DocumentFactory...writer.open()` (i.e., DB/storage side effects). If SqlServer
isn’t initialized in this test scope, that work can throw asynchronously and
won’t fail the assertions, making the suite noisy/flaky. At minimum, please
correct the Scaladoc to avoid stating the suite is DB/storage-free (or
alternatively explicitly initialize/stub the persistence layer in the fixture).
--
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]
