mosche commented on code in PR #17406:
URL: https://github.com/apache/beam/pull/17406#discussion_r871239931
##########
runners/spark/src/test/java/org/apache/beam/runners/spark/coders/SparkRunnerKryoRegistratorTest.java:
##########
@@ -48,61 +49,45 @@ public static class WithKryoSerializer {
public static SparkContextRule contextRule =
new SparkContextRule(
KV.of("spark.serializer",
"org.apache.spark.serializer.KryoSerializer"),
- KV.of("spark.kryo.registrator",
WrapperKryoRegistrator.class.getName()));
+ KV.of("spark.kryo.registrator",
TestKryoRegistrator.class.getName()));
@Test
public void testKryoRegistration() {
+ TestKryoRegistrator.wasInitiated = false;
runSimplePipelineWithSparkContextOptions(contextRule);
- assertTrue(
- "WrapperKryoRegistrator wasn't initiated, probably KryoSerializer is
not set",
- WrapperKryoRegistrator.wasInitiated);
- }
-
- /**
- * A {@link SparkRunnerKryoRegistrator} that registers an internal class
to validate
- * KryoSerialization resolution. Use only for test purposes. Needs to be
public for
- * serialization.
- */
- public static class WrapperKryoRegistrator extends
SparkRunnerKryoRegistrator {
-
- static boolean wasInitiated = false;
-
- public WrapperKryoRegistrator() {
- wasInitiated = true;
- }
-
- @Override
- public void registerClasses(Kryo kryo) {
- super.registerClasses(kryo);
- Registration registration =
kryo.getRegistration(MicrobatchSource.class);
- com.esotericsoftware.kryo.Serializer kryoSerializer =
registration.getSerializer();
- assertTrue(kryoSerializer instanceof StatelessJavaSerializer);
- }
+ assertTrue(TestKryoRegistrator.wasInitiated);
}
}
public static class WithoutKryoSerializer {
@ClassRule
public static SparkContextRule contextRule =
- new SparkContextRule(
- KV.of("spark.kryo.registrator",
KryoRegistratorIsNotCalled.class.getName()));
+ new SparkContextRule(KV.of("spark.kryo.registrator",
TestKryoRegistrator.class.getName()));
@Test
public void testDefaultSerializerNotCallingKryo() {
+ TestKryoRegistrator.wasInitiated = false;
runSimplePipelineWithSparkContextOptions(contextRule);
+ assertFalse(TestKryoRegistrator.wasInitiated);
Review Comment:
Instead of throwing, I'm asserting it was not initiated... the intend of the
test should be much clearer now
--
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]
