From af01cefdd8009768ffae7367af463c03fbc1aa89 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Tue, 6 Feb 2018 22:49:38 -0800 Subject: [PATCH] HBASE-19948 Suggestion Patch This is a dump suggestion. Means a ClassRule to monitor overall class run time (including startup/shutdown) and then the old category-based per test method rule to watch each test method to ensure they stay inside their category timing. Ugly. Would mean each test would have the two Rules added to it. Pluses are we'd have the old per test method timeout back plus the total class timeout so we can kill tests ourselves before surefire does (Adds modifying the class timeout per the number of test methods...). --- .../apache/hadoop/hbase/CategoryBasedTimeout.java | 76 ++++++++++++++++++++++ .../apache/hadoop/hbase/HBaseClassTestRule.java | 18 ++++- .../org/apache/hadoop/hbase/TestTimingOut.java | 58 +++++++++++++++++ 3 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 hbase-common/src/test/java/org/apache/hadoop/hbase/CategoryBasedTimeout.java create mode 100644 hbase-examples/src/test/java/org/apache/hadoop/hbase/TestTimingOut.java diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/CategoryBasedTimeout.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/CategoryBasedTimeout.java new file mode 100644 index 0000000000..fe0e5a5f35 --- /dev/null +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/CategoryBasedTimeout.java @@ -0,0 +1,76 @@ +/* + * 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.hadoop.hbase; + +import java.lang.annotation.Annotation; +import java.util.concurrent.TimeUnit; + +import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.experimental.categories.Category; +import org.junit.internal.runners.statements.FailOnTimeout; +import org.junit.rules.TestRule; +import org.junit.rules.Timeout; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * Set a test method timeout based off the test categories small, medium, large. + * Based on junit Timeout TestRule; see https://github.com/junit-team/junit/wiki/Rules + */ +public class CategoryBasedTimeout extends Timeout { + public CategoryBasedTimeout(long timeout, TimeUnit timeUnit) { + super(timeout, timeUnit); + } + + protected CategoryBasedTimeout(Builder builder) { + super(builder); + } + + public static Timeout forClass(Class clazz) { + return CategoryBasedTimeout.builder().withTimeout(clazz).withLookingForStuckThread(true) + .build(); + } + + public static Builder builder() { + return new CategoryBasedTimeout.Builder(); + } + + static class Builder extends Timeout.Builder { + public Timeout.Builder withTimeout(Class clazz) { + Annotation annotation = clazz.getAnnotation(Category.class); + if (annotation != null) { + Category category = (Category)annotation; + for (Class c: category.value()) { + if (c == SmallTests.class) { + // See SmallTests. Supposed to run 15 seconds. + return withTimeout(30, TimeUnit.SECONDS); + } else if (c == MediumTests.class) { + // See MediumTests. Supposed to run 50 seconds. + return withTimeout(180, TimeUnit.SECONDS); + } else if (c == LargeTests.class) { + // Let large tests have a ten minute timeout. + return withTimeout(10, TimeUnit.MINUTES); + } + } + } + return this; + } + } +} diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java index 734ce3f241..2f4f81cd4a 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseClassTestRule.java @@ -17,11 +17,13 @@ */ package org.apache.hadoop.hbase; +import java.lang.reflect.Method; import java.util.concurrent.TimeUnit; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.yetus.audience.InterfaceAudience; +import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.rules.TestRule; import org.junit.rules.Timeout; @@ -37,6 +39,7 @@ import org.junit.runners.model.Statement; */ @InterfaceAudience.Private public final class HBaseClassTestRule implements TestRule { + private static final long MAX_TIMEOUT_IN_SECONDS = 10 * 60; // Ten minutes. private final Class clazz; @@ -57,6 +60,15 @@ public final class HBaseClassTestRule implements TestRule { private static long getTimeoutInSeconds(Class clazz) { Category[] categories = clazz.getAnnotationsByType(Category.class); + Method[] methods = clazz.getMethods(); + // Count how many @Tests there are in this test suite/class. + int testCount = 0; + for (Method m: methods) { + Test annotation = m.getAnnotation(Test.class); + if (annotation != null) { + testCount++; + } + } if (categories.length == 0) { throw new IllegalArgumentException(clazz.getName() + " is not annotated with @Category"); } @@ -65,13 +77,13 @@ public final class HBaseClassTestRule implements TestRule { // See SmallTests. Supposed to run 15 seconds. // Lots of these timeout on Jenkins... a stall of ten or twenty seconds mess up what looks // fine when run local. - return 60; + return Math.min(testCount * 15, MAX_TIMEOUT_IN_SECONDS); } else if (c == MediumTests.class) { // See MediumTests. Supposed to run 50 seconds. - return 180; + return Math.min(testCount * 50, MAX_TIMEOUT_IN_SECONDS); } else if (c == LargeTests.class) { // Let large tests have a ten minute timeout. - return TimeUnit.MINUTES.toSeconds(10); + return MAX_TIMEOUT_IN_SECONDS; } } throw new IllegalArgumentException( diff --git a/hbase-examples/src/test/java/org/apache/hadoop/hbase/TestTimingOut.java b/hbase-examples/src/test/java/org/apache/hadoop/hbase/TestTimingOut.java new file mode 100644 index 0000000000..92c310fc1d --- /dev/null +++ b/hbase-examples/src/test/java/org/apache/hadoop/hbase/TestTimingOut.java @@ -0,0 +1,58 @@ +package org.apache.hadoop.hbase; + +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Threads; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestRule; + +@Category({SmallTests.class}) +public class TestTimingOut { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestTimingOut.class); + @Rule + public final TestRule TIMEOUT = CategoryBasedTimeout.forClass(TestTimingOut.class); + + @Test + public void oneTest() { + Threads.sleep(10000); + } + + @Test + public void twoTest() { + Threads.sleep(10000); + } + + @Test + public void threeTest() { + Threads.sleep(10000); + } + + @Test + public void fourTest() { + Threads.sleep(10000); + } + + @Test + public void fiveTest() { + Threads.sleep(10000); + } + + @Test + public void sixTest() { + Threads.sleep(10000); + } + + @Test + public void sevenTest() { + Threads.sleep(10000); + } + + @Test + public void eightTest() { + Threads.sleep(10000); + } +} -- 2.11.0 (Apple Git-81)