You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Ted Yu (JIRA)" <ji...@apache.org> on 2012/12/28 22:44:12 UTC
[jira] [Comment Edited] (HBASE-7384) Introducing waitForCondition
function into test cases
[ https://issues.apache.org/jira/browse/HBASE-7384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13540604#comment-13540604 ]
Ted Yu edited comment on HBASE-7384 at 12/28/12 9:42 PM:
---------------------------------------------------------
Putting patch on review board would make reviewing easier.
{code}
+ * A class that provides a standard waitFor implementation pattern
{code}
Remove 'implementation' above.
{code}
+public abstract class Waiter {
{code}
I don't see abstract method inside Waiter. The class shouldn't be abstract.
{code}
+ private static final Log LOG = LogFactory.getLog(Bytes.class);
{code}
Fix class name above.
{code}
+ public static final String TEST_WAITFOR_RATIO_PROP = "test.waitfor.ratio";
{code}
Please explain the meaning of the ratio.
{code}
+ * {@link #waitFor(long, long, Predicate)} and {@link #waitFor(long, long, boolean, Predicate)} method
{code}
Wrap long line above. 'method' -> 'methods'
{code}
+ * This is useful when running tests in slow machines for tests that are time sensitive.
{code}
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 ?
{code}
+ * Returns the 'wait for ratio' used in the {@link #sleep(long)}, {@link #waitFor(int, Predicate)}
+ * and {@link #waitFor(int, boolean, Predicate)} methods for the current test class. <p/> This is
{code}
I think the parameter types for the two waitFor() methods are wrong.
{code}
+ * useful when running tests in slow machines for tests that are time sensitive. <p/> The default
{code}
Suggest similar rephrase for above.
{code}
+ * value is obtained from the Java System property <code>test.wait.for.ratio</code> which defaults
{code}
The property name is different from the value for TEST_WAITFOR_RATIO_PROP
{code}
+ * Makes the current thread sleep for the specified number of milliseconds.
{code}
The above description is not accurate, please mention the role of WaitForRatio
More reviews to follow
was (Author: yuzhihong@gmail.com):
Putting patch on review board would make reviewing easier.
{code}
+ * A class that provides a standard waitFor implementation pattern
{code}
Remove 'implementation' above.
{code}
+public abstract class Waiter {
{code}
I don't see abstract method inside Waiter. The class shouldn't be abstract.
{code}
+ private static final Log LOG = LogFactory.getLog(Bytes.class);
{code}
Fix class name above.
{code}
+ public static final String TEST_WAITFOR_RATIO_PROP = "test.waitfor.ratio";
{code}
Please explain the meaning of the ratio.
{code}
+ * {@link #waitFor(long, long, Predicate)} and {@link #waitFor(long, long, boolean, Predicate)} method
{code}
Wrap long line above. 'method' -> 'methods'
{code}
+ * This is useful when running tests in slow machines for tests that are time sensitive.
{code}
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 ?
{code}
+ * Returns the 'wait for ratio' used in the {@link #sleep(long)}, {@link #waitFor(int, Predicate)}
+ * and {@link #waitFor(int, boolean, Predicate)} methods for the current test class. <p/> This is
{code}
I think the parameter types for the two waitFor() methods are wrong.
{code}
+ * useful when running tests in slow machines for tests that are time sensitive. <p/> The default
{code}
Suggest similar rephrase for above.
{code}
+ * value is obtained from the Java System property <code>test.wait.for.ratio</code> which defaults
{code}
The property name is different from the value for TEST_WAITFOR_RATIO_PROP
{code}
{code}
+ * Makes the current thread sleep for the specified number of milliseconds.
{code}
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