[
https://issues.apache.org/jira/browse/BEAM-14334?focusedWorklogId=769622&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769622
]
ASF GitHub Bot logged work on BEAM-14334:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 12/May/22 13:05
Start Date: 12/May/22 13:05
Worklog Time Spent: 10m
Work Description: echauchot commented on code in PR #17406:
URL: https://github.com/apache/beam/pull/17406#discussion_r871356054
##########
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:
It's way better, thanks for this fix. I know it was not part of the scope
but as we had to touch this file anyway it was better to fix the test.
Issue Time Tracking
-------------------
Worklog Id: (was: 769622)
Time Spent: 8h 40m (was: 8.5h)
> Avoid using forkEvery in Spark runner tests
> -------------------------------------------
>
> Key: BEAM-14334
> URL: https://issues.apache.org/jira/browse/BEAM-14334
> Project: Beam
> Issue Type: Improvement
> Components: runner-spark, testing
> Reporter: Moritz Mack
> Assignee: Moritz Mack
> Priority: P2
> Time Spent: 8h 40m
> Remaining Estimate: 0h
>
> Usage of *{color:#FF0000}forkEvery 1{color}* is typically a strong sign of
> poor quality / bad code and should be avoided:
> * It significantly impacts performance when running tests.
> * And it often hides resource leaks, either in code or worse in the runner
> itself.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
