From e97fc7f176f8bf501c9b3afd8410014e3b0e1602 Mon Sep 17 00:00:00 2001 From: Sean Owen Date: Thu, 3 Mar 2016 09:54:09 +0000 Subject: [SPARK-13423][WIP][CORE][SQL][STREAMING] Static analysis fixes for 2.x ## What changes were proposed in this pull request? Make some cross-cutting code improvements according to static analysis. These are individually up for discussion since they exist in separate commits that can be reverted. The changes are broadly: - Inner class should be static - Mismatched hashCode/equals - Overflow in compareTo - Unchecked warnings - Misuse of assert, vs junit.assert - get(a) + getOrElse(b) -> getOrElse(a,b) - Array/String .size -> .length (occasionally, -> .isEmpty / .nonEmpty) to avoid implicit conversions - Dead code - tailrec - exists(_ == ) -> contains find + nonEmpty -> exists filter + size -> count - reduce(_+_) -> sum map + flatten -> map The most controversial may be .size -> .length simply because of its size. It is intended to avoid implicits that might be expensive in some places. ## How was the this patch tested? Existing Jenkins unit tests. Author: Sean Owen Closes #11292 from srowen/SPARK-13423. --- .../org/apache/spark/network/ChunkFetchIntegrationSuite.java | 2 +- .../apache/spark/network/RequestTimeoutIntegrationSuite.java | 12 ++++++------ .../java/org/apache/spark/network/RpcIntegrationSuite.java | 2 +- .../apache/spark/network/TransportClientFactorySuite.java | 7 ++++--- .../spark/network/protocol/MessageWithHeaderSuite.java | 6 +++--- 5 files changed, 15 insertions(+), 14 deletions(-) (limited to 'common/network-common/src/test/java/org') diff --git a/common/network-common/src/test/java/org/apache/spark/network/ChunkFetchIntegrationSuite.java b/common/network-common/src/test/java/org/apache/spark/network/ChunkFetchIntegrationSuite.java index 70c849d60e..d17e986e17 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/ChunkFetchIntegrationSuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/ChunkFetchIntegrationSuite.java @@ -132,7 +132,7 @@ public class ChunkFetchIntegrationSuite { testFile.delete(); } - class FetchResult { + static class FetchResult { public Set successChunks; public Set failedChunks; public List buffers; diff --git a/common/network-common/src/test/java/org/apache/spark/network/RequestTimeoutIntegrationSuite.java b/common/network-common/src/test/java/org/apache/spark/network/RequestTimeoutIntegrationSuite.java index f9b5bf96d6..e2d026c66f 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/RequestTimeoutIntegrationSuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/RequestTimeoutIntegrationSuite.java @@ -124,8 +124,8 @@ public class RequestTimeoutIntegrationSuite { synchronized (callback1) { client.sendRpc(ByteBuffer.allocate(0), callback1); callback1.wait(4 * 1000); - assert (callback1.failure != null); - assert (callback1.failure instanceof IOException); + assertNotNull(callback1.failure); + assertTrue(callback1.failure instanceof IOException); } semaphore.release(); } @@ -167,8 +167,8 @@ public class RequestTimeoutIntegrationSuite { synchronized (callback0) { client0.sendRpc(ByteBuffer.allocate(0), callback0); callback0.wait(FOREVER); - assert (callback0.failure instanceof IOException); - assert (!client0.isActive()); + assertTrue(callback0.failure instanceof IOException); + assertFalse(client0.isActive()); } // Increment the semaphore and the second request should succeed quickly. @@ -236,7 +236,7 @@ public class RequestTimeoutIntegrationSuite { synchronized (callback1) { // failed at same time as previous - assert (callback0.failure instanceof IOException); + assertTrue(callback0.failure instanceof IOException); } } @@ -244,7 +244,7 @@ public class RequestTimeoutIntegrationSuite { * Callback which sets 'success' or 'failure' on completion. * Additionally notifies all waiters on this callback when invoked. */ - class TestCallback implements RpcResponseCallback, ChunkReceivedCallback { + static class TestCallback implements RpcResponseCallback, ChunkReceivedCallback { int successLength = -1; Throwable failure; diff --git a/common/network-common/src/test/java/org/apache/spark/network/RpcIntegrationSuite.java b/common/network-common/src/test/java/org/apache/spark/network/RpcIntegrationSuite.java index 9e9be98c14..a7a99f3bfc 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/RpcIntegrationSuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/RpcIntegrationSuite.java @@ -91,7 +91,7 @@ public class RpcIntegrationSuite { clientFactory.close(); } - class RpcResult { + static class RpcResult { public Set successMessages; public Set errorMessages; } diff --git a/common/network-common/src/test/java/org/apache/spark/network/TransportClientFactorySuite.java b/common/network-common/src/test/java/org/apache/spark/network/TransportClientFactorySuite.java index dac7d4a5b0..9a89dd114f 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/TransportClientFactorySuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/TransportClientFactorySuite.java @@ -27,6 +27,7 @@ import java.util.concurrent.atomic.AtomicInteger; import com.google.common.collect.Maps; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -95,7 +96,7 @@ public class TransportClientFactorySuite { try { TransportClient client = factory.createClient(TestUtils.getLocalHost(), server1.getPort()); - assert (client.isActive()); + assertTrue(client.isActive()); clients.add(client); } catch (IOException e) { failed.incrementAndGet(); @@ -115,8 +116,8 @@ public class TransportClientFactorySuite { attempts[i].join(); } - assert(failed.get() == 0); - assert(clients.size() == maxConnections); + Assert.assertEquals(0, failed.get()); + Assert.assertEquals(clients.size(), maxConnections); for (TransportClient client : clients) { client.close(); diff --git a/common/network-common/src/test/java/org/apache/spark/network/protocol/MessageWithHeaderSuite.java b/common/network-common/src/test/java/org/apache/spark/network/protocol/MessageWithHeaderSuite.java index fbbe4b7014..b341c5681e 100644 --- a/common/network-common/src/test/java/org/apache/spark/network/protocol/MessageWithHeaderSuite.java +++ b/common/network-common/src/test/java/org/apache/spark/network/protocol/MessageWithHeaderSuite.java @@ -65,7 +65,7 @@ public class MessageWithHeaderSuite { assertEquals(42, result.readLong()); assertEquals(84, result.readLong()); - assert(msg.release()); + assertTrue(msg.release()); assertEquals(0, bodyPassedToNettyManagedBuffer.refCnt()); assertEquals(0, header.refCnt()); } @@ -77,7 +77,7 @@ public class MessageWithHeaderSuite { ByteBuf body = (ByteBuf) managedBuf.convertToNetty(); assertEquals(2, body.refCnt()); MessageWithHeader msg = new MessageWithHeader(managedBuf, header, body, body.readableBytes()); - assert(msg.release()); + assertTrue(msg.release()); Mockito.verify(managedBuf, Mockito.times(1)).release(); assertEquals(0, body.refCnt()); } @@ -94,7 +94,7 @@ public class MessageWithHeaderSuite { for (long i = 0; i < 8; i++) { assertEquals(i, result.readLong()); } - assert(msg.release()); + assertTrue(msg.release()); } private ByteBuf doWrite(MessageWithHeader msg, int minExpectedWrites) throws Exception { -- cgit v1.2.3