hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-19948) Since HBASE-19873, HBaseClassTestRule, Small/Medium/Large has different semantic
Date Thu, 08 Feb 2018 00:38:00 GMT

    [ https://issues.apache.org/jira/browse/HBASE-19948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16356284#comment-16356284

stack commented on HBASE-19948:

bq. Let’s move on and do not stick on the old javadoc.

Will do after we get clarity here. And it is more than just the javadoc. The tests have a
rough categorization by method test time. Changing how we do categorization to overall test
suite elapsed time means we'll have to recast the categories for a bunch of tests. This will
take a load of time and JIRAs and the end result is that categorization means less than it
does now. 

bq.  In general, usually we put several tests in the same class because the function they
test are in the same area, not the running time are in the same range...

Yes. This is a common rule; group all test into one test class though the test length varies.
The longer tests determine the category.

Other approaches are test a class's methods perhaps some of its aggregate functionality with
Mocks so the test suite runs fast and qualifies as 'small'. If more aggregate test needed,
spin up cluster in a new Medium or Large test class to test functionality with more moving
parts involved.

There are lots of approaches.

Some form of categorization seems useful. It allows us stage the test suite run somewhat and
pass different configuration for each step.

The @ClassRule approach we need because it measures the whole test run, startup and shutdown
included (@Rule missed this). We need this overview so we can kill test before surefire does
so we can see what is timing out whether the test or startup/shutdown. A @ClassRule-based
categorization means you can't see which individual test method has gone errant. @Rule will
print it out for you. With @ClassRule you have to mess in logs looking for it. A pain. Categorization
at the class-level is crass. This might be ok though. Individual test method timings are hard
to find, you see in IDE, but Maven test run result shows overall test run elapsed time. You
can see individual test methods up in jenkins test report but thats too late.

I can go @ClassRule and not bother reinstituting @Rule but all the doc on this topic need
update, this needs to be noted as an hbase2 particular change, and a report on soft-violators
of limits would help for those cases where a test verges on large and then a new method is
added and all starts to timeout... i suppose I don't feel too strongly about the change in
approach. I do feel strongly though that the change needs to be publicized and doc'd because
otherwise the new behavior will only baffle.

> 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
>         Attachments: HBASE-19948.branch-2.001.patch
> 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
> 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.
> {noformat}
> @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); }
> }
> {noformat}

This message was sent by Atlassian JIRA

View raw message