Uh oh!
There was an error while loading. Please reload this page.
HBASE-26115 - ServerTestHBaseCluster interface for testing coprocs - #3523
HBASE-26115 - ServerTestHBaseCluster interface for testing coprocs#3523gjacoby126 wants to merge 1 commit into
Conversation
Apache-HBase
commented
Jul 22, 2021
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 22, 2021
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 23, 2021
🎊 +1 overall
This message was automatically generated. |
| * Common helpers for testing HBase that do not depend on specific server/etc. things. | ||
| */ | ||
| @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.PHOENIX) | ||
| @InterfaceAudience.LimitedPrivate({HBaseInterfaceAudience.COPROC, |
There was a problem hiding this comment.
Ah, in general let's not do this.
Mark it as IA.LP("Phoenix") is already a compromise, let's not add more compromise here...
There was a problem hiding this comment.
I'm not sure I understand what the compromise is here?
For example, I wear three different hats. I write code for HBase itself, for Phoenix, and for my dayjob. You seem to be arguing that the type of test infrastructure I require depends on what hat I'm wearing, not on the code I'm writing. Are you saying that server-side component tests are good when I wear the HBase hat, a tolerated compromise when I wear the Phoenix hat, and bad when I wear the dayjob hat? And that that would be the case _even for the same code _ depending on where I intended to push it when it's done?
In my view, HBase tests that just exercise the client API should use the TestingHBaseCluster. Same with Phoenix tests, or dayjob tests that only rely on the HBase client API. Tests that exercise server-side components that need the extra control over server-side state should be able to do so. That's true whether the components are housed in an HBase repo, or a Phoenix repo, or any other project, open source or not.
HBase gives a lot of ways to create server side code, and they all need to be tested. The IA should recognize that.
There was a problem hiding this comment.
I'm not a native English speaker so sorry I can not understand what you mean.
What I say compromise here, is in HBASE-13126, we want to mark HBTU as IA.Private, but consider the current situation, at least we need to mark it as IA.LP("Phoenix"), as we are sure that Phoenix needs to test something internal to HBase. This is a compromise.
But if you offer your help on improving the new testing clsuter interface, I think maybe we could finish the support for Phoenix before 3.0.0 is out, then probably we could mark HBTU as IA.Private in 3.0.0 finally.
Thanks.
| * | ||
| * @return the utility running the minicluster | ||
| */ | ||
| HBaseTestingUtil getUtil(); |
There was a problem hiding this comment.
This is a no no.
Let's find another way to only expose the necessary methods for testing coprocessors and other server side hooks such as tags. The point here is, HBaseTestingUtil is a class, not an interface, which makes very hard for the HBase developper to make it stable while keeping it clean.
There was a problem hiding this comment.
@Apache9 - thanks, that's a good point. In the next revision of this patch, I will create an interface for HBTU (and probably its superclasses) so this interface doesn't return a concrete class. I'm going to be on vacation for about a week but will take this up when I get back.
gjacoby126
commented
Aug 31, 2021
Closing for now as HBASE-26157 is an alternate solution to the same problem |
No description provided.