This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch branch-0.4
in repository https://gitbox.apache.org/repos/asf/incubator-celeborn.git
The following commit(s) were added to refs/heads/branch-0.4 by this push:
new f39179fd7 [CELEBORN-1188][TEST] Using JUnit function instead of java
assert
f39179fd7 is described below
commit f39179fd7e62bc0448ef677405b259e1b9e459f0
Author: zwangsheng <[email protected]>
AuthorDate: Wed Dec 20 21:20:38 2023 +0800
[CELEBORN-1188][TEST] Using JUnit function instead of java assert
### What changes were proposed in this pull request?
Using Junit function instead of java assert.
### Why are the changes needed?
When java assert fail, will throw AssertException, which is hard to find
diff.
Instead, when we use junit assert, we can clearly find diff.
### Does this PR introduce _any_ user-facing change?
NO, only test changed
### How was this patch tested?
Run CI
Closes #2173 from zwangsheng/CELEBORN-1188.
Authored-by: zwangsheng <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit 6c2fdf7477279fccc2d0e571970fa80881340662)
Signed-off-by: Cheng Pan <[email protected]>
---
.../celeborn/client/ShuffleClientSuiteJ.java | 13 +++---
.../server/TransportRequestHandlerSuiteJ.java | 7 ++--
.../common/protocol/PartitionLocationSuiteJ.java | 16 +++----
.../deploy/master/SlotsAllocatorSuiteJ.java | 39 ++++++++---------
.../clustermeta/DefaultMetaSystemSuiteJ.java | 49 +++++++++++-----------
.../deploy/worker/WorkingPartitionSuiteJ.java | 17 ++++----
6 files changed, 73 insertions(+), 68 deletions(-)
diff --git
a/client/src/test/java/org/apache/celeborn/client/ShuffleClientSuiteJ.java
b/client/src/test/java/org/apache/celeborn/client/ShuffleClientSuiteJ.java
index 9901855a8..c4f161b6c 100644
--- a/client/src/test/java/org/apache/celeborn/client/ShuffleClientSuiteJ.java
+++ b/client/src/test/java/org/apache/celeborn/client/ShuffleClientSuiteJ.java
@@ -17,6 +17,7 @@
package org.apache.celeborn.client;
+import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;
@@ -106,12 +107,12 @@ public class ShuffleClientSuiteJ {
1);
if (codec.equals(CompressionCodec.NONE)) {
- assert (pushDataLen == TEST_BUF1.length + BATCH_HEADER_SIZE);
+ assertEquals(TEST_BUF1.length + BATCH_HEADER_SIZE, pushDataLen);
} else {
Compressor compressor = Compressor.getCompressor(conf);
compressor.compress(TEST_BUF1, 0, TEST_BUF1.length);
final int compressedTotalSize = compressor.getCompressedTotalSize();
- assert (pushDataLen == compressedTotalSize + BATCH_HEADER_SIZE);
+ assertEquals(compressedTotalSize + BATCH_HEADER_SIZE, pushDataLen);
}
}
}
@@ -134,12 +135,12 @@ public class ShuffleClientSuiteJ {
1);
if (codec.equals(CompressionCodec.NONE)) {
- assert (mergeSize == TEST_BUF1.length + BATCH_HEADER_SIZE);
+ assertEquals(TEST_BUF1.length + BATCH_HEADER_SIZE, mergeSize);
} else {
Compressor compressor = Compressor.getCompressor(conf);
compressor.compress(TEST_BUF1, 0, TEST_BUF1.length);
final int compressedTotalSize = compressor.getCompressedTotalSize();
- assert (mergeSize == compressedTotalSize + BATCH_HEADER_SIZE);
+ assertEquals(compressedTotalSize + BATCH_HEADER_SIZE, mergeSize);
}
byte[] buf1k =
RandomStringUtils.random(4000).getBytes(StandardCharsets.UTF_8);
@@ -156,12 +157,12 @@ public class ShuffleClientSuiteJ {
1);
if (codec.equals(CompressionCodec.NONE)) {
- assert (largeMergeSize == buf1k.length + BATCH_HEADER_SIZE);
+ assertEquals(buf1k.length + BATCH_HEADER_SIZE, largeMergeSize);
} else {
Compressor compressor = Compressor.getCompressor(conf);
compressor.compress(buf1k, 0, buf1k.length);
final int compressedTotalSize = compressor.getCompressedTotalSize();
- assert (largeMergeSize == compressedTotalSize + BATCH_HEADER_SIZE);
+ assertEquals(compressedTotalSize + BATCH_HEADER_SIZE, largeMergeSize);
}
}
}
diff --git
a/common/src/test/java/org/apache/celeborn/common/network/server/TransportRequestHandlerSuiteJ.java
b/common/src/test/java/org/apache/celeborn/common/network/server/TransportRequestHandlerSuiteJ.java
index e1baafd2e..e54e75e67 100644
---
a/common/src/test/java/org/apache/celeborn/common/network/server/TransportRequestHandlerSuiteJ.java
+++
b/common/src/test/java/org/apache/celeborn/common/network/server/TransportRequestHandlerSuiteJ.java
@@ -17,6 +17,7 @@
package org.apache.celeborn.common.network.server;
+import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.*;
import io.netty.buffer.ByteBuf;
@@ -57,7 +58,7 @@ public class TransportRequestHandlerSuiteJ {
requestHandler.handle(rpcRequest);
verify(msgHandler).receive(eq(reverseClient), eq(rpcRequest), any());
verify(msgHandler, times(0)).receive(eq(reverseClient), eq(rpcRequest));
- assert buffer.refCnt() == 0;
+ assertEquals(0, buffer.refCnt());
}
@Test
@@ -67,7 +68,7 @@ public class TransportRequestHandlerSuiteJ {
requestHandler.handle(oneWayMessage);
verify(msgHandler).receive(eq(reverseClient), eq(oneWayMessage));
verify(msgHandler, times(0)).receive(eq(reverseClient), eq(oneWayMessage),
any());
- assert buffer.refCnt() == 0;
+ assertEquals(0, buffer.refCnt());
}
@Test
@@ -78,6 +79,6 @@ public class TransportRequestHandlerSuiteJ {
requestHandler.handle(pushData);
verify(msgHandler).receive(eq(reverseClient), eq(pushData));
verify(msgHandler, times(0)).receive(eq(reverseClient), eq(pushData),
any());
- assert buffer.refCnt() == 0;
+ assertEquals(0, buffer.refCnt());
}
}
diff --git
a/common/src/test/java/org/apache/celeborn/common/protocol/PartitionLocationSuiteJ.java
b/common/src/test/java/org/apache/celeborn/common/protocol/PartitionLocationSuiteJ.java
index e4200c913..6f63e4870 100644
---
a/common/src/test/java/org/apache/celeborn/common/protocol/PartitionLocationSuiteJ.java
+++
b/common/src/test/java/org/apache/celeborn/common/protocol/PartitionLocationSuiteJ.java
@@ -17,6 +17,8 @@
package org.apache.celeborn.common.protocol;
+import static org.junit.Assert.assertEquals;
+
import org.junit.Test;
import org.roaringbitmap.RoaringBitmap;
@@ -46,13 +48,13 @@ public class PartitionLocationSuiteJ {
byte primaryMode = 0;
byte replicaMode = 1;
- assert PartitionLocation.getMode(primaryMode) ==
PartitionLocation.Mode.PRIMARY;
- assert PartitionLocation.getMode(replicaMode) ==
PartitionLocation.Mode.REPLICA;
+ assertEquals(PartitionLocation.Mode.PRIMARY,
PartitionLocation.getMode(primaryMode));
+ assertEquals(PartitionLocation.Mode.REPLICA,
PartitionLocation.getMode(replicaMode));
for (int i = 2; i < 255; ++i) {
byte otherMode = (byte) i;
// Should we return replica mode when the parameter passed in is neither
0 nor 1?
- assert PartitionLocation.getMode(otherMode) ==
PartitionLocation.Mode.REPLICA;
+ assertEquals(PartitionLocation.Mode.REPLICA,
PartitionLocation.getMode(otherMode));
}
}
@@ -225,9 +227,9 @@ public class PartitionLocationSuiteJ {
+ "
peer:(host-rpcPort-pushPort-fetchPort-replicatePort:localhost-3-1-2-4)\n"
+ " storage hint:StorageInfo{type=MEMORY,
mountPoint='/mnt/disk/0', finalResult=false, filePath=null}\n"
+ " mapIdBitMap:{1,2,3}]";
- assert exp1.equals(location1.toString());
- assert exp2.equals(location2.toString());
- assert exp3.equals(location3.toString());
+ assertEquals(exp1, location1.toString());
+ assertEquals(exp2, location2.toString());
+ assertEquals(exp3, location3.toString());
}
private void checkEqual(
@@ -242,6 +244,6 @@ public class PartitionLocationSuiteJ {
+ "equal, but "
+ (shouldEqual ? "not " : "")
+ "equal.";
- assert location1.equals(location2) == shouldEqual : errorMessage;
+ assertEquals(errorMessage, shouldEqual, location1.equals(location2));
}
}
diff --git
a/master/src/test/java/org/apache/celeborn/service/deploy/master/SlotsAllocatorSuiteJ.java
b/master/src/test/java/org/apache/celeborn/service/deploy/master/SlotsAllocatorSuiteJ.java
index fd724cd70..9ca86fe77 100644
---
a/master/src/test/java/org/apache/celeborn/service/deploy/master/SlotsAllocatorSuiteJ.java
+++
b/master/src/test/java/org/apache/celeborn/service/deploy/master/SlotsAllocatorSuiteJ.java
@@ -17,12 +17,15 @@
package org.apache.celeborn.service.deploy.master;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
import java.util.*;
import scala.Option;
import scala.Tuple2;
-import org.junit.Assert;
import org.junit.Test;
import org.apache.celeborn.common.CelebornConf;
@@ -244,13 +247,14 @@ public class SlotsAllocatorSuiteJ {
v._1.forEach(
i -> {
String uniqueId = i.getUniqueId();
- assert !locationDuplicationSet.contains(uniqueId);
+ assertFalse(locationDuplicationSet.contains(uniqueId));
locationDuplicationSet.add(uniqueId);
for (PartitionLocation location : v._1) {
- assert location.getHost().equals(k.host())
- && location.getRpcPort() == k.rpcPort()
- && location.getPushPort() == k.pushPort()
- && location.getFetchPort() == k.fetchPort();
+ assertTrue(
+ location.getHost().equals(k.host())
+ && location.getRpcPort() == k.rpcPort()
+ && location.getPushPort() == k.pushPort()
+ && location.getFetchPort() == k.fetchPort());
}
});
});
@@ -271,14 +275,14 @@ public class SlotsAllocatorSuiteJ {
allocateToDiskSlots += worker.usedSlots();
}
if (shouldReplicate) {
- Assert.assertTrue(partitionIds.size() * 2 >= unknownDiskSlots +
allocateToDiskSlots);
+ assertTrue(partitionIds.size() * 2 >= unknownDiskSlots +
allocateToDiskSlots);
} else {
- Assert.assertTrue(partitionIds.size() >= unknownDiskSlots +
allocateToDiskSlots);
+ assertTrue(partitionIds.size() >= unknownDiskSlots +
allocateToDiskSlots);
}
- Assert.assertEquals(0, unknownDiskSlots);
+ assertEquals(0, unknownDiskSlots);
} else {
- assert slots.isEmpty()
- : "Expect to fail to offer slots, but return " + slots.size() + "
slots.";
+ assertTrue(
+ "Expect to fail to offer slots, but return " + slots.size() + "
slots.", slots.isEmpty());
}
}
@@ -322,16 +326,9 @@ public class SlotsAllocatorSuiteJ {
allocatedPartitionCount += primaryLocs.size();
allocatedPartitionCount += replicaLocs.size();
}
- if (expectSuccess) {
- Assert.assertEquals(slots.isEmpty(), false);
- } else {
- Assert.assertEquals(slots.isEmpty(), true);
- }
- if (shouldReplicate) {
- Assert.assertEquals(allocatedPartitionCount, partitionIds.size() * 2);
- } else {
- Assert.assertEquals(allocatedPartitionCount, partitionIds.size());
- }
+ assertEquals(expectSuccess, !slots.isEmpty());
+ assertEquals(
+ shouldReplicate ? partitionIds.size() * 2 : partitionIds.size(),
allocatedPartitionCount);
}
@Test
diff --git
a/master/src/test/java/org/apache/celeborn/service/deploy/master/clustermeta/DefaultMetaSystemSuiteJ.java
b/master/src/test/java/org/apache/celeborn/service/deploy/master/clustermeta/DefaultMetaSystemSuiteJ.java
index c8276f4d8..83f5f7232 100644
---
a/master/src/test/java/org/apache/celeborn/service/deploy/master/clustermeta/DefaultMetaSystemSuiteJ.java
+++
b/master/src/test/java/org/apache/celeborn/service/deploy/master/clustermeta/DefaultMetaSystemSuiteJ.java
@@ -17,6 +17,8 @@
package org.apache.celeborn.service.deploy.master.clustermeta;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -25,7 +27,6 @@ import java.util.*;
import java.util.concurrent.atomic.AtomicLong;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -139,7 +140,7 @@ public class DefaultMetaSystemSuiteJ {
userResourceConsumption3,
getNewReqeustId());
- assert (statusSystem.workers.size() == 3);
+ assertEquals(3, statusSystem.workers.size());
}
@Test
@@ -184,11 +185,11 @@ public class DefaultMetaSystemSuiteJ {
statusSystem.handleWorkerExclude(
Arrays.asList(workerInfo1, workerInfo2), Collections.emptyList(),
getNewReqeustId());
- assert (statusSystem.manuallyExcludedWorkers.size() == 2);
+ assertEquals(2, statusSystem.manuallyExcludedWorkers.size());
statusSystem.handleWorkerExclude(
Collections.emptyList(), Collections.singletonList(workerInfo1),
getNewReqeustId());
- assert (statusSystem.manuallyExcludedWorkers.size() == 1);
+ assertEquals(1, statusSystem.manuallyExcludedWorkers.size());
}
@Test
@@ -223,7 +224,7 @@ public class DefaultMetaSystemSuiteJ {
statusSystem.handleWorkerLost(
HOSTNAME1, RPCPORT1, PUSHPORT1, FETCHPORT1, REPLICATEPORT1,
getNewReqeustId());
- assert (statusSystem.workers.size() == 2);
+ assertEquals(2, statusSystem.workers.size());
}
private static final String APPID1 = "appId1";
@@ -296,9 +297,9 @@ public class DefaultMetaSystemSuiteJ {
statusSystem.handleRequestSlots(SHUFFLEKEY1, HOSTNAME1, workersToAllocate,
getNewReqeustId());
- assert (workerInfo1.usedSlots() == 0);
- assert (workerInfo2.usedSlots() == 0);
- assert (workerInfo3.usedSlots() == 0);
+ assertEquals(0, workerInfo1.usedSlots());
+ assertEquals(0, workerInfo2.usedSlots());
+ assertEquals(0, workerInfo3.usedSlots());
}
@Test
@@ -331,7 +332,7 @@ public class DefaultMetaSystemSuiteJ {
userResourceConsumption3,
getNewReqeustId());
- assert 3 == statusSystem.workers.size();
+ assertEquals(3, statusSystem.workers.size());
Map<String, Map<String, Integer>> workersToAllocate = new HashMap<>();
Map<String, Integer> allocation = new HashMap<>();
@@ -352,7 +353,7 @@ public class DefaultMetaSystemSuiteJ {
allocation);
statusSystem.handleRequestSlots(SHUFFLEKEY1, HOSTNAME1, workersToAllocate,
getNewReqeustId());
- Assert.assertEquals(
+ assertEquals(
0,
statusSystem.workers.stream()
.filter(w -> w.host().equals(HOSTNAME1))
@@ -418,11 +419,11 @@ public class DefaultMetaSystemSuiteJ {
statusSystem.handleRequestSlots(SHUFFLEKEY1, HOSTNAME1, workersToAllocate,
getNewReqeustId());
- assert statusSystem.registeredShuffle.size() == 1;
+ assertEquals(1, statusSystem.registeredShuffle.size());
statusSystem.handleAppLost(APPID1, getNewReqeustId());
- assert statusSystem.registeredShuffle.isEmpty();
+ assertTrue(statusSystem.registeredShuffle.isEmpty());
}
@Test
@@ -482,24 +483,24 @@ public class DefaultMetaSystemSuiteJ {
statusSystem.handleRequestSlots(SHUFFLEKEY1, HOSTNAME1, workersToAllocate,
getNewReqeustId());
- assert statusSystem.registeredShuffle.size() == 1;
+ assertEquals(1, statusSystem.registeredShuffle.size());
statusSystem.handleUnRegisterShuffle(SHUFFLEKEY1, getNewReqeustId());
- assert statusSystem.registeredShuffle.isEmpty();
+ assertTrue(statusSystem.registeredShuffle.isEmpty());
}
@Test
public void testHandleAppHeartbeat() {
- long dummy = 1235L;
+ Long dummy = 1235L;
statusSystem.handleAppHeartbeat(APPID1, 1, 1, dummy, getNewReqeustId());
- assert statusSystem.appHeartbeatTime.get(APPID1) == dummy;
+ assertEquals(dummy, statusSystem.appHeartbeatTime.get(APPID1));
String appId2 = "app02";
statusSystem.handleAppHeartbeat(appId2, 1, 1, dummy, getNewReqeustId());
- assert statusSystem.appHeartbeatTime.get(appId2) == dummy;
+ assertEquals(dummy, statusSystem.appHeartbeatTime.get(appId2));
- assert statusSystem.appHeartbeatTime.size() == 2;
+ assertEquals(2, statusSystem.appHeartbeatTime.size());
}
@Test
@@ -545,7 +546,7 @@ public class DefaultMetaSystemSuiteJ {
false,
getNewReqeustId());
- Assert.assertEquals(statusSystem.excludedWorkers.size(), 1);
+ assertEquals(statusSystem.excludedWorkers.size(), 1);
statusSystem.handleWorkerHeartbeat(
HOSTNAME2,
@@ -560,7 +561,7 @@ public class DefaultMetaSystemSuiteJ {
false,
getNewReqeustId());
- Assert.assertEquals(statusSystem.excludedWorkers.size(), 2);
+ assertEquals(statusSystem.excludedWorkers.size(), 2);
statusSystem.handleWorkerHeartbeat(
HOSTNAME3,
@@ -575,7 +576,7 @@ public class DefaultMetaSystemSuiteJ {
false,
getNewReqeustId());
- Assert.assertEquals(statusSystem.excludedWorkers.size(), 2);
+ assertEquals(statusSystem.excludedWorkers.size(), 2);
statusSystem.handleWorkerHeartbeat(
HOSTNAME3,
@@ -590,7 +591,7 @@ public class DefaultMetaSystemSuiteJ {
true,
getNewReqeustId());
- Assert.assertEquals(statusSystem.excludedWorkers.size(), 3);
+ assertEquals(statusSystem.excludedWorkers.size(), 3);
}
@Test
@@ -635,8 +636,8 @@ public class DefaultMetaSystemSuiteJ {
userResourceConsumption1));
statusSystem.handleReportWorkerUnavailable(failedWorkers,
getNewReqeustId());
- assert 1 == statusSystem.shutdownWorkers.size();
- assert statusSystem.excludedWorkers.isEmpty();
+ assertEquals(1, statusSystem.shutdownWorkers.size());
+ assertTrue(statusSystem.excludedWorkers.isEmpty());
}
@Test
diff --git
a/worker/src/test/java/org/apache/celeborn/service/deploy/worker/WorkingPartitionSuiteJ.java
b/worker/src/test/java/org/apache/celeborn/service/deploy/worker/WorkingPartitionSuiteJ.java
index cd2c80208..8718abbbd 100644
---
a/worker/src/test/java/org/apache/celeborn/service/deploy/worker/WorkingPartitionSuiteJ.java
+++
b/worker/src/test/java/org/apache/celeborn/service/deploy/worker/WorkingPartitionSuiteJ.java
@@ -17,6 +17,9 @@
package org.apache.celeborn.service.deploy.worker;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -40,11 +43,11 @@ public class WorkingPartitionSuiteJ {
WorkingPartition pd2 = new WorkingPartition(p2, null);
list.add(pd1);
list.add(pd2);
- assert list.size() == 2;
+ assertEquals(2, list.size());
list.remove(p1);
- assert list.size() == 1;
+ assertEquals(1, list.size());
list.remove(p2);
- assert list.size() == 0;
+ assertEquals(0, list.size());
Map<PartitionLocation, PartitionLocation> map = new HashMap<>();
map.put(pd1, pd1);
@@ -52,12 +55,12 @@ public class WorkingPartitionSuiteJ {
PartitionLocation p =
new PartitionLocation(0, 0, "host1", 10, 9, 8, 11,
PartitionLocation.Mode.REPLICA);
- assert map.containsKey(p);
+ assertTrue(map.containsKey(p));
map.remove(p1);
- assert map.size() == 1;
+ assertEquals(1, map.size());
map.put(p2, p2);
- assert map.size() == 1;
+ assertEquals(1, map.size());
Map<PartitionLocation, PartitionLocation> map2 = new HashMap<>();
PartitionLocation p3 =
@@ -69,6 +72,6 @@ public class WorkingPartitionSuiteJ {
PartitionLocation p4 =
new PartitionLocation(
2, 1, "30.225.12.48", 9096, 9097, 9098, 9099,
PartitionLocation.Mode.REPLICA);
- assert map2.containsKey(p4);
+ assertTrue(map2.containsKey(p4));
}
}
