From d4c7daabc1807c689b12a9b463e684d2e79fe8d0 Mon Sep 17 00:00:00 2001 From: jermy Date: Sun, 26 Nov 2023 16:03:00 +0800 Subject: [PATCH 1/3] fix: Assert.assertThrows() should check result of exceptionConsumer --- .../org/apache/hugegraph/testutil/Assert.java | 26 ++++++---- .../apache/hugegraph/util/ExceptionUtil.java | 51 +++++++++++++++++++ .../apache/hugegraph/testutil/AssertTest.java | 27 ++++++++-- .../unit/util/ReflectionUtilTest.java | 16 +++--- 4 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java b/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java index 218342ea..60228c10 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java @@ -21,6 +21,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import org.apache.hugegraph.util.ExceptionUtil; import org.hamcrest.BaseMatcher; import org.hamcrest.CoreMatchers; import org.hamcrest.Description; @@ -37,28 +38,29 @@ public interface ThrowableConsumer { void accept(T t) throws Throwable; } - public static void assertThrows(Class throwable, - ThrowableRunnable runnable) { - CompletableFuture future = assertThrowsFuture(throwable, runnable); + public static Throwable assertThrows(Class throwable, + ThrowableRunnable runnable) { + CompletableFuture future = assertThrowsFuture(throwable, runnable); future.thenAccept(System.err::println); + return ExceptionUtil.futureGet(future); } public static void assertThrows(Class throwable, ThrowableRunnable runnable, Consumer exceptionConsumer) { - CompletableFuture future = assertThrowsFuture(throwable, - runnable); - future.thenAccept(exceptionConsumer); + CompletableFuture future = assertThrowsFuture(throwable, runnable); + CompletableFuture futureConsumer = future.thenAccept(exceptionConsumer); + ExceptionUtil.futureGet(futureConsumer); } public static CompletableFuture assertThrowsFuture( Class clazz, ThrowableRunnable runnable) { CompletableFuture future = new CompletableFuture<>(); - boolean fail = false; + boolean noException = false; try { runnable.run(); - fail = true; + noException = true; } catch (Throwable e) { if (!clazz.isInstance(e)) { Assert.fail(String.format( @@ -67,7 +69,7 @@ public static CompletableFuture assertThrowsFuture( } future.complete(e); } - if (fail) { + if (noException) { String msg = String.format("No exception was thrown(expected %s)", clazz.getName()); future.completeExceptionally(new AssertionError(msg)); @@ -104,34 +106,40 @@ public static void assertEquals(double expected, Object actual) { org.junit.Assert.assertEquals(expected, actual); } + @SuppressWarnings("deprecation") public static void assertGt(Number expected, Object actual) { org.junit.Assert.assertThat(actual, new NumberMatcher(expected, cmp -> { return cmp > 0; }, ">")); } + @SuppressWarnings("deprecation") public static void assertGte(Number expected, Object actual) { org.junit.Assert.assertThat(actual, new NumberMatcher(expected, cmp -> { return cmp >= 0; }, ">=")); } + @SuppressWarnings("deprecation") public static void assertLt(Number expected, Object actual) { org.junit.Assert.assertThat(actual, new NumberMatcher(expected, cmp -> { return cmp < 0; }, "<")); } + @SuppressWarnings("deprecation") public static void assertLte(Number expected, Object actual) { org.junit.Assert.assertThat(actual, new NumberMatcher(expected, cmp -> { return cmp <= 0; }, "<=")); } + @SuppressWarnings("deprecation") public static void assertContains(String sub, String actual) { org.junit.Assert.assertThat(actual, CoreMatchers.containsString(sub)); } + @SuppressWarnings("deprecation") public static void assertInstanceOf(Class clazz, Object object) { org.junit.Assert.assertThat(object, CoreMatchers.instanceOf(clazz)); } diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java b/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java new file mode 100644 index 00000000..20bc0b7a --- /dev/null +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java @@ -0,0 +1,51 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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.hugegraph.util; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; + +public final class ExceptionUtil { + + public static Throwable rootCause(Throwable e) { + Throwable cause = e; + while (cause.getCause() != null) { + cause = cause.getCause(); + } + return cause; + } + + public static RuntimeException transToRuntimeException(Throwable e) { + if (e instanceof RuntimeException) { + return (RuntimeException) e; + } + return new RuntimeException(rootCause(e).getMessage(), e); + } + + public static T futureGet(Future future) { + try { + return future.get(); + } catch (InterruptedException e) { + throw ExceptionUtil.transToRuntimeException(e); + } catch (ExecutionException e) { + throw ExceptionUtil.transToRuntimeException(e.getCause()); + } + } +} diff --git a/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java b/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java index cf08fdaf..7fdc9dae 100644 --- a/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java +++ b/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java @@ -17,9 +17,8 @@ package org.apache.hugegraph.testutil; -import org.junit.Test; - import org.apache.hugegraph.unit.BaseUnitTest; +import org.junit.Test; public class AssertTest extends BaseUnitTest { @@ -175,6 +174,12 @@ public void testAssertThrows() { throw new RuntimeException(); }); + Throwable exception = Assert.assertThrows(RuntimeException.class, () -> { + throw new RuntimeException("fake-error"); + }); + Assert.assertInstanceOf(RuntimeException.class, exception); + Assert.assertEquals("fake-error", exception.getMessage()); + Assert.assertThrows(RuntimeException.class, () -> { throw new RuntimeException("fake-error"); }, e -> { @@ -183,7 +188,7 @@ public void testAssertThrows() { } @Test - public void testAssertThrowsWithError() { + public void testAssertThrowsWithTypeError() { try { Assert.assertThrows(NullPointerException.class, () -> { // pass @@ -204,6 +209,22 @@ public void testAssertThrowsWithError() { } } + @Test + public void testAssertThrowsWithMessageError() { + try { + Assert.assertThrows(RuntimeException.class, () -> { + throw new RuntimeException("fake-error"); + }, e -> { + Assert.assertEquals("fake-error-typo", e.getMessage()); + }); + Assert.fail("Expect error"); + } catch (Exception e) { + Assert.assertInstanceOf(RuntimeException.class, e); // not AssertionError + Assert.assertContains("expected: but was:", + e.getMessage()); + } + } + @Test public void testAssertGt() { Assert.assertGt((byte) 1, Byte.valueOf("2")); diff --git a/hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java b/hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java index c83b2ad9..f511ddfc 100644 --- a/hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java +++ b/hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java @@ -22,19 +22,19 @@ import java.util.Comparator; import java.util.List; -import org.junit.Test; - +import org.apache.commons.collections.IteratorUtils; +import org.apache.hugegraph.perf.PerfUtil; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.unit.BaseUnitTest; import org.apache.hugegraph.unit.perf.testclass.TestClass; import org.apache.hugegraph.unit.perf.testclass.TestClass.Bar; import org.apache.hugegraph.unit.perf.testclass.TestClass.Base; import org.apache.hugegraph.unit.perf.testclass.TestClass.Foo; import org.apache.hugegraph.unit.perf.testclass.TestClass.ManuallyProfile; import org.apache.hugegraph.unit.perf.testclass.TestClass.Sub; -import org.apache.hugegraph.perf.PerfUtil; -import org.apache.hugegraph.unit.BaseUnitTest; import org.apache.hugegraph.util.ReflectionUtil; -import org.apache.commons.collections.IteratorUtils; +import org.junit.Test; + import com.google.common.reflect.ClassPath.ClassInfo; import javassist.NotFoundException; @@ -94,7 +94,7 @@ public void testClasses() throws IOException { @SuppressWarnings("unchecked") List classes = IteratorUtils.toList(ReflectionUtil.classes( "org.apache.hugegraph.util")); - Assert.assertEquals(18, classes.size()); + Assert.assertEquals(19, classes.size()); classes.sort(Comparator.comparing(ClassInfo::getName)); Assert.assertEquals("org.apache.hugegraph.util.Bytes", classes.get(0).getName()); @@ -102,8 +102,8 @@ public void testClasses() throws IOException { classes.get(1).getName()); Assert.assertEquals("org.apache.hugegraph.util.CollectionUtil", classes.get(2).getName()); - Assert.assertEquals("org.apache.hugegraph.util.VersionUtil", - classes.get(17).getName()); + Assert.assertEquals("org.apache.hugegraph.util.DateUtil", + classes.get(3).getName()); } @Test From 1d692daf503a8793879b305640a6386eca87bd33 Mon Sep 17 00:00:00 2001 From: jermy Date: Sun, 26 Nov 2023 22:18:31 +0800 Subject: [PATCH 2/3] fix some warnings --- .../java/org/apache/hugegraph/config/HugeConfig.java | 12 ++++++------ .../java/org/apache/hugegraph/perf/PerfUtil.java | 12 ++++++------ .../org/apache/hugegraph/rest/RestClientConfig.java | 1 + 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java b/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java index c48219fc..48371545 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java @@ -22,8 +22,8 @@ import java.util.List; import java.util.Map; -import org.apache.hugegraph.util.E; -import org.apache.hugegraph.util.Log; +import javax.annotation.Nullable; + import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.FileBasedConfiguration; import org.apache.commons.configuration2.PropertiesConfiguration; @@ -35,10 +35,10 @@ import org.apache.commons.configuration2.io.FileHandler; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; import org.slf4j.Logger; -import javax.annotation.Nullable; - public class HugeConfig extends PropertiesConfiguration { private static final Logger LOG = Log.logger(HugeConfig.class); @@ -117,7 +117,7 @@ public void addPropertyDirect(String key, Object value) { value = this.validateOption(key, value); } if (this.containsKey(key) && value instanceof List) { - for (Object item : (List) value) { + for (Object item : (List) value) { super.addPropertyDirect(key, item); } } else { @@ -137,7 +137,7 @@ private Object validateOption(String key, Object value) { return option.parseConvert((String) value); } - Class dataType = option.dataType(); + Class dataType = option.dataType(); if (dataType.isInstance(value)) { return value; } diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/perf/PerfUtil.java b/hugegraph-common/src/main/java/org/apache/hugegraph/perf/PerfUtil.java index 456611b6..77b68d6b 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/perf/PerfUtil.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/perf/PerfUtil.java @@ -556,7 +556,7 @@ public long preventOptimizePadding() { } } - public static final class LocalStack { + public static final class LocalStack { private final Object[] elementData; private int elementCount; @@ -574,27 +574,27 @@ boolean empty() { return this.elementCount == 0; } - public void push(E elem) { + public void push(T elem) { this.elementData[this.elementCount++] = elem; } - public E pop() { + public T pop() { if (this.elementCount == 0) { throw new EmptyStackException(); } this.elementCount--; @SuppressWarnings("unchecked") - E elem = (E) this.elementData[this.elementCount]; + T elem = (T) this.elementData[this.elementCount]; this.elementData[this.elementCount] = null; return elem; } - public E peek() { + public T peek() { if (this.elementCount == 0) { throw new EmptyStackException(); } @SuppressWarnings("unchecked") - E elem = (E) this.elementData[this.elementCount - 1]; + T elem = (T) this.elementData[this.elementCount - 1]; return elem; } } diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java b/hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java index ef3e9b0e..fc63613b 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java @@ -24,6 +24,7 @@ @Builder @Getter @Setter +@SuppressWarnings("unused") public class RestClientConfig { private String user; From 68b359e643d33203dc76d4d423cf42aa8b89ca3b Mon Sep 17 00:00:00 2001 From: jermy Date: Sun, 26 Nov 2023 23:35:29 +0800 Subject: [PATCH 3/3] remove assertThrowsFuture() from Assert --- .../org/apache/hugegraph/testutil/Assert.java | 50 ++++++++----------- .../apache/hugegraph/util/ExceptionUtil.java | 2 - .../apache/hugegraph/testutil/AssertTest.java | 3 +- .../apache/hugegraph/unit/BaseUnitTest.java | 8 +-- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java b/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java index 60228c10..bd82eef6 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/testutil/Assert.java @@ -17,11 +17,9 @@ package org.apache.hugegraph.testutil; -import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; import java.util.function.Function; -import org.apache.hugegraph.util.ExceptionUtil; import org.hamcrest.BaseMatcher; import org.hamcrest.CoreMatchers; import org.hamcrest.Description; @@ -38,44 +36,36 @@ public interface ThrowableConsumer { void accept(T t) throws Throwable; } - public static Throwable assertThrows(Class throwable, - ThrowableRunnable runnable) { - CompletableFuture future = assertThrowsFuture(throwable, runnable); - future.thenAccept(System.err::println); - return ExceptionUtil.futureGet(future); - } - - public static void assertThrows(Class throwable, + public static void assertThrows(Class clazz, ThrowableRunnable runnable, Consumer exceptionConsumer) { - CompletableFuture future = assertThrowsFuture(throwable, runnable); - CompletableFuture futureConsumer = future.thenAccept(exceptionConsumer); - ExceptionUtil.futureGet(futureConsumer); + Throwable expectedException = assertThrows(clazz, runnable); + assert expectedException != null; + exceptionConsumer.accept(expectedException); } - public static CompletableFuture assertThrowsFuture( - Class clazz, - ThrowableRunnable runnable) { - CompletableFuture future = new CompletableFuture<>(); - boolean noException = false; + public static Throwable assertThrows(Class clazz, + ThrowableRunnable runnable) { try { + // expect throwing here runnable.run(); - noException = true; } catch (Throwable e) { if (!clazz.isInstance(e)) { - Assert.fail(String.format( - "Bad exception type %s(expected %s)", - e.getClass().getName(), clazz.getName())); + // exception type not matched + Assert.fail(String.format("Bad exception type %s(expected %s)", + e.getClass().getName(), clazz.getName())); } - future.complete(e); - } - if (noException) { - String msg = String.format("No exception was thrown(expected %s)", - clazz.getName()); - future.completeExceptionally(new AssertionError(msg)); - Assert.fail(msg); + + return e; } - return future; + + // no exception + Assert.fail(String.format("No exception was thrown(expected %s)", + clazz.getName())); + + // unavailable + assert false; + return null; } public static void assertEquals(byte expected, Object actual) { diff --git a/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java b/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java index 20bc0b7a..7142aea7 100644 --- a/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java +++ b/hugegraph-common/src/main/java/org/apache/hugegraph/util/ExceptionUtil.java @@ -1,6 +1,4 @@ /* - * Copyright 2017 HugeGraph Authors - * * 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 diff --git a/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java b/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java index 7fdc9dae..53f60247 100644 --- a/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java +++ b/hugegraph-common/src/test/java/org/apache/hugegraph/testutil/AssertTest.java @@ -218,8 +218,7 @@ public void testAssertThrowsWithMessageError() { Assert.assertEquals("fake-error-typo", e.getMessage()); }); Assert.fail("Expect error"); - } catch (Exception e) { - Assert.assertInstanceOf(RuntimeException.class, e); // not AssertionError + } catch (AssertionError e) { Assert.assertContains("expected: but was:", e.getMessage()); } diff --git a/hugegraph-common/src/test/java/org/apache/hugegraph/unit/BaseUnitTest.java b/hugegraph-common/src/test/java/org/apache/hugegraph/unit/BaseUnitTest.java index 96133d40..6b9ffcc8 100644 --- a/hugegraph-common/src/test/java/org/apache/hugegraph/unit/BaseUnitTest.java +++ b/hugegraph-common/src/test/java/org/apache/hugegraph/unit/BaseUnitTest.java @@ -22,12 +22,12 @@ import java.net.URL; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import org.apache.commons.io.FileUtils; +import org.apache.hugegraph.util.ExceptionUtil; import org.apache.hugegraph.util.TimeUtil; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -51,11 +51,7 @@ protected static void runWithThreads(int threads, Runnable task) { futures.add(executor.submit(task)); } for (Future future : futures) { - try { - future.get(); - } catch (InterruptedException | ExecutionException e) { - throw new RuntimeException(e); - } + ExceptionUtil.futureGet(future); } }