[SPARK-19534][TESTS] Convert Java tests to use lambdas, Java 8 features by srowen · Pull Request #16964 · apache/spark

srowen · 2017-02-16T20:22:03Z

What changes were proposed in this pull request?

Convert tests to use Java 8 lambdas, and modest related fixes to surrounding code.

How was this patch tested?

Jenkins tests

…ounding code

dongjoon-hyun · 2017-02-16T21:18:07Z

+1, LGTM, too. Very tidy!

srowen · 2017-02-16T21:26:29Z

sql/core/src/test/java/test/org/apache/spark/sql/JavaDataFrameSuite.java

-    Assert.assertEquals(sketch1.totalCount(), 1000);
-    Assert.assertEquals(sketch1.depth(), 10);
-    Assert.assertEquals(sketch1.width(), 20);
+    Assert.assertEquals(1000, sketch1.totalCount());


Here and in a few tests, I fixed the switched ordering on the asserts, while I was touching the code.

srowen · 2017-02-16T21:43:51Z

common/network-common/src/test/java/org/apache/spark/network/crypto/AuthIntegrationSuite.java

      TransportServerBootstrap auth = enableAes ? new AuthServerBootstrap(conf, keyHolder)
        : new SaslServerBootstrap(conf, keyHolder);
-      this.server = ctx.createServer(Lists.newArrayList(auth, introspector));
+      this.server = ctx.createServer(Arrays.asList(auth, introspector));


I also think we can do away with things like Lists.newArrayList, which used to be a much more concise ways of making a list of things, but by Java 7, is just a non-standard version of the identical Arrays.asList

srowen · 2017-02-16T21:44:24Z

common/network-common/src/test/java/org/apache/spark/network/sasl/SparkSaslSuite.java

  @Test
  public void testFileRegionEncryption() throws Exception {
-    final Map<String, String> testConf = ImmutableMap.of(
+    Map<String, String> testConf = ImmutableMap.of(


Anyone wondering about 'final' -- these are no longer needed because lambdas are OK with variables that are effectively final in the enclosing scope.

SparkQA · 2017-02-16T22:15:30Z

Test build #73011 has finished for PR 16964 at commit 05e95fb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-02-17T00:08:29Z

Test build #73017 has finished for PR 16964 at commit 6d11cb1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

zzcclp · 2017-02-17T02:27:39Z

@srowen after update to master, in Eclipse IDE, there is an error in JavaConsumerStrategySuite.java line 52:

line 49: final Map<TopicPartition, Long> offsets = new HashMap<>();
line 50: offsets.put(tp1, 23L);
line 51: final scala.collection.Map<TopicPartition, Object> sOffsets =
line 52: JavaConverters.mapAsScalaMapConverter(offsets).asScala().mapValues(
line 53: new scala.runtime.AbstractFunction1<Long, Object>() {
line 54: @Override
line 55: public Object apply(Long x) {
line 56: return (Object) x;
line 57: }
line 58: }
line 59: );

Error is as follows:
The method mapValues(Function1<Long,Object>) is ambiguous for the type Map<TopicPartition,Long>

Is it related to updating Java 7 to 8?

srowen · 2017-02-17T08:49:23Z

That code didn't change in the update for Java 8, and the error itself doesn't sound directly like a Java language level issue. (Although you see an anonymous function here, actually, this is an instance that could not change.) I don't see a compile error locally or on any Jenkins build either. Check your local env? clean build?

srowen · 2017-02-17T10:14:29Z

...n/network-common/src/test/java/org/apache/spark/network/util/TransportFrameDecoderSuite.java

  public void testLargeFrame() throws Exception {
    // Frame length includes the frame size field, so need to add a few more bytes.
-    testInvalidFrame(Integer.MAX_VALUE + 9);
+    testInvalidFrame((long) Integer.MAX_VALUE + 9);


@vanzin I 'fixed' this test because as written it was actually testing negative input (this overflows) but that's tested above. However now this doesn't fail as expected. Is the test something I should remove actually?

To unblock you, can you add @Ignore and file a bug to fix or remove this test?

It looks like this call isn't invalid per se at the moment. There is a check later on in the code that compares a frame size to Integer.MAX_VALUE (but, as a long, so it works) and throws an exception, but that doesn't occur until later.

vanzin

I'm gonna miss all those anonymous classes and @Overrides...

vanzin · 2017-02-17T19:51:07Z

common/network-common/src/test/java/org/apache/spark/network/sasl/SparkSaslSuite.java

-            return new FileSegmentManagedBuffer(conf, file, 0, file.length());
-          }
-        });
+      when(sm.getChunk(anyLong(), anyInt())).thenAnswer(invocation -> new FileSegmentManagedBuffer(conf, file, 0, file.length()));


This line is now too long (although we don't have automatic java style checks.)

vanzin · 2017-02-17T19:55:23Z

mllib/src/test/java/org/apache/spark/ml/feature/JavaPCASuite.java

 import java.util.List;

-import scala.Tuple2;
+import org.apache.spark.mllib.linalg.DenseVector;


nit: import order

SparkQA · 2017-02-18T13:52:12Z

Test build #73103 has finished for PR 16964 at commit 72fe178.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2017-02-19T17:43:05Z

Merged to master

## What changes were proposed in this pull request? Convert tests to use Java 8 lambdas, and modest related fixes to surrounding code. ## How was this patch tested? Jenkins tests Author: Sean Owen <sowen@cloudera.com> Closes #16964 from srowen/SPARK-19534.

## What changes were proposed in this pull request? Convert tests to use Java 8 lambdas, and modest related fixes to surrounding code. ## How was this patch tested? Jenkins tests Author: Sean Owen <sowen@cloudera.com> Closes apache#16964 from srowen/SPARK-19534.

Alanthur · 2017-10-16T02:29:04Z

@zzcclp@srowen I have the same error. In JavaConsumerStrategySuite.java,error is as follows:
The method mapValues(Function1<Long,Object>) is ambiguous for the type Map<TopicPartition,Long>
I use java 8.I clean build. But the error still exist. What should I do?Any help is appreciated.

srowen · 2017-10-16T07:49:17Z

This isn't the right place to ask, and master builds fine, so you should probably use that.

Alanthur · 2017-10-16T08:04:00Z

@srowen I want to know which IDE do you use? Eclipse or IDEA?

zzcclp · 2017-11-10T08:20:06Z

@dahaian I think this is the issue of Eclipse, you can open 'JavaConsumerStrategySuite.java' file, remove the '.mapValues' and then retype '.mapValues' again, save file to recompile, it will resolve this issue. I resolved it by this way.

Alanthur · 2017-11-10T09:20:46Z

@zzcclp I tried, failed. Any suggestions?

Convert tests to use Java 8 lambdas, and modest related fixes to surr…

05e95fb

…ounding code

srowen commented Feb 16, 2017

View reviewed changes

Additional simplification: remove Guava classes and unneeded types

6d11cb1

srowen commented Feb 17, 2017

View reviewed changes

vanzin approved these changes Feb 17, 2017

View reviewed changes

Remove obsolete test and fix some style issues per review

72fe178

srowen closed this Feb 19, 2017

srowen deleted the SPARK-19534 branch February 19, 2017 17:43

Conversation

srowen commented Feb 16, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 16, 2017

Uh oh!

SparkQA commented Feb 17, 2017

Uh oh!

zzcclp commented Feb 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen commented Feb 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vanzin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 18, 2017

Uh oh!

srowen commented Feb 19, 2017

Uh oh!

Alanthur commented Oct 16, 2017

Uh oh!

srowen commented Oct 16, 2017

Uh oh!

Alanthur commented Oct 16, 2017

Uh oh!

zzcclp commented Nov 10, 2017

Uh oh!

Alanthur commented Nov 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zzcclp commented Feb 17, 2017 •

edited

Loading