This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new f7c9a2a [TEST] Improve code coverage in rss-common (#26)
f7c9a2a is described below
commit f7c9a2abcc78a29e67fe7efe094328da7b64793c
Author: Kaijie Chen <[email protected]>
AuthorDate: Tue Jul 5 16:24:23 2022 +0800
[TEST] Improve code coverage in rss-common (#26)
### What changes were proposed in this pull request?
1. Add `BufferSegmentTest` and `PartitionRangeTest`.
2. Improve `RemoteStorageInfoTest`.
### Why are the changes needed?
1. A program with high test coverage has more of its source code executed
during testing, which suggests it has a lower chance of containing undetected
bugs.
3. Demostrates how to improve test coverage with the help of Codecov.
### Does this PR introduce _any_ user-facing change?
No production code change.
### How was this patch tested?
https://app.codecov.io/gh/kaijchen/incubator-uniffle/commit/cf7f30b1ee216bd6a36ba0eea37f14e7bc0ccbcb
---
.../apache/uniffle/common/BufferSegmentTest.java | 69 +++++++++++++
.../apache/uniffle/common/PartitionRangeTest.java | 77 +++++++++++++++
.../uniffle/common/RemoteStorageInfoTest.java | 109 ++++++++++++++++-----
3 files changed, 229 insertions(+), 26 deletions(-)
diff --git
a/common/src/test/java/org/apache/uniffle/common/BufferSegmentTest.java
b/common/src/test/java/org/apache/uniffle/common/BufferSegmentTest.java
new file mode 100644
index 0000000..1e40447
--- /dev/null
+++ b/common/src/test/java/org/apache/uniffle/common/BufferSegmentTest.java
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.uniffle.common;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class BufferSegmentTest {
+
+ @Test
+ public void testEquals() {
+ BufferSegment segment1 = new BufferSegment(0, 1, 2, 3, 4, 5);
+ BufferSegment segment2 = new BufferSegment(0, 1, 2, 3, 4, 5);
+ assertEquals(segment1, segment2);
+ assertNotEquals(segment1, null);
+ assertNotEquals(segment1, new Object());
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "6, 1, 2, 3, 4, 5",
+ "0, 6, 2, 3, 4, 5",
+ "0, 1, 6, 3, 4, 5",
+ "0, 1, 2, 6, 4, 5",
+ "0, 1, 2, 3, 6, 5",
+ "0, 1, 2, 3, 4, 6",
+ })
+ public void testNotEquals(long blockId, long offset, int length, int
uncompressLength, long crc, long taskAttemptId) {
+ BufferSegment segment1 = new BufferSegment(0, 1, 2, 3, 4, 5);
+ BufferSegment segment2 = new BufferSegment(blockId, offset, length,
uncompressLength, crc, taskAttemptId);
+ assertNotEquals(segment1, segment2);
+ }
+
+ @Test
+ public void testToString() {
+ BufferSegment segment = new BufferSegment(0, 1, 2, 3, 4, 5);
+ assertEquals("BufferSegment{blockId[0], taskAttemptId[5], offset[1],
length[2], crc[4], uncompressLength[3]}",
+ segment.toString());
+ }
+
+ @Test
+ public void testGetOffset() {
+ BufferSegment segment1 = new BufferSegment(0, Integer.MAX_VALUE, 2, 3, 4,
5);
+ assertEquals(Integer.MAX_VALUE, segment1.getOffset());
+ BufferSegment segment2 = new BufferSegment(0, (long) Integer.MAX_VALUE +
1, 2, 3, 4, 5);
+ assertThrows(RuntimeException.class, segment2::getOffset);
+ }
+
+}
diff --git
a/common/src/test/java/org/apache/uniffle/common/PartitionRangeTest.java
b/common/src/test/java/org/apache/uniffle/common/PartitionRangeTest.java
new file mode 100644
index 0000000..69bf97f
--- /dev/null
+++ b/common/src/test/java/org/apache/uniffle/common/PartitionRangeTest.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.uniffle.common;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class PartitionRangeTest {
+
+ @Test
+ public void testPartitionRange() {
+ PartitionRange partitionRange = new PartitionRange(3, 4);
+ assertEquals(3, partitionRange.getStart());
+ assertEquals(4, partitionRange.getEnd());
+ assertEquals(2, partitionRange.getPartitionNum());
+ assertThrows(IllegalArgumentException.class, () -> new PartitionRange(-1,
1));
+ assertThrows(IllegalArgumentException.class, () -> new PartitionRange(2,
1));
+ assertDoesNotThrow(() -> new PartitionRange(0, 0));
+ }
+
+ @Test
+ public void testEquals() {
+ PartitionRange partitionRange1 = new PartitionRange(1, 2);
+ PartitionRange partitionRange2 = new PartitionRange(1, 2);
+ PartitionRange partitionRange3 = new PartitionRange(1, 1);
+ PartitionRange partitionRange4 = new PartitionRange(2, 2);
+ assertEquals(partitionRange1, partitionRange1);
+ assertEquals(partitionRange1, partitionRange2);
+ assertNotEquals(partitionRange1, partitionRange3);
+ assertNotEquals(partitionRange1, partitionRange4);
+ assertNotEquals(partitionRange1, null);
+ assertNotEquals(partitionRange1, new Object());
+ }
+
+ @Test
+ public void testToString() {
+ PartitionRange partitionRange = new PartitionRange(1, 2);
+ assertEquals("PartitionRange[1, 2]", partitionRange.toString());
+ }
+
+ @Test
+ public void testHashCode() {
+ PartitionRange partitionRange1 = new PartitionRange(1, 2);
+ PartitionRange partitionRange2 = new PartitionRange(1, 2);
+ assertEquals(partitionRange1.hashCode(), partitionRange2.hashCode());
+ }
+
+ @Test
+ public void testCompareTo() {
+ PartitionRange partitionRange1 = new PartitionRange(1, 2);
+ PartitionRange partitionRange2 = new PartitionRange(1, 3);
+ PartitionRange partitionRange3 = new PartitionRange(2, 3);
+ assertEquals(0, partitionRange1.compareTo(partitionRange2));
+ assertEquals(-1, partitionRange1.compareTo(partitionRange3));
+ assertEquals(1, partitionRange3.compareTo(partitionRange2));
+ }
+
+}
diff --git
a/common/src/test/java/org/apache/uniffle/common/RemoteStorageInfoTest.java
b/common/src/test/java/org/apache/uniffle/common/RemoteStorageInfoTest.java
index ce676ec..a5ba988 100644
--- a/common/src/test/java/org/apache/uniffle/common/RemoteStorageInfoTest.java
+++ b/common/src/test/java/org/apache/uniffle/common/RemoteStorageInfoTest.java
@@ -18,41 +18,98 @@
package org.apache.uniffle.common;
import com.google.common.collect.ImmutableMap;
+import org.apache.uniffle.common.util.Constants;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
+import java.util.Collections;
import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
public class RemoteStorageInfoTest {
+ private static final String TEST_PATH = "hdfs://test";
+ private static final String CONF_STRING = "k1=v1,k2=v2";
+ private static final Map<String, String> confMap = ImmutableMap.of("k1",
"v1", "k2", "v2");
+
+ private static Stream<Arguments> confItemsParams() {
+ return Stream.of(
+ arguments(null, null, String.join(Constants.COMMA_SPLIT_CHAR,
TEST_PATH, "empty conf")),
+ arguments("", Collections.emptyMap(),
String.join(Constants.COMMA_SPLIT_CHAR, TEST_PATH, "empty conf")),
+ arguments(CONF_STRING, confMap,
String.join(Constants.COMMA_SPLIT_CHAR, TEST_PATH, CONF_STRING))
+ );
+ }
+
+ @ParameterizedTest
+ @NullAndEmptySource
+ public void testEmptyStoragePath(String path) {
+ RemoteStorageInfo info = new RemoteStorageInfo(path);
+ assertTrue(info.isEmpty());
+ assertEquals(path, info.getPath());
+ assertEquals("Empty Remote Storage", info.toString());
+ }
+
+ @ParameterizedTest
+ @MethodSource("confItemsParams")
+ public void testRemoteStorageInfo(String confString, Map<String, String>
confItems, String expectedToString) {
+ RemoteStorageInfo info = new RemoteStorageInfo(TEST_PATH, confItems);
+ assertFalse(info.isEmpty());
+ assertEquals(TEST_PATH, info.getPath());
+
assertEquals(Optional.ofNullable(confItems).orElse(Collections.emptyMap()),
info.getConfItems());
+ assertEquals(Optional.ofNullable(confString).orElse(""),
info.getConfString());
+ assertEquals(expectedToString, info.toString());
+
+ info = new RemoteStorageInfo(TEST_PATH, confString);
+ assertFalse(info.isEmpty());
+ assertEquals(TEST_PATH, info.getPath());
+
assertEquals(Optional.ofNullable(confItems).orElse(Collections.emptyMap()),
info.getConfItems());
+ assertEquals(Optional.ofNullable(confString).orElse(""),
info.getConfString());
+ assertEquals(expectedToString, info.toString());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {",", "=,", ",="})
+ public void testUncommonConfString(String confString) {
+ RemoteStorageInfo info = new RemoteStorageInfo(TEST_PATH, confString);
+ assertEquals(TEST_PATH, info.getPath());
+ assertEquals(Collections.emptyMap(), info.getConfItems());
+ assertEquals("", info.getConfString());
+ }
+
+ @Test
+ public void testEquals() {
+ RemoteStorageInfo info = new RemoteStorageInfo(TEST_PATH, confMap);
+ assertEquals(info, info);
+ assertNotEquals(info, null);
+ assertNotEquals(info, new Object());
+ RemoteStorageInfo info1 = new RemoteStorageInfo(TEST_PATH, CONF_STRING);
+ assertEquals(info, info1);
+ RemoteStorageInfo info2 = new RemoteStorageInfo(TEST_PATH + "2", confMap);
+ assertNotEquals(info, info2);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"k1=v1", "k1=v1,k2=v3", "k1=v1,k3=v2"})
+ public void testNotEquals(String confString) {
+ RemoteStorageInfo info = new RemoteStorageInfo(TEST_PATH, CONF_STRING);
+ RemoteStorageInfo info2 = new RemoteStorageInfo(TEST_PATH, confString);
+ assertNotEquals(info, info2);
+ }
+
@Test
- public void test() {
- final String testPath = "hdfs://test";
- final String confString = "k1=v1,k2=v2";
- final Map<String, String> confMap = ImmutableMap.of("k1", "v1", "k2",
"v2");
- assertTrue(new RemoteStorageInfo("", "test").isEmpty());
- RemoteStorageInfo remoteStorageInfo = new RemoteStorageInfo(testPath);
- assertEquals(testPath, remoteStorageInfo.getPath());
- assertTrue(remoteStorageInfo.getConfItems().isEmpty());
- assertEquals("", remoteStorageInfo.getConfString());
-
- remoteStorageInfo =
- new RemoteStorageInfo(testPath, confMap);
- assertEquals(2, remoteStorageInfo.getConfItems().size());
- assertEquals(testPath, remoteStorageInfo.getPath());
- assertEquals(confString, remoteStorageInfo.getConfString());
- assertEquals("v1", remoteStorageInfo.getConfItems().get("k1"));
- assertEquals("v2", remoteStorageInfo.getConfItems().get("k2"));
-
- RemoteStorageInfo remoteStorageInfo1 = new RemoteStorageInfo(testPath,
confString);
- assertEquals(remoteStorageInfo1, remoteStorageInfo);
- RemoteStorageInfo remoteStorageInfo2 =
- new RemoteStorageInfo(testPath, ImmutableMap.of("k1", "v11"));
- assertNotEquals(remoteStorageInfo1, remoteStorageInfo2);
- RemoteStorageInfo remoteStorageInfo3 =
- new RemoteStorageInfo(testPath + "3", confMap);
- assertNotEquals(remoteStorageInfo1, remoteStorageInfo3);
+ public void testHashCode() {
+ RemoteStorageInfo info = new RemoteStorageInfo(TEST_PATH, confMap);
+ RemoteStorageInfo info1 = new RemoteStorageInfo(TEST_PATH, CONF_STRING);
+ assertEquals(info.hashCode(), info1.hashCode());
}
}
