Skip to content

Commit f2d27e4

Browse files
committed
[runners-spark] Cover Scala-array fallback and EvaluationContext error paths
Address codecov/patch on PR #38255 by exercising the new branches added for Scala 2.13 / null-safe error logging: - Refactor SparkRunnerKryoRegistrator's nested Scala-array Class.forName fallback into a small @VisibleForTesting findFirstAvailableClass helper and add unit tests for first-hit, fallback, no-match, and empty-input paths. - Add EvaluationContextTest covering the catch (RuntimeException) / catch (Exception) blocks in evaluate() and collect(), including the null-message path that motivated the String.valueOf wrap.
1 parent aa4e68e commit f2d27e4

3 files changed

Lines changed: 161 additions & 12 deletions

File tree

runners/spark/src/main/java/org/apache/beam/runners/spark/coders/SparkRunnerKryoRegistrator.java

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@
2828
import org.apache.beam.sdk.transforms.windowing.PaneInfo;
2929
import org.apache.beam.sdk.values.KV;
3030
import org.apache.beam.sdk.values.TupleTag;
31+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.annotations.VisibleForTesting;
3132
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.HashBasedTable;
3233
import org.apache.spark.serializer.KryoRegistrator;
34+
import org.checkerframework.checker.nullness.qual.Nullable;
3335
import org.slf4j.Logger;
3436
import org.slf4j.LoggerFactory;
3537

@@ -65,18 +67,18 @@ public void registerClasses(Kryo kryo) {
6567
kryo.register(StateAndTimers.class);
6668
kryo.register(TupleTag.class);
6769
// Scala 2.12 uses WrappedArray$ofRef, Scala 2.13 renamed it to ArraySeq$ofRef
68-
try {
69-
kryo.register(Class.forName("scala.collection.mutable.ArraySeq$ofRef"));
70-
} catch (ClassNotFoundException e) {
71-
try {
72-
kryo.register(Class.forName("scala.collection.mutable.WrappedArray$ofRef"));
73-
} catch (ClassNotFoundException ignored) {
74-
LOG.warn(
75-
"Neither scala.collection.mutable.ArraySeq$ofRef (Scala 2.13) nor "
76-
+ "scala.collection.mutable.WrappedArray$ofRef (Scala 2.12) was found on the "
77-
+ "classpath. Kryo serialization of Scala wrapped arrays will fall back to Java "
78-
+ "serialization or fail at runtime if spark.kryo.registrationRequired is true.");
79-
}
70+
Class<?> scalaArrayClass =
71+
findFirstAvailableClass(
72+
"scala.collection.mutable.ArraySeq$ofRef",
73+
"scala.collection.mutable.WrappedArray$ofRef");
74+
if (scalaArrayClass != null) {
75+
kryo.register(scalaArrayClass);
76+
} else {
77+
LOG.warn(
78+
"Neither scala.collection.mutable.ArraySeq$ofRef (Scala 2.13) nor "
79+
+ "scala.collection.mutable.WrappedArray$ofRef (Scala 2.12) was found on the "
80+
+ "classpath. Kryo serialization of Scala wrapped arrays will fall back to Java "
81+
+ "serialization or fail at runtime if spark.kryo.registrationRequired is true.");
8082
}
8183

8284
try {
@@ -90,4 +92,16 @@ public void registerClasses(Kryo kryo) {
9092
throw new IllegalStateException("Unable to register classes with kryo.", e);
9193
}
9294
}
95+
96+
@VisibleForTesting
97+
static @Nullable Class<?> findFirstAvailableClass(String... classNames) {
98+
for (String name : classNames) {
99+
try {
100+
return Class.forName(name);
101+
} catch (ClassNotFoundException ignored) {
102+
// try the next candidate
103+
}
104+
}
105+
return null;
106+
}
93107
}

runners/spark/src/test/java/org/apache/beam/runners/spark/coders/SparkRunnerKryoRegistratorTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
*/
1818
package org.apache.beam.runners.spark.coders;
1919

20+
import static org.junit.Assert.assertEquals;
2021
import static org.junit.Assert.assertFalse;
22+
import static org.junit.Assert.assertNull;
23+
import static org.junit.Assert.assertSame;
2124
import static org.junit.Assert.assertTrue;
2225

2326
import com.esotericsoftware.kryo.Kryo;
@@ -73,6 +76,54 @@ public void testDefaultSerializerNotCallingKryo() {
7376
}
7477
}
7578

79+
/** Unit tests for the {@link SparkRunnerKryoRegistrator#findFirstAvailableClass} helper. */
80+
public static class FindFirstAvailableClassTest {
81+
82+
@Test
83+
public void returnsFirstWhenAvailable() {
84+
Class<?> result =
85+
SparkRunnerKryoRegistrator.findFirstAvailableClass(
86+
"java.lang.String", "java.lang.Integer");
87+
assertSame(String.class, result);
88+
}
89+
90+
@Test
91+
public void fallsBackWhenFirstMissing() {
92+
Class<?> result =
93+
SparkRunnerKryoRegistrator.findFirstAvailableClass(
94+
"does.not.Exist", "java.lang.Integer");
95+
assertSame(Integer.class, result);
96+
}
97+
98+
@Test
99+
public void returnsNullWhenNoneAvailable() {
100+
Class<?> result =
101+
SparkRunnerKryoRegistrator.findFirstAvailableClass(
102+
"does.not.Exist1", "does.not.Exist2");
103+
assertNull(result);
104+
}
105+
106+
@Test
107+
public void returnsNullForEmptyInput() {
108+
assertNull(SparkRunnerKryoRegistrator.findFirstAvailableClass());
109+
}
110+
111+
@Test
112+
public void resolvesScalaWrappedArrayClassOnRealClasspath() {
113+
// On any supported Scala version (2.12 ArraySeq$ofRef does not exist; 2.13 it does), at
114+
// least one of the two wrapped-array class names must resolve. This is the production call
115+
// the registrator makes.
116+
Class<?> result =
117+
SparkRunnerKryoRegistrator.findFirstAvailableClass(
118+
"scala.collection.mutable.ArraySeq$ofRef",
119+
"scala.collection.mutable.WrappedArray$ofRef");
120+
assertEquals(
121+
"expected one of the Scala wrapped-array classes to be on the classpath",
122+
true,
123+
result != null);
124+
}
125+
}
126+
76127
// Hide TestKryoRegistrator from the Enclosed JUnit runner
77128
interface Others {
78129
class TestKryoRegistrator extends SparkRunnerKryoRegistrator {
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.beam.runners.spark.structuredstreaming.translation;
19+
20+
import static org.junit.Assert.assertSame;
21+
import static org.junit.Assert.assertThrows;
22+
import static org.mockito.Mockito.doThrow;
23+
import static org.mockito.Mockito.mock;
24+
25+
import org.apache.spark.sql.Dataset;
26+
import org.junit.Test;
27+
28+
/**
29+
* Unit tests for the static error-path branches of {@link EvaluationContext}. The happy-path
30+
* branches are covered end-to-end by the structured-streaming translation tests; these tests
31+
* specifically exercise the {@code catch} blocks that wrap and rethrow underlying Spark failures.
32+
*/
33+
public class EvaluationContextTest {
34+
35+
@Test
36+
public void evaluateWrapsAndRethrowsRuntimeException() {
37+
@SuppressWarnings("unchecked")
38+
Dataset<Object> ds = mock(Dataset.class);
39+
RuntimeException underlying = new RuntimeException("boom");
40+
doThrow(underlying).when(ds).write();
41+
42+
RuntimeException thrown =
43+
assertThrows(RuntimeException.class, () -> EvaluationContext.evaluate("test-ds", ds));
44+
assertSame(underlying, thrown.getCause());
45+
}
46+
47+
@Test
48+
public void evaluateHandlesNullExceptionMessage() {
49+
// Reproduces the original NPE motivation for the String.valueOf wrap: a RuntimeException
50+
// whose root cause carries a null message must not crash the error logger.
51+
@SuppressWarnings("unchecked")
52+
Dataset<Object> ds = mock(Dataset.class);
53+
RuntimeException underlying = new RuntimeException((String) null);
54+
doThrow(underlying).when(ds).write();
55+
56+
RuntimeException thrown =
57+
assertThrows(RuntimeException.class, () -> EvaluationContext.evaluate("test-ds", ds));
58+
assertSame(underlying, thrown.getCause());
59+
}
60+
61+
@Test
62+
public void collectWrapsAndRethrowsException() {
63+
@SuppressWarnings("unchecked")
64+
Dataset<Object> ds = mock(Dataset.class);
65+
RuntimeException underlying = new RuntimeException("boom");
66+
doThrow(underlying).when(ds).collect();
67+
68+
RuntimeException thrown =
69+
assertThrows(RuntimeException.class, () -> EvaluationContext.collect("test-ds", ds));
70+
assertSame(underlying, thrown.getCause());
71+
}
72+
73+
@Test
74+
public void collectHandlesNullExceptionMessage() {
75+
@SuppressWarnings("unchecked")
76+
Dataset<Object> ds = mock(Dataset.class);
77+
RuntimeException underlying = new RuntimeException((String) null);
78+
doThrow(underlying).when(ds).collect();
79+
80+
RuntimeException thrown =
81+
assertThrows(RuntimeException.class, () -> EvaluationContext.collect("test-ds", ds));
82+
assertSame(underlying, thrown.getCause());
83+
}
84+
}

0 commit comments

Comments
 (0)