This is an automated email from the ASF dual-hosted git repository.
Yicong-Huang 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 a54518cbf2 fix: unify DeployStrategy empty-array errors to
NoSuchElementException (#5029)
a54518cbf2 is described below
commit a54518cbf2ac09ab50f155c7b8ab928c421bd7dc
Author: Matthew B. <[email protected]>
AuthorDate: Tue May 19 01:14:19 2026 -0700
fix: unify DeployStrategy empty-array errors to NoSuchElementException
(#5029)
### What changes were proposed in this PR?
`OneOnEach`, `RoundRobinDeployment`, and `RandomDeployment` each leaked
a different implementation-detail exception when `next()` was called on
an empty `available` array (`IndexOutOfBoundsException`,
`ArithmeticException`,
and `IllegalArgumentException` respectively), so callers had no single
contract to handle. Each `next()` now guards `available.isEmpty` and
throws `NoSuchElementException("no available addresses")`, the standard
Scala/Java
contract for "no element to return". For `OneOnEach`, the post-iteration
"exhausted" branch was also switched to `NoSuchElementException`, so
both empty-init and exhausted paths agree.
### Any related issues, documentation, or discussions?
Closes: #4732
### How was this PR tested?
- Updated the three pinning specs in `DeployStrategiesSpec` (previously
asserting the divergent exception types) to assert
`NoSuchElementException` for: `OneOnEach` empty-init, `OneOnEach`
exhausted-after-iteration,
`OneOnEach` cursor-preserved-across-reinit, `RoundRobinDeployment`
empty, and `RandomDeployment` empty.
- Ran `sbt scalafmtAll` no formatting changes to the modified files.
- Local `sbt testOnly` for the spec could not run due to a pre-existing
build.sbt load error (`AddMetaInfLicenseFiles not found`), unrelated to
this change; CI is expected to run the spec.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.7 in compliance with ASF
---
.../deploysemantics/deploystrategy/OneOnEach.scala | 2 +-
.../deploystrategy/RandomDeployment.scala | 3 +++
.../deploystrategy/RoundRobinDeployment.scala | 3 +++
.../deploystrategy/DeployStrategiesSpec.scala | 29 +++++++---------------
4 files changed, 16 insertions(+), 21 deletions(-)
diff --git
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala
index db7e6c834c..f35a134811 100644
---
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala
+++
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala
@@ -37,7 +37,7 @@ class OneOnEach extends DeployStrategy {
override def next(): Address = {
val i = index
if (i >= available.length) {
- throw new IndexOutOfBoundsException()
+ throw new NoSuchElementException("no available addresses")
}
index += 1
available(i)
diff --git
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala
index aebab32fca..5e3ebfa972 100644
---
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala
+++
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala
@@ -33,6 +33,9 @@ class RandomDeployment extends DeployStrategy {
}
override def next(): Address = {
+ if (available.isEmpty) {
+ throw new NoSuchElementException("no available addresses")
+ }
available(util.Random.nextInt(available.length))
}
}
diff --git
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala
index 3fee912d7d..7047297493 100644
---
a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala
+++
b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala
@@ -34,6 +34,9 @@ class RoundRobinDeployment extends DeployStrategy {
}
override def next(): Address = {
+ if (available.isEmpty) {
+ throw new NoSuchElementException("no available addresses")
+ }
val i = index
index = (index + 1) % available.length
available(i)
diff --git
a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala
b/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala
index f57ad182f2..568f58e36f 100644
---
a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala
+++
b/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala
@@ -42,17 +42,17 @@ class DeployStrategiesSpec extends AnyFlatSpec with
Matchers {
strategy.next() shouldBe nodeC
}
- it should "raise IndexOutOfBoundsException once the array is exhausted" in {
+ it should "raise NoSuchElementException once the array is exhausted" in {
val strategy = OneOnEach()
strategy.initialize(Array(nodeA))
strategy.next() shouldBe nodeA
- assertThrows[IndexOutOfBoundsException](strategy.next())
+ assertThrows[NoSuchElementException](strategy.next())
}
- it should "raise IndexOutOfBoundsException immediately when initialized with
an empty array" in {
+ it should "raise NoSuchElementException immediately when initialized with an
empty array" in {
val strategy = OneOnEach()
strategy.initialize(Array.empty[Address])
- assertThrows[IndexOutOfBoundsException](strategy.next())
+ assertThrows[NoSuchElementException](strategy.next())
}
it should "reset its iteration cursor on re-initialization" in {
@@ -62,7 +62,7 @@ class DeployStrategiesSpec extends AnyFlatSpec with Matchers {
strategy.next() shouldBe nodeB
strategy.initialize(Array(nodeC))
strategy.next() shouldBe nodeC
- assertThrows[IndexOutOfBoundsException](strategy.next())
+ assertThrows[NoSuchElementException](strategy.next())
}
"OneOnEach.apply" should "produce a fresh, independent instance" in {
@@ -89,16 +89,10 @@ class DeployStrategiesSpec extends AnyFlatSpec with
Matchers {
for (_ <- 1 to 5) strategy.next() shouldBe nodeA
}
- it should "raise ArithmeticException on next() with an empty array (current
behavior)" in {
- // Pin: RoundRobinDeployment.next does `index = (index + 1) % length`,
- // which divides by zero when length == 0 and crashes with
- // ArithmeticException before any address is returned. Other strategies
- // raise IndexOutOfBoundsException for the same situation, so this is a
- // contract divergence — pinning the current behavior so a future fix
- // that aligns the empty-array error type will need to update this spec.
+ it should "raise NoSuchElementException on next() with an empty array" in {
val strategy = RoundRobinDeployment()
strategy.initialize(Array.empty[Address])
- assertThrows[ArithmeticException](strategy.next())
+ assertThrows[NoSuchElementException](strategy.next())
}
"RoundRobinDeployment.apply" should "produce a fresh, independent instance"
in {
@@ -125,15 +119,10 @@ class DeployStrategiesSpec extends AnyFlatSpec with
Matchers {
for (_ <- 1 to 5) strategy.next() shouldBe nodeA
}
- it should "raise IllegalArgumentException on next() with an empty array
(current behavior)" in {
- // Pin: RandomDeployment.next() calls Random.nextInt(0), which throws
- // IllegalArgumentException with bound must be positive. Same root issue
- // as the empty-array case for RoundRobinDeployment: each strategy reports
- // the empty-array fault with a different exception type. Pinning this
- // separately so a unification fix shows up here.
+ it should "raise NoSuchElementException on next() with an empty array" in {
val strategy = RandomDeployment()
strategy.initialize(Array.empty[Address])
- assertThrows[IllegalArgumentException](strategy.next())
+ assertThrows[NoSuchElementException](strategy.next())
}
"RandomDeployment.apply" should "produce a fresh, independent instance" in {
