Copilot commented on code in PR #6902:
URL: https://github.com/apache/texera/pull/6902#discussion_r3651722513
##########
amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala:
##########
@@ -21,15 +21,29 @@ package org.apache.texera.web.service
import com.fasterxml.jackson.core.JsonProcessingException
import jakarta.ws.rs.core.Response
+import org.apache.arrow.memory.RootAllocator
Review Comment:
This spec mixes `jakarta.ws.rs.core.Response` with production code returning
`javax.ws.rs.core.Response` (and other Amber specs import
`javax.ws.rs.core.Response`). Keeping the same Response type as the service
avoids confusion and accidental type mismatches when using `Response` APIs.
##########
amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala:
##########
@@ -85,4 +99,254 @@ class ResultExportServiceSpec extends AnyFlatSpec with
Matchers {
service.validateExportRequest(requestWith(List(OperatorExportInfo("op-1",
"csv"))))
result shouldBe None
}
+
+ // -- fakes for the export writers
-------------------------------------------
+
+ private val schema: Schema = Schema()
+ .add("name", AttributeType.STRING)
+ .add("count", AttributeType.INTEGER)
+
+ private def tupleOf(name: String, count: Int): Tuple =
+ Tuple.builder(schema).addSequentially(Array[Any](name, count)).build()
+
+ // A minimal in-memory document overriding only the members the writers
touch;
+ // every other VirtualDocument method keeps its default throwing body.
+ private class FakeDoc(
+ rows: Seq[Tuple],
+ countOverride: Option[Long] = None,
+ stream: Option[InputStream] = None
+ ) extends VirtualDocument[Tuple] {
+ override def getURI: URI = new URI("file:///stub/export")
+ override def clear(): Unit = ()
+ override def getCount: Long = countOverride.getOrElse(rows.length.toLong)
+ override def get(): Iterator[Tuple] = rows.iterator
+ override def getRange(from: Int, until: Int, columns:
Option[Seq[String]]): Iterator[Tuple] =
+ rows.slice(from, until).iterator
+ override def asInputStream(): InputStream =
stream.getOrElse(super.asInputStream())
+ }
+
+ private class TrackingInputStream(bytes: Array[Byte]) extends
ByteArrayInputStream(bytes) {
+ var closed = false
+ override def close(): Unit = { closed = true; super.close() }
+ }
+
+ private class TrackingOutputStream extends ByteArrayOutputStream {
+ var closed = false
+ override def close(): Unit = { closed = true; super.close() }
+ }
+
+ private val streamDocumentAsCSV =
PrivateMethod[Unit](Symbol("streamDocumentAsCSV"))
+ private val streamDocumentAsArrow =
PrivateMethod[Unit](Symbol("streamDocumentAsArrow"))
+ private val streamDocumentAsHTML =
PrivateMethod[Unit](Symbol("streamDocumentAsHTML"))
+ private val streamDocumentAsParquetZip =
+ PrivateMethod[Unit](Symbol("streamDocumentAsParquetZip"))
+ private val streamCellData = PrivateMethod[Unit](Symbol("streamCellData"))
+ private val convertFieldToBytes =
PrivateMethod[Array[Byte]](Symbol("convertFieldToBytes"))
+
+ private def utf8(out: ByteArrayOutputStream): String =
+ new String(out.toByteArray, StandardCharsets.UTF_8)
+
+ // Assert on parsed lines rather than raw bytes so the CSV library's choice
of
+ // line terminator does not make these tests brittle.
+ private def csvLines(out: ByteArrayOutputStream): List[String] =
utf8(out).linesIterator.toList
+
+ // -- streamDocumentAsCSV
----------------------------------------------------
+
+ "streamDocumentAsCSV" should "write nothing when the document is empty" in {
+ val out = new ByteArrayOutputStream()
+ service invokePrivate streamDocumentAsCSV(new FakeDoc(Seq.empty), out,
None)
+ out.size shouldBe 0
+ }
+
+ it should "write nothing when the iterator yields no rows despite a non-zero
count" in {
+ val out = new ByteArrayOutputStream()
+ val doc = new FakeDoc(Seq.empty, countOverride = Some(2L))
+ service invokePrivate streamDocumentAsCSV(doc, out, None)
+ out.size shouldBe 0
+ }
+
+ it should "infer the header from the first row's schema and still write that
row" in {
+ val out = new ByteArrayOutputStream()
+ val rows = Seq(tupleOf("a", 1), tupleOf("b", 2))
+ service invokePrivate streamDocumentAsCSV(new FakeDoc(rows), out, None)
+
+ csvLines(out) shouldBe List("name,count", "a,1", "b,2")
+ }
+
+ it should "use the supplied headers without consuming the first row" in {
+ val out = new ByteArrayOutputStream()
+ val rows = Seq(tupleOf("a", 1), tupleOf("b", 2))
+ val headers = Some(List("h1", "h2"))
+ service invokePrivate streamDocumentAsCSV(new FakeDoc(rows), out, headers)
+
+ // The supplied header replaces the inferred one, and — unlike the None
+ // branch above — the first row is not consumed by header inference, so it
+ // still appears in the body.
+ csvLines(out) shouldBe List("h1,h2", "a,1", "b,2")
+ }
+
+ it should "write every row when the document exceeds the chunk size" in {
+ val out = new ByteArrayOutputStream()
+ val rowCount = Constants.CHUNK_SIZE * 2 + 5
+ val rows = (1 to rowCount).map(i => tupleOf(s"r$i", i))
+ service invokePrivate streamDocumentAsCSV(new FakeDoc(rows), out,
Some(List("h1", "h2")))
+
+ val lines = csvLines(out)
+ lines.head shouldBe "h1,h2"
+ lines.tail should have length rowCount.toLong
+ lines.tail.head shouldBe "r1,1"
Review Comment:
`lines.tail` is a `List`, so its `length` is an `Int`. Using
`rowCount.toLong` here is unnecessary and can be a compile-time type mismatch
depending on the ScalaTest matcher overloads.
--
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]
