Jonathan Hsieh 2013-11-20, 23:27
Enis Söztutar 2013-11-21, 00:37
Andrew Purtell 2013-11-20, 23:40
Stack 2013-11-22, 20:54
Andrew Purtell 2013-11-23, 18:40
Ted Yu 2013-11-23, 18:59
Todd Lipcon 2013-11-22, 23:06
Andrew Purtell 2013-11-23, 18:51
Steve Loughran 2013-11-25, 10:27
Enis Söztutar 2013-11-25, 19:06
-Re: Adding frameworks and coding for testabilitywas [Re: Testing the fix for race condition between Compaction and StoreScanner.init]
One thing that is needed for proper mocking is also good class boundaries.
We don't have these right now for e.g. HStore/HRegion and splitting them
into granular parts may be a lot of work.
This creates two problems,
1) Tests Enis mentions that substitute a class while running a mini cluster
- because the classes are too big it's hard to test them in isolation, or
it requires some more ugly plumbing in production code like special test
But, this can be solved in the usual manner (and is already doable for
compactor), by specifying class name in config. That's a bit similar to
some DI frameworks, where you go to a central authority and ask for an
implementation of such and such interface to use.
2) Mocking may not work - for example, HBASE-8518 would be hard to fix with
mocking right now because it's some HStore code calling some other HStore
code. So there's nothing to mock.
For such cases, some fault injection is needed now; it could be done using
CPs but IMHO it's not any better than the FB framework from original JIRA -
it's still arbitrary injection point, it's just masking as something else,
a genuine coproc entry point (which we already have too many imho).
Btw, integration tests could use a fault injection framework but it would
be very different from what the FB one does, it won't muck with internals
in such manner.
TL; DR: to summarize, I think we can inject classes using config as a
current solution for some tests that we want but cannot add cleanly, but
ideally we should make classes more unit testable.
Then we won't need fault injection framework for commit tests.
On Mon, Nov 25, 2013 at 11:06 AM, Enis Söztutar <[EMAIL PROTECTED]> wrote:
> Similar to what Todd describes, I remember using our own co-processors at a
> couple of tests for doing fault injection. Since the coprocessors are
> embedded in a lot of places, just wrapping the FI into a coprocessor class
> worked for me.
> The problem with mocking is that, a lot of the tests we have want to change
> a single class (lets say Compactor), but have the whole mini cluster up and
> running. I've seen some use of FI in the 0.89-fb branch.
> On Mon, Nov 25, 2013 at 2:27 AM, Steve Loughran <[EMAIL PROTECTED]
> > On 23 November 2013 18:51, Andrew Purtell <[EMAIL PROTECTED]> wrote:
> > > Personally I would try to mock before adding fault injection framework.
> > > (Guilty of doing that in a recent patch-in-progress, but I have come to
> > my
> > > senses in time.) No objection to fault injection frameworks per se.
> > > HDFS as an example again, please correct me if I'm mistaken, there was
> > > AOP fault injection framework once but it is currently disabled, a
> > > of the migration from Ant to Maven, and possibly will be removed.
> > That and the fact that not only was it brittle, it was under-understood
> > so undermaintained -people got scared of it, and when it reported
> > the "blame the test framework" became the strategy.
> > > The
> > > trouble with testing frameworks is the added debt they accumulate over
> > > time, like everything else. If we commit to adding one, and also use it
> > as
> > > much as possible, that would be fine with me. Either way, we should
> > > definitely discuss the new/proposed framework and commit it on its own
> > > JIRA. I'm concerned how one got through the back door on HBASE-9949.
> > >
> > >
> > Another is "test frameworks may impose requirements on the underlying
> > code". This exists in YARN where I couldn't make some of the service
> > of YARN-117 final as it screwed up Mockito.
> > Things like Guice are very visible here, but if you can adapt your code
> > use it in other ways it may be acceptable.
> > --
> > CONFIDENTIALITY NOTICE
> > NOTICE: This message is intended for the use of the individual or entity
> > which it is addressed and may contain information that is confidential,
> > privileged and exempt from disclosure under applicable law. If the reader
NOTICE: This message is intended for the use of the individual or entity to
which it is addressed and may contain information that is confidential,
privileged and exempt from disclosure under applicable law. If the reader
of this message is not the intended recipient, you are hereby notified that
any printing, copying, dissemination, distribution, disclosure or
forwarding of this communication is strictly prohibited. If you have
received this communication in error, please contact the sender immediately
and delete it from your system. Thank You.
Jonathan Hsieh 2013-11-26, 17:51