This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6822-58e9e0acab6ca45be362d1ab3acaa3ffa70a189b in repository https://gitbox.apache.org/repos/asf/texera.git
commit 17cdba4ecd22d5574bd8d9f063a525438c3856e8 Author: Xinyuan Lin <[email protected]> AuthorDate: Thu Jul 23 17:22:19 2026 -0700 test(pybuilder): add unit test coverage for BoundaryValidator and EncodableInspector (#6822) ### What changes were proposed in this PR? Adds unit coverage for the two macro-only pybuilder helpers. Both classes run only during macro expansion, so the tests drive real expansions through a runtime `ToolBox` (the pattern the existing suite already uses) and assert on the captured expansion errors/behavior. - `BoundaryValidatorSpec` (+17): the `validateCompileTime` abort branches (splice inside an unclosed quote, after a `#` comment, bad left/right neighbor) and pass-through cases, plus the `runtimeChecksForNestedBuilder` guard (throws only for Encodable content in an unsafe context). - `EncodableInspectorSpec` (new, 12): Encodable detection via TYPE_USE annotation, symbol annotation, case-class accessor hop, and annotated return type; plus the non-Encodable cases and the nested-builder short-circuit. No source files changed. ### Any related issues, documentation, discussions? Closes #6820. ### How was this PR tested? `sbt -java-home <jbr-17> "PyBuilder/testOnly *BoundaryValidatorSpec *EncodableInspectorSpec"` -> 32 succeeded, 0 failed. scalafmtCheckAll + scalafixAll --check clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .../amber/pybuilder/BoundaryValidatorSpec.scala | 212 ++++++++++++++++++++- .../amber/pybuilder/EncodableInspectorSpec.scala | 192 +++++++++++++++++++ 2 files changed, 400 insertions(+), 4 deletions(-) diff --git a/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/BoundaryValidatorSpec.scala b/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/BoundaryValidatorSpec.scala index d009b52fde..cda21efac6 100644 --- a/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/BoundaryValidatorSpec.scala +++ b/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/BoundaryValidatorSpec.scala @@ -20,16 +20,72 @@ package org.apache.texera.amber.pybuilder import org.apache.texera.amber.pybuilder.BoundaryValidator.{CompileTimeContext, RuntimeContext} +import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.{ + EncodableStringRenderer, + PythonTemplateBuilderStringContext +} import org.scalatest.funsuite.AnyFunSuite +import scala.reflect.runtime.currentMirror +import scala.tools.reflect.{ToolBox, ToolBoxError} + /** - * Characterization tests for the data carriers on `BoundaryValidator`'s - * companion. In production the macro is the only place that constructs - * these, so Jacoco never sees them at runtime; this spec pins the - * apply/accessor contract that the rest of the macro pipeline depends on. + * Tests for `BoundaryValidator`. + * + * The first block are characterization tests for the data carriers on the companion. In + * production the macro is the only place that constructs these, so Jacoco never sees them at + * runtime; this pins the apply/accessor contract the rest of the macro pipeline depends on. + * + * The rest drive the two validation methods through a runtime `ToolBox` so their branches execute + * inside the instrumented JVM. A `pyb` snippet can only be *compiled* (not `eval`-run) through the + * ToolBox, because the expanded code calls the `private[amber]` `fromInterpolated`, which the + * ToolBox's synthetic `__wrapper` package cannot access. Compilation is enough: the macro fully + * expands (running the validator) before that access error surfaces, giving two observable + * outcomes we assert on via the captured `ToolBoxError` message: + * + * - a `validateCompileTime` '''abort''' for a *direct* Encodable arg in an unsafe context + * (message carries the specific boundary reason), versus + * - a '''benign''' expansion whose only failure is the `fromInterpolated` access error, meaning + * the validator ran without aborting. + * + * `runtimeChecksForNestedBuilder` never aborts compilation (it emits a deferred runtime guard), so + * its branches are covered by compiling nested-builder snippets in each context, and the emitted + * guard's actual throw/pass behavior is verified separately with direct `pyb` interpolations. */ class BoundaryValidatorSpec extends AnyFunSuite { + private lazy val tb: ToolBox[scala.reflect.runtime.universe.type] = currentMirror.mkToolBox() + + private val header = + """import org.apache.texera.amber.pybuilder.PythonTemplateBuilder._ + |import org.apache.texera.amber.pybuilder.PyStringTypes._""".stripMargin + + /** Marker present in every `BoundaryValidator` compile-time abort message. */ + private val boundaryMarker = "@EncodableStringAnnotation argument #" + + /** Compile a self-contained `pyb` snippet; it always fails, so capture the ToolBox message. */ + private def macroError(body: String): String = + intercept[ToolBoxError] { + tb.compile(tb.parse(s"{\n$header\n$body\n}")) + }.getMessage + + /** Assert `validateCompileTime` aborted with the given reason fragment. */ + private def assertAborts(body: String, fragment: String): Unit = { + val msg = macroError(body) + assert(msg.contains(boundaryMarker), s"expected a boundary abort, got: $msg") + assert(msg.contains(fragment), s"expected reason fragment [$fragment], got: $msg") + } + + /** Assert `validateCompileTime` did NOT abort: only the benign fromInterpolated access fails. */ + private def assertNoAbort(body: String): Unit = { + val msg = macroError(body) + assert(!msg.contains(boundaryMarker), s"unexpected boundary abort: $msg") + assert( + msg.contains("fromInterpolated"), + s"expected the benign private-access failure after full expansion, got: $msg" + ) + } + test("BoundaryValidator companion object is loadable") { // Force a direct reference to the outer companion (not just the nested // CompileTimeContext / RuntimeContext) so its static initializer is @@ -69,4 +125,152 @@ class BoundaryValidatorSpec extends AnyFunSuite { assert(ctx.argIndex == 3) assert(ctx.errorPos == "Foo.scala:42") } + + // ======================================================================== + // validateCompileTime: direct Encodable args, unsafe boundaries -> abort. + // Each abort asserts on the specific templated reason from BoundaryErrors. + // ======================================================================== + + test("validateCompileTime aborts when a direct Encodable arg is inside a quoted string") { + assertAborts( + """val ui: EncodableString = "x" + |pyb"print('$ui')"""".stripMargin, + "inside a quoted Python string literal" + ) + } + + test("validateCompileTime aborts when a direct Encodable arg follows a comment marker") { + assertAborts( + """val ui: EncodableString = "x" + |pyb"code # $ui"""".stripMargin, + "after a '#' comment marker" + ) + } + + test("validateCompileTime aborts when a direct Encodable arg is glued to the left neighbor") { + assertAborts( + """val ui: EncodableString = "x" + |pyb"foo$ui"""".stripMargin, + "on the left" + ) + } + + test("validateCompileTime aborts when a direct Encodable arg is glued to the right neighbor") { + assertAborts( + """val ui: EncodableString = "x" + |pyb"${ui}bar"""".stripMargin, + "on the right" + ) + } + + test("validateCompileTime aborts on a quote neighbor (isBadNeighbor quote, not identifier)") { + // Exercises the isBadNeighbor quote branch rather than the ident branch. + assertAborts( + """val ui: EncodableString = "x" + |pyb"${ui}'"""".stripMargin, + "on the right" + ) + } + + test("validateCompileTime allows a direct Encodable arg with whitespace neighbors") { + // All four checks fall through: no unclosed quote, no comment, both neighbors safe. + assertNoAbort( + """val ui: EncodableString = "x" + |pyb"foo $ui bar"""".stripMargin + ) + } + + test("validateCompileTime allows a direct Encodable arg with empty left and right parts") { + // Exercises both `leftPart.nonEmpty == false` and `rightPart.nonEmpty == false` branches. + assertNoAbort( + """val ui: EncodableString = "x" + |pyb"$ui"""".stripMargin + ) + } + + test("validateCompileTime allows a direct Encodable arg next to safe punctuation") { + // leftPart/rightPart are non-empty but the neighbors are not bad (comma/paren). + assertNoAbort( + """val ui: EncodableString = "x" + |pyb"f($ui, 1)"""".stripMargin + ) + } + + // ======================================================================== + // runtimeChecksForNestedBuilder: never aborts, emits a deferred guard. + // Compiling nested-builder snippets in each context runs every branch of + // the method (insideQuoted / afterComment / left / right / empty -> Nil). + // ======================================================================== + + test("nested builder inside quotes expands with a deferred guard (no compile abort)") { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"print('$inner')"""".stripMargin + ) + } + + test("nested builder after a comment marker expands with a deferred guard (no compile abort)") { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"code # $inner"""".stripMargin + ) + } + + test( + "nested builder glued to the left neighbor expands with a deferred guard (no compile abort)" + ) { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"foo$inner"""".stripMargin + ) + } + + test( + "nested builder glued to the right neighbor expands with a deferred guard (no compile abort)" + ) { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"${inner}bar"""".stripMargin + ) + } + + test("nested builder in a safe context emits no guard at all (throwStmts empty -> Nil)") { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"foo $inner bar"""".stripMargin + ) + } + + test("nested builder at the string edges emits no guard (both neighbor Options are None)") { + assertNoAbort( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"$inner"""".stripMargin + ) + } + + // ======================================================================== + // Behavior of the emitted guard (direct pyb; the throw/pass happens at + // runtime). These pin that the deferred check actually fires only when the + // nested builder carries Encodable content AND the context is unsafe. + // ======================================================================== + + test("emitted guard throws when a nested Encodable builder sits in an unsafe context") { + val inner = pyb"${EncodableStringRenderer("x")}" + intercept[IllegalArgumentException](pyb"print('$inner')") + intercept[IllegalArgumentException](pyb"code # $inner") + intercept[IllegalArgumentException](pyb"foo$inner") + intercept[IllegalArgumentException](pyb"${inner}bar") + } + + test("emitted guard does not throw when the nested Encodable builder is in a safe context") { + val inner = pyb"${EncodableStringRenderer("x")}" + assert(pyb"foo $inner bar".plain == "foo x bar") + assert(pyb"$inner".plain == "x") + } + + test("no guard fires when the nested builder carries no Encodable content") { + val inner = pyb"hello" + assert(pyb"foo$inner".plain == "foohello") + assert(pyb"print('$inner')".plain == "print('hello')") + } } diff --git a/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/EncodableInspectorSpec.scala b/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/EncodableInspectorSpec.scala new file mode 100644 index 0000000000..d87f782508 --- /dev/null +++ b/common/pybuilder/src/test/scala/org/apache/texera/amber/pybuilder/EncodableInspectorSpec.scala @@ -0,0 +1,192 @@ +/* + * 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.pybuilder + +import org.scalatest.funsuite.AnyFunSuite + +import scala.reflect.runtime.currentMirror +import scala.tools.reflect.{ToolBox, ToolBoxError} + +/** + * Runtime coverage for `EncodableInspector`. + * + * `EncodableInspector` only executes while the `pyb"..."` macro expands. In an ordinary build + * that expansion happens before Jacoco is attached, so none of its classification logic is + * observed at runtime. To exercise it inside the instrumented JVM we drive real macro expansions + * through a runtime `ToolBox`. + * + * We cannot `eval` a `pyb` snippet: the macro expands to a call to the `private[amber]` + * `PythonTemplateBuilder.fromInterpolated`, which the ToolBox's synthetic `__wrapper` package + * cannot access, so evaluation always fails on that access check. Compilation is still useful, + * though, because the macro fully expands (running the whole inspector) *before* that + * post-typecheck access error surfaces. That gives two observable outcomes we assert on: + * + * - '''boundary abort''': the inspector classified a *direct* argument as Encodable, so + * `BoundaryValidator.validateCompileTime` ran and aborted with a `@EncodableStringAnnotation` + * message (only possible when the arg was deemed Encodable and the context is unsafe); + * - '''benign expansion''': no boundary abort fired and the only failure is the private-access + * error on `fromInterpolated`, i.e. the inspector deemed the arg a plain literal (or a nested + * builder), the macro finished expanding, and lowering/`wrapArg` ran. + * + * Placing an Encodable-marked value next to a "bad neighbor" therefore proves it was classified + * Encodable; placing a plain value there proves it was not. + */ +class EncodableInspectorSpec extends AnyFunSuite { + + private lazy val tb: ToolBox[scala.reflect.runtime.universe.type] = currentMirror.mkToolBox() + + private val header = + """import org.apache.texera.amber.pybuilder.PythonTemplateBuilder._ + |import org.apache.texera.amber.pybuilder.PyStringTypes._ + |import org.apache.texera.amber.pybuilder.EncodableStringAnnotation""".stripMargin + + /** Marker present in every `BoundaryValidator` compile-time abort message. */ + private val boundaryMarker = "@EncodableStringAnnotation argument #" + + /** Compile a self-contained `pyb` snippet; it always fails, so capture the ToolBox message. */ + private def macroError(body: String): String = + intercept[ToolBoxError] { + tb.compile(tb.parse(s"{\n$header\n$body\n}")) + }.getMessage + + /** Assert the arg was classified Encodable: an unsafe splice triggers a boundary abort. */ + private def assertClassifiedEncodable(body: String): Unit = { + val msg = macroError(body) + assert(msg.contains(boundaryMarker), s"expected an Encodable boundary abort, got: $msg") + } + + /** Assert there was no BoundaryValidator compile-time abort: the macro expands and only fromInterpolated fails. */ + private def assertNotClassifiedEncodable(body: String): Unit = { + val msg = macroError(body) + assert(!msg.contains(boundaryMarker), s"unexpected Encodable boundary abort: $msg") + assert( + msg.contains("fromInterpolated"), + s"expected the benign private-access failure after full expansion, got: $msg" + ) + } + + // ======================================================================== + // Classified Encodable (unsafe splice => BoundaryValidator aborts). + // Each case forces a distinct detection path inside the inspector. + // ======================================================================== + + test("TYPE_USE EncodableString alias is classified as Encodable") { + // typeHasEncodableString -> AnnotatedType branch. + assertClassifiedEncodable( + """val ui: EncodableString = "x" + |pyb"foo$ui"""".stripMargin + ) + } + + test("inline String @EncodableStringAnnotation type is classified as Encodable") { + assertClassifiedEncodable( + """val ui: String @EncodableStringAnnotation = "x" + |pyb"foo$ui"""".stripMargin + ) + } + + test("@EncodableStringAnnotation local val (symbol annotation, inferred type) is Encodable") { + // treeHasEncodableString -> symHasAnn via safeAccessed on the val's own symbol. + assertClassifiedEncodable( + """@EncodableStringAnnotation val ui = "x" + |pyb"foo$ui"""".stripMargin + ) + } + + test("@(EncodableStringAnnotation @field) case class field is Encodable via accessor hop") { + // safeAccessed hops from the accessor to the annotated backing field. + assertClassifiedEncodable( + """import scala.annotation.meta.field + |final case class Holder(@(EncodableStringAnnotation @field) ui: String) + |val h = Holder("x") + |pyb"foo${h.ui}"""".stripMargin + ) + } + + test("@EncodableStringAnnotation def return type is classified as Encodable") { + // methodReturnHasAnn -> typeHasEncodableString(finalResultType). + assertClassifiedEncodable( + """object Holder { @EncodableStringAnnotation def ui: String = "x" } + |pyb"foo${Holder.ui}"""".stripMargin + ) + } + + test("pre-wrapped EncodableStringRenderer arg is classified as Encodable") { + // isDirectEncodableStringArg via the encodableStringRendererTpe subtype check. + assertClassifiedEncodable( + """val r = EncodableStringRenderer("x") + |pyb"foo$r"""".stripMargin + ) + } + + // ======================================================================== + // NOT classified Encodable (macro expands; only fromInterpolated access fails). + // ======================================================================== + + test("plain Int arg is not classified as Encodable") { + assertNotClassifiedEncodable("""pyb"foo${42}"""") + } + + test("unannotated String arg is not classified as Encodable") { + assertNotClassifiedEncodable( + """val raw: String = "x" + |pyb"foo$raw"""".stripMargin + ) + } + + test("case class param annotated WITHOUT @field is not reachable as Encodable via the accessor") { + // The accessor's accessed symbol carries no annotation, so classification stays literal. + assertNotClassifiedEncodable( + """final case class Holder(@EncodableStringAnnotation ui: String) + |val h = Holder("x") + |pyb"foo${h.ui}"""".stripMargin + ) + } + + test("pre-wrapped PyLiteralStringRenderer arg is not classified as Encodable") { + assertNotClassifiedEncodable( + """val r = PyLiteralStringRenderer("x") + |pyb"foo$r"""".stripMargin + ) + } + + test("nested PythonTemplateBuilder arg short-circuits the direct-Encodable check") { + // isPythonTemplateBuilderArg true => isDirectEncodableStringArg returns false immediately, + // so even a nested builder that carries Encodable content is never a *direct* Encodable arg. + assertNotClassifiedEncodable( + """val inner = pyb"${EncodableStringRenderer("x")}" + |pyb"foo$inner"""".stripMargin + ) + } + + // ======================================================================== + // wrapArg lowering: only runs when there is no compile-time abort, so a *safe* + // splice of an Encodable value exercises wrapArg's EncodableStringRenderer branch. + // (The literal and StringRenderer-cast branches are covered by the plain-value and + // PyLiteralStringRenderer cases above, whose lowering also runs.) + // ======================================================================== + + test("safe Encodable splice expands through wrapArg without a boundary abort") { + assertNotClassifiedEncodable( + """val ui: EncodableString = "x" + |pyb"a $ui b"""".stripMargin + ) + } +}
