[
https://issues.apache.org/jira/browse/HBASE-19948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16354992#comment-16354992
]
stack commented on HBASE-19948:
-------------------------------
bq. I think it is reasonable to have a hard limit higher than the expect run
time since we will kill the test directly after the timeout?
Yes. That'd be good; us killing the test rather than surefire doing it for us.
bq. We can say explicitly that, a small test is expected to finish within 15
seconds, but the timeout is 60s which makes the test stable on most(slow)
machines, ditto for medium and large test. And the test runs nearly 10 minutes,
you’d better split it into several small ones.
Agree w/ this general sentiment. A bit of slop is easier on the user (a soft
and hard limit would be idea; warning before cut-off).
Problem is the move to test class-based timing. They'd be fine if we were
always categorizing tests by total time taken by the test suite/class but thats
not how they were written. So, the last few days we've been filing tens of
issues moving tests between one notion of what the categories meant to a new
inspecific hoping they'll pass again... And the movements are arbitrary; if
they hit the 180 second limit, make them large. If they hit 60 seconds, make
them medium (Small was 30 seconds until a few days ago).
The categories have lost meaning.
If we keep on this route, why not just up the timeouts to be ten minutes
everywhere and remove categories altogether? It would save us a bunch of time
moving test suites between categories (e.g. looking at a nightly run, tests
that were problematic in the flakey list like TestCreateTableProcedure and
TestSplitTableRegionProcedure failed in the last run up against the 180second
timeout; so now these should be made large tests, and on we go for the next few
weeks it looks like?).
> Since HBASE-19873, HBaseClassTestRule, Small/Medium/Large has different
> semantic
> --------------------------------------------------------------------------------
>
> Key: HBASE-19948
> URL: https://issues.apache.org/jira/browse/HBASE-19948
> Project: HBase
> Issue Type: Bug
> Reporter: stack
> Assignee: stack
> Priority: Major
> Fix For: 2.0.0-beta-2
>
>
> I was confused on how SmallTest/MediumTest/LargeTest were being interpreted
> since HBASE-19873 where we added HBaseClassTestRule enforcing a ClassRule.
> Small/Medium/Large are defined up in the refguide here:
> http://hbase.apache.org/book.html#hbase.unittests
> E.g: "Small test cases are executed in a shared JVM and individual test cases
> should run in 15 seconds or less..."
> I've always read the above as each method in a test suite/class should take
> 15 seconds (see below for finding by [~appy] [1]).
> The old CategoryBasedTimeout annotation used to try and enforce a test method
> taking only its designated category amount of time.
> The JUnit Timeout Rule talks about enforcing the timeout per test method:
> https://junit.org/junit4/javadoc/4.12/org/junit/rules/Timeout.html
> The above meant that you could have as many tests as you wanted in a
> class/suite and it could run as along as you liked as along as each
> individual test stayed within its category-based elapsed amount of time (and
> the whole suite completed inside the surefire fork timeout of 15mins).
> Then came HBASE-19873 which addressed an awkward issue around accounting for
> time spent in startup/shutdown -- i.e. time taken outside of a test method
> run -- and trying to have a timeout that cuts in before the surefire fork one
> does. It ended up adding a ClassRule that set a timeout on the whole test
> *suite/class* -- Good -- but the timeout set varies dependent upon the test
> category. A suite/class with 60 small tests that each take a second to
> complete now times out if you add one more test to the suite (61 seconds > 60
> seconds timeout -- give or take vagaries of the platform you run the test on).
> This latter change I have trouble with. It changes how small/medium/large
> have classically been understood. I think it will confuse too as now devs
> must do careful counting of test methods per class; one fat one (i.e.
> 'large') is same as N small ones. Could we set a single timeout on the whole
> test suite/class, one that was well less than the surefire fork kill timeout
> of 900seconds but keep the old timeout on each method as we used to have with
> the category-based annotation?
> (Am just looking for agreement that we have a problem here and that we want
> categories to be per test method as it used be; how to do it doesn't look
> easy and is for later).
> 1. @appy pointed out that the actual SmallTest annotation says something
> other than what is in the refguide: "Tag a test as 'small', meaning that the
> test class has the following characteristics: ....ideally, last less than 15
> seconds...."
> https://github.com/apache/hbase/blob/master/hbase-annotations/src/test/java/org/apache/hadoop/hbase/testclassification/SmallTests.java#L22
> 2. Here is code to show how timeout has changed now... previous the below
> would have 'run' without timing out.
> @Category({SmallTests.class})
> public class TestTimingOut {
> @ClassRule
> public static final HBaseClassTestRule CLASS_RULE =
> HBaseClassTestRule.forClass(TestTimingOut.class);
> @Test
> public void oneTest() {
> Threads.sleep(14000);
> }
> @Test
> public void twoTest() {
> Threads.sleep(14000);
> }
> @Test
> public void threeTest() {
> Threads.sleep(14000);
> }
> @Test
> public void fourTest() {
> Threads.sleep(14000);
> }
> @Test
> public void fiveTest() {
> Threads.sleep(14000);
> }
> }
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
