This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new d76a51e347 test(amber): add unit test coverage for FutureBijection and
ElidableStatement (#5555)
d76a51e347 is described below
commit d76a51e347f54c6c3ff43a7f8cd11f14ae5739ea
Author: Xinyuan Lin <[email protected]>
AuthorDate: Fri Jun 12 10:06:53 2026 -0700
test(amber): add unit test coverage for FutureBijection and
ElidableStatement (#5555)
### What changes were proposed in this PR?
Pin behavior of two utility modules in `engine/common`. No
production-code changes.
| Spec | Source class | Tests |
| --- | --- | --- |
| `FutureBijectionSpec` | `FutureBijection` | 11 |
| `ElidableStatementSpec` | `ElidableStatement` | 9 |
Both spec files follow the `<srcClassName>Spec.scala` one-to-one
convention.
**Behavior pinned — `FutureBijection`**
| Surface | Contract |
| --- | --- |
| `TwitterFuture.value.asScala` | resolves to the same value (type
preserved, `null` preserved) |
| `TwitterFuture.exception.asScala` | resolves with the same `Throwable`
instance (type, message, `eq` identity) |
| `ScalaFuture.successful.asTwitter` | resolves to the same value (type
preserved, `null` preserved) |
| `ScalaFuture.failed.asTwitter` | resolves with the same `Throwable`
instance |
| Twitter → Scala on an already-resolved future | the resulting Scala
future is already completed when the implicit returns |
| Twitter → Scala → Twitter round-trip | preserves both values and
exceptions |
| Scala → Twitter → Scala round-trip | preserves values |
**Behavior pinned — `ElidableStatement`**
The texera build sets `-Xelide-below WARNING` (`amber/build.sbt`). Every
`ElidableStatement` helper is annotated with an elide level **strictly
below WARNING** (FINEST / FINER / FINE / INFO), so the Scala compiler
replaces every CALL to these helpers with a `()` Unit value at *compile*
time. The spec pins this silent-in-production contract:
| Surface | Contract |
| --- | --- |
| `info` / `fine` / `finer` / `finest` (with side-effect block) | the
side effect MUST NOT fire (counter stays at 0) |
| same methods (with throwing block) | the exception MUST NOT propagate
|
| 1000 successive elided calls | no side-effect accumulation |
| Return type | `Unit` (compile-time enforced) |
| Parameter shape | accepts a `=> Unit` by-name block (compile-time
enforced) |
A regression that bumped a method's elide level above WARNING, removed
the `@elidable` annotation, or relaxed `-Xelide-below` in the build
would re-enable side effects in production — and this spec would catch
it.
### Any related issues, documentation, discussions?
Closes #5551.
### How was this PR tested?
Pure unit-test additions; verified locally with:
- `sbt "WorkflowExecutionService/testOnly
org.apache.texera.amber.engine.common.FutureBijectionSpec
org.apache.texera.amber.engine.common.ElidableStatementSpec"` — 20
tests, all green
- `sbt scalafmtCheckAll` — clean
- CI to confirm
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Sonnet 4.5)
---
.../engine/common/ElidableStatementSpec.scala | 141 +++++++++++++++++++++
.../amber/engine/common/FutureBijectionSpec.scala | 139 ++++++++++++++++++++
2 files changed, 280 insertions(+)
diff --git
a/amber/src/test/scala/org/apache/texera/amber/engine/common/ElidableStatementSpec.scala
b/amber/src/test/scala/org/apache/texera/amber/engine/common/ElidableStatementSpec.scala
new file mode 100644
index 0000000000..6f7e2eb2b3
--- /dev/null
+++
b/amber/src/test/scala/org/apache/texera/amber/engine/common/ElidableStatementSpec.scala
@@ -0,0 +1,141 @@
+/*
+ * 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.engine.common
+
+import org.scalatest.flatspec.AnyFlatSpec
+
+class ElidableStatementSpec extends AnyFlatSpec {
+
+ //
---------------------------------------------------------------------------
+ // Context — the texera build sets `-Xelide-below WARNING` (see
+ // `amber/build.sbt`). Every `ElidableStatement` helper is annotated with
+ // an elide level strictly below WARNING (FINEST / FINER / FINE / INFO),
+ // so the Scala compiler replaces every CALL to these helpers with a
+ // `()` Unit value at *compile* time. The by-name block argument is
+ // never even constructed, let alone evaluated, in production / test
+ // builds — that is the entire point of the abstraction.
+ //
+ // This spec pins that contract: a regression that bumped a method's
+ // elide level above WARNING (e.g. `@elidable(SEVERE)`), removed the
+ // `@elidable` annotation, or relaxed `-Xelide-below` in the build
+ // would re-enable side effects and break the silent-in-production
+ // promise — and this spec would catch it.
+ //
---------------------------------------------------------------------------
+
+ //
---------------------------------------------------------------------------
+ // Each helper compiles to a no-op (block side effect does NOT fire)
+ //
---------------------------------------------------------------------------
+
+ "ElidableStatement.finest" should
+ "be elided at the build's elide level — its by-name block must NOT
execute" in {
+ var counter = 0
+ ElidableStatement.finest { counter += 1 }
+ assert(counter == 0, "block should be elided away, counter must remain at
0")
+ }
+
+ "ElidableStatement.finer" should
+ "be elided at the build's elide level — its by-name block must NOT
execute" in {
+ var counter = 0
+ ElidableStatement.finer { counter += 1 }
+ assert(counter == 0)
+ }
+
+ "ElidableStatement.fine" should
+ "be elided at the build's elide level — its by-name block must NOT
execute" in {
+ var counter = 0
+ ElidableStatement.fine { counter += 1 }
+ assert(counter == 0)
+ }
+
+ "ElidableStatement.info" should
+ "be elided at the build's elide level — its by-name block must NOT
execute" in {
+ var counter = 0
+ ElidableStatement.info { counter += 1 }
+ assert(counter == 0)
+ }
+
+ //
---------------------------------------------------------------------------
+ // Even a throwing block must NOT propagate — it's never evaluated.
+ //
---------------------------------------------------------------------------
+
+ "Elided helpers" should
+ "not propagate an exception that would have been thrown by their block" in
{
+ // If `info` accidentally stopped being elided, this would re-raise the
+ // RuntimeException and fail the test. Pinning the suppression directly
+ // catches that regression.
+ ElidableStatement.info { throw new RuntimeException("must never fire") }
+ ElidableStatement.fine { throw new RuntimeException("must never fire") }
+ ElidableStatement.finer { throw new RuntimeException("must never fire") }
+ ElidableStatement.finest { throw new RuntimeException("must never fire") }
+ succeed
+ }
+
+ //
---------------------------------------------------------------------------
+ // Multiple calls don't accumulate side effects (each one is independently
+ // elided).
+ //
---------------------------------------------------------------------------
+
+ "Repeated elided calls" should "stay no-ops across 1000 invocations" in {
+ var counter = 0
+ var i = 0
+ while (i < 1000) {
+ ElidableStatement.info { counter += 1 }
+ i += 1
+ }
+ assert(
+ counter == 0,
+ s"1000 elided info calls should not accumulate side effects, got:
$counter"
+ )
+ }
+
+ //
---------------------------------------------------------------------------
+ // Return-type contract — each helper still type-checks as `=> Unit ⇒ Unit`.
+ //
---------------------------------------------------------------------------
+
+ "ElidableStatement methods" should "all return Unit (compile-time enforced)"
in {
+ // Assignments would fail to typecheck if a method's signature drifted
+ // — e.g. someone made `info` return the block's result. The fact that
+ // these compile under `-Xelide-below WARNING` also confirms each call
+ // is replaced with the Unit `()` value, not with an exception.
+ val r1: Unit = ElidableStatement.info { () }
+ val r2: Unit = ElidableStatement.fine { () }
+ val r3: Unit = ElidableStatement.finer { () }
+ val r4: Unit = ElidableStatement.finest { () }
+ assert(r1 == r2 && r2 == r3 && r3 == r4)
+ }
+
+ //
---------------------------------------------------------------------------
+ // By-name parameter shape — each helper accepts a `=> Unit` block
+ // (verified at compile time by passing a parameter-less lambda body).
+ //
---------------------------------------------------------------------------
+
+ "ElidableStatement methods" should "accept a by-name `=> Unit` argument
(compile-time enforced)" in {
+ // The fact that these expressions compile proves the parameter shape:
+ // a value-typed expression of type Unit AND a thunk that runs side
+ // effects are both accepted. Under `-Xelide-below WARNING`, neither
+ // executes — but the type contract still holds.
+ ElidableStatement.info { () }
+ ElidableStatement.info { val x = 1; val y = x + 1; () }
+ ElidableStatement.info {
+ println("debug")
+ }
+ succeed
+ }
+}
diff --git
a/amber/src/test/scala/org/apache/texera/amber/engine/common/FutureBijectionSpec.scala
b/amber/src/test/scala/org/apache/texera/amber/engine/common/FutureBijectionSpec.scala
new file mode 100644
index 0000000000..1ff59594dd
--- /dev/null
+++
b/amber/src/test/scala/org/apache/texera/amber/engine/common/FutureBijectionSpec.scala
@@ -0,0 +1,139 @@
+/*
+ * 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.engine.common
+
+import com.twitter.util.{Await => TwitterAwait, Future => TwitterFuture}
+import org.apache.texera.amber.engine.common.FutureBijection._
+import org.scalatest.flatspec.AnyFlatSpec
+
+import scala.concurrent.duration._
+import scala.concurrent.{Await => ScalaAwait, Future => ScalaFuture}
+
+class FutureBijectionSpec extends AnyFlatSpec {
+
+ // Short timeout — the futures complete synchronously (or near-so) for
+ // the values used here; we keep the bound tight so a regression that
+ // *fails to resolve* surfaces as a timeout rather than hanging CI.
+ private val timeout: FiniteDuration = 5.seconds
+ private val twitterTimeout: com.twitter.util.Duration =
com.twitter.util.Duration.fromSeconds(5)
+
+ //
---------------------------------------------------------------------------
+ // Twitter Future → Scala Future
+ //
---------------------------------------------------------------------------
+
+ "RichTwitterFuture.asScala" should "resolve to the same value as the wrapped
TwitterFuture" in {
+ val tf = TwitterFuture.value(42)
+ val result = ScalaAwait.result(tf.asScala, timeout)
+ assert(result == 42)
+ }
+
+ it should "preserve the exception type and message on the failure path" in {
+ val cause = new IllegalStateException("boom")
+ val tf = TwitterFuture.exception[Int](cause)
+ val ex = intercept[IllegalStateException] {
+ ScalaAwait.result(tf.asScala, timeout)
+ }
+ assert(ex.getMessage == "boom")
+ assert(ex eq cause, "the same Throwable instance should propagate through")
+ }
+
+ it should "preserve a null value on the value path (Twitter Return wraps any
AnyRef)" in {
+ // The conversion calls `promise.success(value)` directly; for a `null`
+ // value, the Scala promise resolves to `null`. Pin that the wire does
+ // not coerce null into an exception.
+ val tf = TwitterFuture.value[String](null)
+ val result = ScalaAwait.result(tf.asScala, timeout)
+ assert(result == null)
+ }
+
+ it should "preserve the value type (compile-time enforced)" in {
+ val tf: TwitterFuture[String] = TwitterFuture.value("hello")
+ val sf: ScalaFuture[String] = tf.asScala
+ assert(ScalaAwait.result(sf, timeout) == "hello")
+ }
+
+ it should "produce a future that has already completed when the source
TwitterFuture is already resolved" in {
+ val tf = TwitterFuture.value(7)
+ val sf = tf.asScala
+ // The conversion uses `respond` which fires synchronously for already-
+ // resolved futures, so the scala future is complete by the time the
+ // implicit returns.
+ assert(sf.isCompleted, "the converted future should be completed
immediately")
+ }
+
+ //
---------------------------------------------------------------------------
+ // Scala Future → Twitter Future
+ //
---------------------------------------------------------------------------
+
+ "RichScalaFuture.asTwitter" should "resolve to the same value as the wrapped
ScalaFuture" in {
+ val sf = ScalaFuture.successful(42)
+ val tf = sf.asTwitter()
+ assert(TwitterAwait.result(tf, twitterTimeout) == 42)
+ }
+
+ it should "preserve the exception type and message on the failure path" in {
+ val cause = new IllegalArgumentException("nope")
+ val sf = ScalaFuture.failed[Int](cause)
+ val ex = intercept[IllegalArgumentException] {
+ TwitterAwait.result(sf.asTwitter(), twitterTimeout)
+ }
+ assert(ex.getMessage == "nope")
+ assert(ex eq cause, "the same Throwable instance should propagate through")
+ }
+
+ it should "preserve a null value on the value path" in {
+ val sf = ScalaFuture.successful[String](null)
+ val tf = sf.asTwitter()
+ assert(TwitterAwait.result(tf, twitterTimeout) == null)
+ }
+
+ it should "preserve the value type (compile-time enforced)" in {
+ val sf: ScalaFuture[String] = ScalaFuture.successful("hello")
+ val tf: TwitterFuture[String] = sf.asTwitter()
+ assert(TwitterAwait.result(tf, twitterTimeout) == "hello")
+ }
+
+ //
---------------------------------------------------------------------------
+ // Round-trip — Twitter → Scala → Twitter and vice versa
+ //
---------------------------------------------------------------------------
+
+ "FutureBijection" should "round-trip a value through Twitter → Scala →
Twitter" in {
+ val tf1 = TwitterFuture.value("payload")
+ val tf2: TwitterFuture[String] = tf1.asScala.asTwitter()
+ assert(TwitterAwait.result(tf2, twitterTimeout) == "payload")
+ }
+
+ it should "round-trip a value through Scala → Twitter → Scala" in {
+ val sf1 = ScalaFuture.successful("payload")
+ val sf2: ScalaFuture[String] = sf1.asTwitter().asScala
+ assert(ScalaAwait.result(sf2, timeout) == "payload")
+ }
+
+ it should "round-trip an exception through Twitter → Scala → Twitter (type +
message preserved)" in {
+ val cause = new RuntimeException("round-trip-err")
+ val tf1 = TwitterFuture.exception[Int](cause)
+ val tf2: TwitterFuture[Int] = tf1.asScala.asTwitter()
+ val ex = intercept[RuntimeException] {
+ TwitterAwait.result(tf2, twitterTimeout)
+ }
+ assert(ex.getMessage == "round-trip-err")
+ assert(ex eq cause)
+ }
+}
