New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-23963 Split TestFromClientSide; it takes too long to complete t… #1271
Conversation
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a minor concern on whether we need to add RunWith annotation for sub classes. If it is confirmed to be OK then I will give a +1.
static int SLAVES = 1; | ||
|
||
// To keep the child classes happy. | ||
FromClientSideBase() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better have these field and methods as protected? No a big deal though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me fix.
/** | ||
* Test all client operations with a coprocessor that just implements the default flush/compact/scan | ||
* policy. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not need RunWith?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its doing the right thing w/o (there was no @RunWith before I showed up).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked the code for RunWith
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
public @interface RunWith {
/**
* @return a Runner class (must have a constructor that takes a single Class to run)
*/
Class<? extends Runner> value();
}
It has an @Inherited
annotation so it should be fine.
🎊 +1 overall
This message was automatically generated. |
Why a failed UT is also a -0? Because of this is a JDK11 run? Or? @ndimiduk |
And @saintstack sir, mind providing patch for master too? |
💔 -1 overall
This message was automatically generated. |
The run failed w/ OOME. Let me add in the protected annotations. Thanks for review @Apache9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
…iming out Split TestGetFromClientSide. Means have to also split the superclass TestFromClientSideWithCoprocessor.
f42b839
to
472c290
Compare
I just pushed the last patch which adds in protected qualifiiers on base class to branch-2 after testing locally. I'm watching tests upstream so if issue will deal w/ it. Will forward-port to master now. Thanks for review @Apache9 |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Yes right now we have known failures with |
…iming out
Split TestGetFromClientSide. Means have to also split the superclass
TestFromClientSideWithCoprocessor.