hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ted Yu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-7384) Introducing waitForCondition function into test cases
Date Fri, 28 Dec 2012 21:34:14 GMT

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

Ted Yu commented on HBASE-7384:

Putting patch on review board would make reviewing easier.

+ * A class that provides a standard waitFor implementation pattern
Remove 'implementation' above.
+public abstract class Waiter {
I don't see abstract method inside Waiter. The class shouldn't be abstract.
+  private static final Log LOG = LogFactory.getLog(Bytes.class);
Fix class name above.
+  public static final String TEST_WAITFOR_RATIO_PROP = "test.waitfor.ratio";
Please explain the meaning of the ratio.
+   * {@link #waitFor(long, long, Predicate)} and {@link #waitFor(long, long, boolean, Predicate)}
Wrap long line above. 'method' -> 'methods'
+   * This is useful when running tests in slow machines for tests that are time sensitive.
Rephrase the above as 'This is useful when running time sensitive tests on slow machines'
Currently setWaitForRatio() is not called. In what circumstance would it be used ?
+   * Returns the 'wait for ratio' used in the {@link #sleep(long)}, {@link #waitFor(int,
+   * and {@link #waitFor(int, boolean, Predicate)} methods for the current test class. <p/>
This is
I think the parameter types for the two waitFor() methods are wrong.
+   * useful when running tests in slow machines for tests that are time sensitive. <p/>
The default
Suggest similar rephrase for above.
+   * value is obtained from the Java System property <code>test.wait.for.ratio</code>
which defaults
The property name is different from the value for TEST_WAITFOR_RATIO_PROP
+   * Makes the current thread sleep for the specified number of milliseconds.
The above description is not accurate, please mention the role of WaitForRatio

More reviews to follow
> Introducing waitForCondition function into test cases
> -----------------------------------------------------
>                 Key: HBASE-7384
>                 URL: https://issues.apache.org/jira/browse/HBASE-7384
>             Project: HBase
>          Issue Type: Test
>          Components: test
>            Reporter: Jeffrey Zhong
>            Assignee: Jeffrey Zhong
>              Labels: test
>             Fix For: 0.96.0
>         Attachments: hbase-7384_1.0.patch, hbase-7384.patch, Waiter.java
> Recently I'm working on flaky test cases and found we have many places using while loop
and sleep to wait for a condition to be true. There are several issues in existing ways:
> 1) Many similar code doing the same thing
> 2) When time out happens, different errors are reported without explicitly indicating
a time out situation
> 3) When we want to increase the max timeout value to verify if a test case fails due
to a not-enough time out value, we have to recompile & redeploy code
> I propose to create a waitForCondition function as a test utility function like the following:
> {code}
>     public interface WaitCheck {
>         public boolean Check() ;
>     }
>     public boolean waitForCondition(int timeOutInMilliSeconds, int checkIntervalInMilliSeconds,
WaitCheck s)
>             throws InterruptedException {
>         int multiplier = 1;
>         String multiplierProp = System.getProperty("extremeWaitMultiplier");
>         if(multiplierProp != null) {
>             multiplier = Integer.parseInt(multiplierProp);
>             if(multiplier < 1) {
>                 LOG.warn(String.format("Invalid extremeWaitMultiplier property value:%s.
is ignored.", multiplierProp));
>                 multiplier = 1;
>             }
>         }
>         int timeElapsed = 0;
>         while(timeElapsed < timeOutInMilliSeconds * multiplier) {
>             if(s.Check()) {
>                 return true;
>             }
>             Thread.sleep(checkIntervalInMilliSeconds);
>             timeElapsed += checkIntervalInMilliSeconds;
>         }
>         assertTrue("WaitForCondition failed due to time out(" + timeOutInMilliSeconds
+ " milliseconds expired)",
>                 false);
>         return false;
>     }
> {code}
> By doing the above way, there are several advantages:
> 1) Clearly report time out error when such situation happens
> 2) Use System property extremeWaitMultiplier to increase max time out dynamically for
a quick verification
> 3) Standardize current wait situations
> Pleas let me know what your thoughts on this.
> Thanks,
> -Jeffrey

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message