aglinxinyuan commented on code in PR #5740: URL: https://github.com/apache/texera/pull/5740#discussion_r3424478109
########## common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/ReadonlyVirtualDocumentSpec.scala: ########## @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.core.storage.model + +import org.scalatest.flatspec.AnyFlatSpec + +import java.io.{ByteArrayInputStream, File, InputStream} +import java.net.URI + +class ReadonlyVirtualDocumentSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Stub impl — provides one sentinel value per accessor so each abstract + // method can be exercised in isolation, plus an int-typed variant so + // the type parameter `T` is observed end-to-end. + // --------------------------------------------------------------------------- + + private class StubReadonlyIntDoc(items: IndexedSeq[Int]) extends ReadonlyVirtualDocument[Int] { + override def getURI: URI = new URI("file:///stub/int") + override def getItem(i: Int): Int = items(i) + override def get(): Iterator[Int] = items.iterator + override def getRange(from: Int, until: Int, columns: Option[Seq[String]]): Iterator[Int] = + items.slice(from, until).iterator + override def getAfter(offset: Int): Iterator[Int] = items.drop(offset + 1).iterator + override def getCount: Long = items.size.toLong + override def asInputStream(): InputStream = + new ByteArrayInputStream(items.map(_.toByte).toArray) + override def asFile(): File = new File("/tmp/stub-int") + } + + // --------------------------------------------------------------------------- + // Accessor surface — return verbatim + // --------------------------------------------------------------------------- + + "ReadonlyVirtualDocument.getURI" should "return the impl-supplied URI" in { + val d = new StubReadonlyIntDoc(IndexedSeq(1, 2, 3)) + assert(d.getURI == new URI("file:///stub/int")) + } + + "ReadonlyVirtualDocument.getItem" should "delegate to the impl's index lookup" in { + val d = new StubReadonlyIntDoc(IndexedSeq(10, 20, 30)) + assert(d.getItem(0) == 10) + assert(d.getItem(2) == 30) + } + + "ReadonlyVirtualDocument.get" should "iterate every item from the impl in order" in { + val d = new StubReadonlyIntDoc(IndexedSeq(7, 8, 9)) + assert(d.get().toList == List(7, 8, 9)) + } + + "ReadonlyVirtualDocument.getRange" should + "yield items in `[from, until)` (half-open interval — `until` is exclusive)" in { + val d = new StubReadonlyIntDoc(IndexedSeq(0, 1, 2, 3, 4)) + assert(d.getRange(1, 4).toList == List(1, 2, 3)) + } Review Comment: Fixed in 154bbdc7d4 — the `getRange(1, 4)` case now types the value as `ReadonlyVirtualDocument[Int]`, so the default-arg contract is resolved through the trait at the call site (Scala resolves default parameters from the static type). ########## common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/ReadonlyVirtualDocumentSpec.scala: ########## @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.core.storage.model + +import org.scalatest.flatspec.AnyFlatSpec + +import java.io.{ByteArrayInputStream, File, InputStream} +import java.net.URI + +class ReadonlyVirtualDocumentSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Stub impl — provides one sentinel value per accessor so each abstract + // method can be exercised in isolation, plus an int-typed variant so + // the type parameter `T` is observed end-to-end. + // --------------------------------------------------------------------------- + + private class StubReadonlyIntDoc(items: IndexedSeq[Int]) extends ReadonlyVirtualDocument[Int] { + override def getURI: URI = new URI("file:///stub/int") + override def getItem(i: Int): Int = items(i) + override def get(): Iterator[Int] = items.iterator + override def getRange(from: Int, until: Int, columns: Option[Seq[String]]): Iterator[Int] = + items.slice(from, until).iterator + override def getAfter(offset: Int): Iterator[Int] = items.drop(offset + 1).iterator + override def getCount: Long = items.size.toLong + override def asInputStream(): InputStream = + new ByteArrayInputStream(items.map(_.toByte).toArray) + override def asFile(): File = new File("/tmp/stub-int") + } + + // --------------------------------------------------------------------------- + // Accessor surface — return verbatim + // --------------------------------------------------------------------------- + + "ReadonlyVirtualDocument.getURI" should "return the impl-supplied URI" in { + val d = new StubReadonlyIntDoc(IndexedSeq(1, 2, 3)) + assert(d.getURI == new URI("file:///stub/int")) + } + + "ReadonlyVirtualDocument.getItem" should "delegate to the impl's index lookup" in { + val d = new StubReadonlyIntDoc(IndexedSeq(10, 20, 30)) + assert(d.getItem(0) == 10) + assert(d.getItem(2) == 30) + } + + "ReadonlyVirtualDocument.get" should "iterate every item from the impl in order" in { + val d = new StubReadonlyIntDoc(IndexedSeq(7, 8, 9)) + assert(d.get().toList == List(7, 8, 9)) + } + + "ReadonlyVirtualDocument.getRange" should + "yield items in `[from, until)` (half-open interval — `until` is exclusive)" in { + val d = new StubReadonlyIntDoc(IndexedSeq(0, 1, 2, 3, 4)) + assert(d.getRange(1, 4).toList == List(1, 2, 3)) + } + + it should "accept an optional `columns` argument (default None) without changing the result" in { + val d = new StubReadonlyIntDoc(IndexedSeq(0, 1, 2)) + // Call-site with all three positional args. + assert(d.getRange(0, 2, columns = Some(Seq("c"))).toList == List(0, 1)) + // Call-site that omits the third arg — must compile via default. + assert(d.getRange(0, 2).toList == List(0, 1)) + } Review Comment: Fixed in 154bbdc7d4 — same approach: `d` is now declared as `ReadonlyVirtualDocument[Int]` so the `getRange(0, 2)` call resolves `columns: Option[…] = None` through the trait at compile time. ########## common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/ReadonlyVirtualDocumentSpec.scala: ########## @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.core.storage.model + +import org.scalatest.flatspec.AnyFlatSpec + +import java.io.{ByteArrayInputStream, File, InputStream} +import java.net.URI + +class ReadonlyVirtualDocumentSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Stub impl — provides one sentinel value per accessor so each abstract + // method can be exercised in isolation, plus an int-typed variant so + // the type parameter `T` is observed end-to-end. + // --------------------------------------------------------------------------- + + private class StubReadonlyIntDoc(items: IndexedSeq[Int]) extends ReadonlyVirtualDocument[Int] { + override def getURI: URI = new URI("file:///stub/int") + override def getItem(i: Int): Int = items(i) + override def get(): Iterator[Int] = items.iterator + override def getRange(from: Int, until: Int, columns: Option[Seq[String]]): Iterator[Int] = + items.slice(from, until).iterator + override def getAfter(offset: Int): Iterator[Int] = items.drop(offset + 1).iterator + override def getCount: Long = items.size.toLong + override def asInputStream(): InputStream = + new ByteArrayInputStream(items.map(_.toByte).toArray) + override def asFile(): File = new File("/tmp/stub-int") + } Review Comment: Fixed in 154bbdc7d4 — replaced the hardcoded `/tmp/stub-int` path with `new File(System.getProperty("java.io.tmpdir"), "stub-int")` so the stub is OS-portable (Windows / macOS / Linux). ########## common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/VirtualCollectionSpec.scala: ########## @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.core.storage.model + +import org.scalatest.flatspec.AnyFlatSpec + +import java.net.URI +import scala.collection.mutable + +class VirtualCollectionSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Test harness — a minimal in-memory concrete impl exercises every + // abstract method (getURI / getDocuments / getDocument / remove). + // + // The contained `VirtualDocument`s are stubbed with the smallest + // concrete impl: `clear()` and `getURI` are the only abstract members + // not given a default by the base class. + // --------------------------------------------------------------------------- + + private class StubDocument(uriValue: URI) extends VirtualDocument[Nothing] { + override def getURI: URI = uriValue + override def clear(): Unit = () + } + + private class StubCollection(uriValue: URI) extends VirtualCollection { + private val children = mutable.LinkedHashMap.empty[String, VirtualDocument[_]] + private var removed = false + + def addChild(name: String, doc: VirtualDocument[_]): Unit = children(name) = doc + def wasRemoved: Boolean = removed + + override def getURI: URI = uriValue + override def getDocuments: List[VirtualDocument[_]] = children.values.toList + override def getDocument(name: String): VirtualDocument[_] = + children.getOrElse(name, throw new NoSuchElementException(name)) + override def remove(): Unit = { + children.clear() + removed = true + } + } + + private def uri(s: String): URI = new URI(s) + + // --------------------------------------------------------------------------- + // Trait declares four abstract methods — pinned via concrete subclass + // --------------------------------------------------------------------------- Review Comment: Fixed in 154bbdc7d4 — replaced every "trait" reference with "abstract class" (in the section divider, the `getDocument` comment, and the type-pattern case-name + comment). VirtualCollection is declared `abstract class`, not `trait`. -- 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]
