hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Enis Söztutar <enis....@gmail.com>
Subject Re: Adding frameworks and coding for testabilitywas [Re: Testing the fix for race condition between Compaction and StoreScanner.init]
Date Thu, 21 Nov 2013 00:37:53 GMT
+1 on improving on testability / DI side of things.


On Wed, Nov 20, 2013 at 3:27 PM, Jonathan Hsieh <jon@cloudera.com> wrote:

> tl;dr if we are going to add new frameworks, please make them a separate
> patch.  another attempt to bring up coding for testability/dep injection
> again.
>
> I've changed the subject because I really meant this to be a bigger
> discussion about setting up our code to be more testable and using
> conventions that allow us to do dependency injection (ala guava or in a way
> we can use mockito) or deciding to to include this new-to-me
> InjectionHandler frameworks from the 89-fb branch.  Porting HBASE-9949's
> testing code depended this Injector infrastructure that I found
> distasteful.
>
> This particular example smells funny and looked like another stovepiped
> framework to me --
> 1) It added references and objects to our core code instead of having it
> only present for our tests.
> 2) This made the core code more cumbersome to read.
> 3) Initial version used a brittle convention.   Initially, the injection
> point was identified by passing empty object array of particular size (0
> size mean option 0, size 1 meant injection point 1, etc).  Later changed to
> a enum, As used in the 89-fb branch we have a new global pool of unrelated
> enum/constant values that seemed brittle for further extension.
>
> <soapbox>Generally tests should be able to setup custom objects with
> constructors so we can use mockito or something similar in unit tests.  If
> done well we can instead of adding runtime injector objects to production
> code.  I'm making a plea to spend sometime tightening the code up and to
> conveying more design in design.   </soapbox>
>
> Recently been spending time in the hlog area and will likely personally
> start there improving comments/names, conveying design intent, and
> hopefully improving it. ;)
>
> Jon.
>
> On Wed, Nov 20, 2013 at 9:48 AM, Ted Yu <yuzhihong@gmail.com> wrote:
>
> > As requested by Jonathan, I am posting the following approach for testing
> > fix of HBASE-9949 :
> >
> >
> > 1. Introduce config parameter for StoreScanner implementation which would
> > be used by HStore for creating scanner
> > 2. In TestStoreScanner, add StoreScanner implementation which extends
> > StoreScanner and set the above config parameter to this class.
> > 3. Register custom ChangedReadersObserver through the following API of
> > HStore :
> >
> >   public void addChangedReaderObserver(ChangedReadersObserver o) {
> >
> > The BEFORE_SEEK hook would be activated before Store.getScanner() is
> > called.
> > The AFTER_SEEK hook can be activated at the end of ctor of StoreScanner
> > wrapper created in #2
> > The custom ChangedReadersObserver would activate the COMPACT_COMPLETE
> hook.
> >
> > Comments are welcome.
> >
>
>
>
> --
> // Jonathan Hsieh (shay)
> // Software Engineer, Cloudera
> // jon@cloudera.com
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message