[
https://issues.apache.org/jira/browse/HBASE-12260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16202898#comment-16202898
]
Appy commented on HBASE-12260:
------------------------------
After looking at MockNoopHMaster, here's why i think we should rather have a
separate internal interface like RegionServerServices.
(copy pasting my comment on MockHMaster.java from RB version10)
This doesn't look like either a mock or a noop master ([it's probably a
'fake'?|https://stackoverflow.com/questions/346372/whats-the-difference-between-faking-mocking-and-stubbing])
It's not mock because:
- HMaster constructor will still initialize a lot of state. If a test call
getAM() or getMasterCoprocessorHost(), it'll get something, but if a test
calls, getChoreService() or getCoordinatedStateManager(), it'll get null.
We only are only nullifying few fns here, rest are still from HMaster. It's
like, if a car frame is a mock, and a new car is a proper HMaster object, this
one is like http://www.3sgto.org/garage_attachment.php?id=1733.
Somethings work, somethings don't.
(Note that not calling super() constructor won't help. One will still have to
dig into code to figure out what behavior this noop/mock object will exhibit)
- It's not easy to maintain either. We cannot enforce that any change to
HMaster (or any of it's parent interfaces) will correspondingly add a
nullifying function here. Which means that as more fns are added, this will
become more and more like real object, and test can start to rely on that,
which will be very bad.
That doesn't happen when tests do Mockito.mock(<class>) by themselves. One, its
isolated from concrete classes; Two, test can setup exact behavior with
when().then().
- Tests using this will start depending on each other inexplicably in two ways:
1) If a new test comes which wants to mock something, it can't, because
previous tests are probably relying on current non-mocked behavior
2) First test to add mocked version of any function will govern what 'mock'
means. Does it mean returning null, or does it mean returning empty.
That's why, although RSS has an extra internal interface, i like that better
since interfaces give better testing platform using mocks. Both ways, we need
something extra, either an interface, or a mock class. I am just justifying why
former is much better than latter.
> MasterServices needs a short-back-and-sides; pare-back exposure of internals
> and IA.Private classes
> ---------------------------------------------------------------------------------------------------
>
> Key: HBASE-12260
> URL: https://issues.apache.org/jira/browse/HBASE-12260
> Project: HBase
> Issue Type: Sub-task
> Components: master
> Reporter: ryan rawson
> Assignee: stack
> Priority: Critical
> Fix For: 2.0.0-alpha-4
>
> Attachments: HBASE-12260.master.001.patch,
> HBASE-12260.master.002.patch, HBASE-12260.master.003.patch,
> HBASE-12260.master.004.patch, HBASE-12260.master.005.patch,
> HBASE-12260.master.006.patch, HBASE-12260.master.007.patch,
> HBASE-12260.master.008.patch, HBASE-12260.master.009.patch,
> HBASE-12260.master.010.patch, HBASE-12260.master.011.patch,
> HBASE-12260.master.011.patch, HBASE-12260.master.012.patch,
> HBASE-12260.master.013.patch, HBASE-12260.master.014.patch
>
>
> A major issue with MasterServices is the MasterCoprocessorEnvironment exposes
> this class even though MasterServices is tagged with
> @InterfaceAudience.Private
> This means that the entire internals of the HMaster is essentially part of
> the coprocessor API. Many of the classes returned by the MasterServices API
> are highly internal, extremely powerful, and subject to constant change.
> Perhaps a new API to replace MasterServices that is use-case focused, and
> justified based on real world co-processors would suit things better.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
