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 47f5c579 Cleanup UnitConverter and improve UnitConverterTest (#504)
47f5c579 is described below
commit 47f5c57922d12d4326a74f20e995cdeec6867801
Author: Kaijie Chen <[email protected]>
AuthorDate: Fri Jan 20 00:02:35 2023 +0800
Cleanup UnitConverter and improve UnitConverterTest (#504)
### What changes were proposed in this pull request?
1. Remove unused code in UnitConverter.
2. Make UnitConverter final, and constructor private.
3. Improve coverage in UnitConverterTest.
### Why are the changes needed?
Code quality and test coverage.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing CI.
---
.../apache/uniffle/common/util/UnitConverter.java | 62 +---------
.../uniffle/common/util/UnitConverterTest.java | 129 +++++++++++++++------
2 files changed, 100 insertions(+), 91 deletions(-)
diff --git
a/common/src/main/java/org/apache/uniffle/common/util/UnitConverter.java
b/common/src/main/java/org/apache/uniffle/common/util/UnitConverter.java
index 77f6e0eb..b4ff4921 100644
--- a/common/src/main/java/org/apache/uniffle/common/util/UnitConverter.java
+++ b/common/src/main/java/org/apache/uniffle/common/util/UnitConverter.java
@@ -26,7 +26,10 @@ import java.util.regex.Pattern;
import com.google.common.collect.ImmutableMap;
// copy from org.apache.spark.network.util.JavaUtils
-public class UnitConverter {
+public final class UnitConverter {
+
+ private UnitConverter() {
+ }
private static final Map<String, ByteUnit> byteSuffixes =
ImmutableMap.<String, ByteUnit>builder()
@@ -64,16 +67,6 @@ public class UnitConverter {
return false;
}
- public static boolean isTimeString(String str) {
- String strLower = str.toLowerCase();
- for (String key: timeSuffixes.keySet()) {
- if (strLower.contains(key)) {
- return true;
- }
- }
- return false;
- }
-
/**
* Convert a passed byte string (e.g. 50b, 100kb, or 250mb) to the given. If
no suffix is
* provided, a direct conversion to the provided unit is attempted.
@@ -120,36 +113,6 @@ public class UnitConverter {
return byteStringAs(str, ByteUnit.BYTE);
}
- /**
- * Convert a passed byte string (e.g. 50b, 100k, or 250m) to kibibytes for
- * internal use.
- *
- * If no suffix is provided, the passed number is assumed to be in kibibytes.
- */
- public static long byteStringAsKb(String str) {
- return byteStringAs(str, ByteUnit.KiB);
- }
-
- /**
- * Convert a passed byte string (e.g. 50b, 100k, or 250m) to mebibytes for
- * internal use.
- *
- * If no suffix is provided, the passed number is assumed to be in mebibytes.
- */
- public static long byteStringAsMb(String str) {
- return byteStringAs(str, ByteUnit.MiB);
- }
-
- /**
- * Convert a passed byte string (e.g. 50b, 100k, or 250m) to gibibytes for
- * internal use.
- *
- * If no suffix is provided, the passed number is assumed to be in gibibytes.
- */
- public static long byteStringAsGb(String str) {
- return byteStringAs(str, ByteUnit.GiB);
- }
-
/**
* Convert a passed time string (e.g. 50s, 100ms, or 250us) to a time count
in the given unit.
* The unit is also considered the default if the given string does not
specify a unit.
@@ -185,21 +148,4 @@ public class UnitConverter {
throw new NumberFormatException(timeError + "\n" + e.getMessage());
}
}
-
- /**
- * Convert a time parameter such as (50s, 100ms, or 250us) to milliseconds
for internal use. If
- * no suffix is provided, the passed number is assumed to be in ms.
- */
- public static long timeStringAsMs(String str) {
- return timeStringAs(str, TimeUnit.MILLISECONDS);
- }
-
- /**
- * Convert a time parameter such as (50s, 100ms, or 250us) to seconds for
internal use. If
- * no suffix is provided, the passed number is assumed to be in seconds.
- */
- public static long timeStringAsSec(String str) {
- return timeStringAs(str, TimeUnit.SECONDS);
- }
-
}
diff --git
a/common/src/test/java/org/apache/uniffle/common/util/UnitConverterTest.java
b/common/src/test/java/org/apache/uniffle/common/util/UnitConverterTest.java
index 0f395601..f8e91101 100644
--- a/common/src/test/java/org/apache/uniffle/common/util/UnitConverterTest.java
+++ b/common/src/test/java/org/apache/uniffle/common/util/UnitConverterTest.java
@@ -17,9 +17,15 @@
package org.apache.uniffle.common.util;
-import org.junit.jupiter.api.Test;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class UnitConverterTest {
@@ -29,37 +35,94 @@ public class UnitConverterTest {
private static final long MB = (long)ByteUnit.MiB.toBytes(1L);
private static final long KB = (long)ByteUnit.KiB.toBytes(1L);
- @Test
- public void testByteString() {
-
- assertEquals(10 * PB, UnitConverter.byteStringAs("10PB", ByteUnit.BYTE));
- assertEquals(10 * PB, UnitConverter.byteStringAs("10pb", ByteUnit.BYTE));
- assertEquals(10 * PB, UnitConverter.byteStringAs("10pB", ByteUnit.BYTE));
- assertEquals(10 * PB, UnitConverter.byteStringAs("10p", ByteUnit.BYTE));
- assertEquals(10 * PB, UnitConverter.byteStringAs("10P", ByteUnit.BYTE));
-
- assertEquals(10 * TB, UnitConverter.byteStringAs("10TB", ByteUnit.BYTE));
- assertEquals(10 * TB, UnitConverter.byteStringAs("10tb", ByteUnit.BYTE));
- assertEquals(10 * TB, UnitConverter.byteStringAs("10tB", ByteUnit.BYTE));
- assertEquals(10 * TB, UnitConverter.byteStringAs("10T", ByteUnit.BYTE));
- assertEquals(10 * TB, UnitConverter.byteStringAs("10t", ByteUnit.BYTE));
-
- assertEquals(10 * GB, UnitConverter.byteStringAs("10GB", ByteUnit.BYTE));
- assertEquals(10 * GB, UnitConverter.byteStringAs("10gb", ByteUnit.BYTE));
- assertEquals(10 * GB, UnitConverter.byteStringAs("10gB", ByteUnit.BYTE));
-
- assertEquals(10 * MB, UnitConverter.byteStringAs("10MB", ByteUnit.BYTE));
- assertEquals(10 * MB, UnitConverter.byteStringAs("10mb", ByteUnit.BYTE));
- assertEquals(10 * MB, UnitConverter.byteStringAs("10mB", ByteUnit.BYTE));
- assertEquals(10 * MB, UnitConverter.byteStringAs("10M", ByteUnit.BYTE));
- assertEquals(10 * MB, UnitConverter.byteStringAs("10m", ByteUnit.BYTE));
-
- assertEquals(10 * KB, UnitConverter.byteStringAs("10KB", ByteUnit.BYTE));
- assertEquals(10 * KB, UnitConverter.byteStringAs("10kb", ByteUnit.BYTE));
- assertEquals(10 * KB, UnitConverter.byteStringAs("10Kb", ByteUnit.BYTE));
- assertEquals(10 * KB, UnitConverter.byteStringAs("10K", ByteUnit.BYTE));
- assertEquals(10 * KB, UnitConverter.byteStringAs("10k", ByteUnit.BYTE));
-
- assertEquals(1111, UnitConverter.byteStringAs("1111", ByteUnit.BYTE));
+ private static Stream<Arguments> byteStringArgs() {
+ return Stream.of(
+ Arguments.arguments(10 * PB, "10PB", ByteUnit.BYTE),
+ Arguments.arguments(10 * PB, "10pb", ByteUnit.BYTE),
+ Arguments.arguments(10 * PB, "10pB", ByteUnit.BYTE),
+ Arguments.arguments(10 * PB, "10p", ByteUnit.BYTE),
+ Arguments.arguments(10 * PB, "10P", ByteUnit.BYTE),
+
+ Arguments.arguments(10 * TB, "10TB", ByteUnit.BYTE),
+ Arguments.arguments(10 * TB, "10tb", ByteUnit.BYTE),
+ Arguments.arguments(10 * TB, "10tB", ByteUnit.BYTE),
+ Arguments.arguments(10 * TB, "10T", ByteUnit.BYTE),
+ Arguments.arguments(10 * TB, "10t", ByteUnit.BYTE),
+
+ Arguments.arguments(10 * GB, "10GB", ByteUnit.BYTE),
+ Arguments.arguments(10 * GB, "10gb", ByteUnit.BYTE),
+ Arguments.arguments(10 * GB, "10gB", ByteUnit.BYTE),
+
+ Arguments.arguments(10 * MB, "10MB", ByteUnit.BYTE),
+ Arguments.arguments(10 * MB, "10mb", ByteUnit.BYTE),
+ Arguments.arguments(10 * MB, "10mB", ByteUnit.BYTE),
+ Arguments.arguments(10 * MB, "10M", ByteUnit.BYTE),
+ Arguments.arguments(10 * MB, "10m", ByteUnit.BYTE),
+
+ Arguments.arguments(10 * KB, "10KB", ByteUnit.BYTE),
+ Arguments.arguments(10 * KB, "10kb", ByteUnit.BYTE),
+ Arguments.arguments(10 * KB, "10Kb", ByteUnit.BYTE),
+ Arguments.arguments(10 * KB, "10K", ByteUnit.BYTE),
+ Arguments.arguments(10 * KB, "10k", ByteUnit.BYTE),
+
+ Arguments.arguments(1111L, "1111", ByteUnit.BYTE),
+
+ Arguments.arguments(null, "1/2", ByteUnit.BYTE),
+ Arguments.arguments(null, "10f", ByteUnit.BYTE),
+ Arguments.arguments(null, "f91", ByteUnit.BYTE),
+ Arguments.arguments(null, "1.0", ByteUnit.BYTE)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("byteStringArgs")
+ public void testByteString(Long expected, String value, ByteUnit unit) {
+ if (expected == null) {
+ assertThrows(NumberFormatException.class, () ->
UnitConverter.byteStringAs(value, unit));
+ } else {
+ assertEquals(expected, UnitConverter.byteStringAs(value, unit));
+ }
+ }
+
+ private static final long US = TimeUnit.MICROSECONDS.toMicros(1L);
+ private static final long MS = TimeUnit.MILLISECONDS.toMicros(1L);
+ private static final long SEC = TimeUnit.SECONDS.toMicros(1L);
+ private static final long MIN = TimeUnit.MINUTES.toMicros(1L);
+ private static final long HOUR = TimeUnit.HOURS.toMicros(1L);
+ private static final long DAY = TimeUnit.DAYS.toMicros(1L);
+
+ private static Stream<Arguments> timeStringArgs() {
+ return Stream.of(
+ Arguments.arguments(3 * US, "3us", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * MS, "3ms", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * SEC, "3s", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * MIN, "3m", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * MIN, "3min", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * HOUR, "3h", TimeUnit.MICROSECONDS),
+ Arguments.arguments(3 * DAY, "3d", TimeUnit.MICROSECONDS),
+
+ Arguments.arguments(5L, "5", TimeUnit.MILLISECONDS),
+ Arguments.arguments(5L, "5", TimeUnit.SECONDS),
+ Arguments.arguments(5L, "5", TimeUnit.MINUTES),
+
+ Arguments.arguments(null, "3ns", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "3sec", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "1.5h", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "3hours", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "3days", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "3w", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "3weeks", TimeUnit.MICROSECONDS),
+ Arguments.arguments(null, "foo", TimeUnit.MICROSECONDS)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("timeStringArgs")
+ public void testTimeString(Long expected, String value, TimeUnit unit) {
+ if (expected == null) {
+ assertThrows(NumberFormatException.class, () ->
UnitConverter.timeStringAs(value, unit));
+ } else {
+ assertEquals(expected, UnitConverter.timeStringAs(value, unit));
+ }
}
}
