This is an automated email from the ASF dual-hosted git repository.
ckj 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 b8d424d0 [MINOR] test: fix assertion in tests (#574)
b8d424d0 is described below
commit b8d424d05562648c6e1e789e4614584cbad01a99
Author: Kaijie Chen <[email protected]>
AuthorDate: Thu Feb 9 23:01:37 2023 +0800
[MINOR] test: fix assertion in tests (#574)
### What changes were proposed in this pull request?
Fix misuse of `assert` in tests.
Fix typo in test name of `EventFetcherTest`.
### Why are the changes needed?
Improve code quality.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI.
---
...{EvenFetcherTest.java => EventFetcherTest.java} | 40 +++++++---------------
.../hadoop/mapreduce/task/reduce/FetcherTest.java | 23 +++++--------
2 files changed, 21 insertions(+), 42 deletions(-)
diff --git
a/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EvenFetcherTest.java
b/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EventFetcherTest.java
similarity index 93%
rename from
client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EvenFetcherTest.java
rename to
client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EventFetcherTest.java
index d2a03f39..d3de8b0a 100644
---
a/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EvenFetcherTest.java
+++
b/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/EventFetcherTest.java
@@ -33,13 +33,14 @@ import org.apache.hadoop.mapreduce.TaskType;
import org.junit.jupiter.api.Test;
import org.roaringbitmap.longlong.Roaring64NavigableMap;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-public class EvenFetcherTest {
+public class EventFetcherTest {
private static final int MAX_EVENTS_TO_FETCH = 100;
@Test
@@ -63,7 +64,7 @@ public class EvenFetcherTest {
}
Roaring64NavigableMap taskIdBitmap = ef.fetchAllRssTaskIds();
- validate(expected, taskIdBitmap);
+ assertEquals(expected, taskIdBitmap);
}
@Test
@@ -90,7 +91,7 @@ public class EvenFetcherTest {
}
Roaring64NavigableMap taskIdBitmap = ef.fetchAllRssTaskIds();
- validate(expected, taskIdBitmap);
+ assertEquals(expected, taskIdBitmap);
}
@Test
@@ -123,7 +124,7 @@ public class EvenFetcherTest {
expected.addLong(rssTaskId);
}
Roaring64NavigableMap taskIdBitmap = ef.fetchAllRssTaskIds();
- validate(expected, taskIdBitmap);
+ assertEquals(expected, taskIdBitmap);
}
@Test
@@ -147,13 +148,9 @@ public class EvenFetcherTest {
1);
expected.addLong(rssTaskId);
}
- try {
- ef.fetchAllRssTaskIds();
- fail();
- } catch (Exception e) {
- assert (e.getMessage()
- .contains("TaskAttemptIDs are inconsistent with map tasks"));
- }
+ IllegalStateException ex =
+ assertThrows(IllegalStateException.class, () ->
ef.fetchAllRssTaskIds());
+ assertEquals("TaskAttemptIDs are inconsistent with map tasks",
ex.getMessage());
}
@Test
@@ -177,13 +174,9 @@ public class EvenFetcherTest {
1);
expected.addLong(rssTaskId);
}
- try {
- ef.fetchAllRssTaskIds();
- fail();
- } catch (Exception e) {
- assert (e.getMessage()
- .contains("TaskAttemptIDs are inconsistent with map tasks"));
- }
+ IllegalStateException ex =
+ assertThrows(IllegalStateException.class, () ->
ef.fetchAllRssTaskIds());
+ assertEquals("TaskAttemptIDs are inconsistent with map tasks",
ex.getMessage());
}
@Test
@@ -222,14 +215,7 @@ public class EvenFetcherTest {
}
Roaring64NavigableMap taskIdBitmap = ef.fetchAllRssTaskIds();
- validate(expected, taskIdBitmap);
- }
-
- private void validate(Roaring64NavigableMap expected, Roaring64NavigableMap
actual) {
- assert (expected.getLongCardinality() == actual.getLongCardinality());
- actual.forEach(taskId -> {
- assert (expected.contains(taskId));
- });
+ assertEquals(expected, taskIdBitmap);
}
private MapTaskCompletionEventsUpdate getMockedCompletionEventsUpdate(
diff --git
a/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/FetcherTest.java
b/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/FetcherTest.java
index 7c7ce990..54f24672 100644
---
a/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/FetcherTest.java
+++
b/client-mr/src/test/java/org/apache/hadoop/mapreduce/task/reduce/FetcherTest.java
@@ -121,8 +121,8 @@ public class FetcherTest {
allKeys.add(new Text(key).toString().trim());
allValues.add(new Text(value).toString().trim());
}
- validate(allKeysExpected, allKeys);
- validate(allValuesExpected, allValues);
+ assertEquals(allKeysExpected, allKeys);
+ assertEquals(allValuesExpected, allValues);
}
@Test
@@ -152,8 +152,8 @@ public class FetcherTest {
allKeys.add(new Text(key).toString().trim());
allValues.add(new Text(value).toString().trim());
}
- validate(allKeysExpected, allKeys);
- validate(allValuesExpected, allValues);
+ assertEquals(allKeysExpected, allKeys);
+ assertEquals(allValuesExpected, allValues);
}
@Test
@@ -184,11 +184,11 @@ public class FetcherTest {
allKeys.add(new Text(key).toString().trim());
allValues.add(new Text(value).toString().trim());
}
- validate(allKeysExpected, allKeys);
- validate(allValuesExpected, allValues);
+ assertEquals(allKeysExpected, allKeys);
+ assertEquals(allValuesExpected, allValues);
// There will be 2 retries
- assert (fetcher.getRetryCount() == 2);
- assert (((MockMergeManagerImpl)merger).happenedFails.size() == 2);
+ assertEquals(2, fetcher.getRetryCount());
+ assertEquals(2, ((MockMergeManagerImpl)merger).happenedFails.size());
}
public void testCodecIsDuplicated() throws Exception {
@@ -208,13 +208,6 @@ public class FetcherTest {
(InMemoryMapOutput) mapOutput1),
RssBypassWriter.getDecompressor((InMemoryMapOutput) mapOutput2));
}
- private void validate(List<String> expected, List<String> actual) {
- assert (expected.size() == actual.size());
- for (int i = 0; i < expected.size(); i++) {
- assert (expected.get(i).equals(actual.get(i)));
- }
- }
-
private static void initRssData() throws Exception {
data = new LinkedList<>();
Map<String, String> map1 = new TreeMap<>();
