FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Dennis Byrne <de...@dbyrne.net>]

>I updated the Shale snapshots about half an hour ago... coincidence?
>Wendy

This would have happened w/ or w/out your update.  

The fix for this was to put FactoryFinder.releaseFactories() in setup() and in tearDown() .  This setup() call is to clear configured Factory info from previous tests ( which taints the first test of the current running TestCase ).  The tearDown() call is to prevent interference w/ subsequent TestCases.

In an offlist conversation, Sean and I recently came to the agreement that AbstractJsfTestCase should be calling FactoryFinder.releaseFactories() .  This is important because AbstractJsfTestCase.setup() configures each Factory, and AFAIK, it was interfering w/ his tests, which were children of AbstractJsfTestCase.  Sean ended up putting this in AbstractJsfTestCase.setup(), probably for the same reason it needs to now go in some of ours.

@ Sean or Wendy or Craig - can one of you please put FactoryFinder.releaseFactories() in AbstractJsfTestCase.tearDown() also ?

Dennis Byrne

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Craig McClanahan <cr...@apache.org>]

On 2/20/06, John Fallows <jo...@gmail.com> wrote:
>
> fyi - IIRC we tackled this problem slightly differently in the ADF Faces
> codebase.
>
> In an effort to fully isolate anything that might be keyed by
> ContextClassLoader (including FactoryFinder internal state), we created a
> trivial wrapper ClassLoader to provide a unique ContextClassLoader in
> setUp() and restore it back to the original in tearDown().


That is a *really* good idea ... I'll be adding it to Shale's test framework
tonight. I don't see it as mutually exclusive with any of the other cleanups
we are doing to ensure that different tests do not interfere with each
other.

Kind Regards,
> John Fallows.


Craig

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[John Fallows <jo...@gmail.com>]

fyi - IIRC we tackled this problem slightly differently in the ADF Faces
codebase.

In an effort to fully isolate anything that might be keyed by
ContextClassLoader (including FactoryFinder internal state), we created a
trivial wrapper ClassLoader to provide a unique ContextClassLoader in
setUp() and restore it back to the original in tearDown().

Kind Regards,
John Fallows.

On 2/20/06, Sean Schofield <se...@gmail.com> wrote:
>
> > The call is required in the setUp() method to make things work correctly
> on
> > the *first* test, when you have the MyFaces implementation in the
> classpath
> > of the tests.  Calling it in tearDown() doesn't hurt anything, but
> protects
> > test developers who try to subvert the JUnit test lifecycle stuff.
>
> Exactly.  I don't think its necessary in the teardown but it won't
> hurt.  Its *defnitely* needed in the setup for the reason you
> mentioned.
>
> > Craig
>
> Sean
>



--
http://apress.com/book/bookDisplay.html?bID=10044
Author: Pro JSF and Ajax: Building Rich Internet Components, Apress

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Sean Schofield <se...@gmail.com>]

> The call is required in the setUp() method to make things work correctly on
> the *first* test, when you have the MyFaces implementation in the classpath
> of the tests.  Calling it in tearDown() doesn't hurt anything, but protects
> test developers who try to subvert the JUnit test lifecycle stuff.

Exactly.  I don't think its necessary in the teardown but it won't
hurt.  Its *defnitely* needed in the setup for the reason you
mentioned.

> Craig

Sean

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Craig McClanahan <cr...@apache.org>]

On 2/19/06, Adam Winer <aw...@gmail.com> wrote:
>
> Purely speaking, FactoryFinder.releaseFactories() should
> only be necessary in tearDown() - which is where it makes
> the most sense, since releasing factories is cleanup, which
> is the job of tearDown().
>
> Practically speaking, it's simplest and safest to call it
> in both methods - which means that your test is
> doing the right thing (by calling it in tearDown() to clean up
> properly), and is guarding against other tests doing
> the wrong thing (by calling it in setUp()).  If all tests
> followed the rules, you'd only need to call it in one,
> but you're better off not assuming that all tests will.


Good point ... I'll go ahead and add it to the tearDown() method as well.
It'll be in the 20060220 nightly build.  Paranoid programming rulez :-)

The call is required in the setUp() method to make things work correctly on
the *first* test, when you have the MyFaces implementation in the classpath
of the tests.  Calling it in tearDown() doesn't hurt anything, but protects
test developers who try to subvert the JUnit test lifecycle stuff.


-- Adam


Craig

On 2/19/06, Craig McClanahan <cr...@apache.org> wrote:
> >
> >
> >
> > On 2/19/06, Dennis Byrne <de...@dbyrne.net> wrote:
> > > >I updated the Shale snapshots about half an hour ago... coincidence?
> > > >Wendy
> > >
> > > This would have happened w/ or w/out your update.
> > >
> > > The fix for this was to put FactoryFinder.releaseFactories() in
> setup()
> > and in tearDown() .  This setup() call is to clear configured Factory
> info
> > from previous tests ( which taints the first test of the current running
> > TestCase ).  The tearDown() call is to prevent interference w/
> subsequent
> > TestCases.
> > >
> > > In an offlist conversation, Sean and I recently came to the agreement
> that
> > AbstractJsfTestCase should be calling FactoryFinder.releaseFactories() .
> > This is important because AbstractJsfTestCase.setup() configures each
> > Factory, and AFAIK, it was interfering w/ his tests, which were children
> of
> > AbstractJsfTestCase.  Sean ended up putting this in
> > AbstractJsfTestCase.setup(), probably for the same reason it needs to
> now go
> > in some of ours.
> > >
> > > @ Sean or Wendy or Craig - can one of you please put
> > FactoryFinder.releaseFactories() in AbstractJsfTestCase.tearDown() also
> ?
> >
> > In the current code, FactoryFinder.releaseFactories() is called at the
> > *beginning* of the setUp() method.  Doesn't that make calling it in
> > tearDown() redundant, because every test (including the first one) is
> going
> > to call it as part of the setup process anyway?
> >
> > > Dennis Byrne
> > >
> > >
> > >
> > Craig
> >
> >
>

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Adam Winer <aw...@gmail.com>]

Purely speaking, FactoryFinder.releaseFactories() should
only be necessary in tearDown() - which is where it makes
the most sense, since releasing factories is cleanup, which
is the job of tearDown().

Practically speaking, it's simplest and safest to call it
in both methods - which means that your test is
doing the right thing (by calling it in tearDown() to clean up
properly), and is guarding against other tests doing
the wrong thing (by calling it in setUp()).  If all tests
followed the rules, you'd only need to call it in one,
but you're better off not assuming that all tests will.

-- Adam

On 2/19/06, Craig McClanahan <cr...@apache.org> wrote:
>
>
>
> On 2/19/06, Dennis Byrne <de...@dbyrne.net> wrote:
> > >I updated the Shale snapshots about half an hour ago... coincidence?
> > >Wendy
> >
> > This would have happened w/ or w/out your update.
> >
> > The fix for this was to put FactoryFinder.releaseFactories() in setup()
> and in tearDown() .  This setup() call is to clear configured Factory info
> from previous tests ( which taints the first test of the current running
> TestCase ).  The tearDown() call is to prevent interference w/ subsequent
> TestCases.
> >
> > In an offlist conversation, Sean and I recently came to the agreement that
> AbstractJsfTestCase should be calling FactoryFinder.releaseFactories() .
> This is important because AbstractJsfTestCase.setup() configures each
> Factory, and AFAIK, it was interfering w/ his tests, which were children of
> AbstractJsfTestCase.  Sean ended up putting this in
> AbstractJsfTestCase.setup(), probably for the same reason it needs to now go
> in some of ours.
> >
> > @ Sean or Wendy or Craig - can one of you please put
> FactoryFinder.releaseFactories() in AbstractJsfTestCase.tearDown() also ?
>
> In the current code, FactoryFinder.releaseFactories() is called at the
> *beginning* of the setUp() method.  Doesn't that make calling it in
> tearDown() redundant, because every test (including the first one) is going
> to call it as part of the setup process anyway?
>
> > Dennis Byrne
> >
> >
> >
> Craig
>
>

Re: FactoryFinder.releaseFactories() - was [continuum] BUILD FAILURE: API

[Craig McClanahan <cr...@apache.org>]

On 2/19/06, Dennis Byrne <de...@dbyrne.net> wrote:
>
> >I updated the Shale snapshots about half an hour ago... coincidence?
> >Wendy
>
> This would have happened w/ or w/out your update.
>
> The fix for this was to put FactoryFinder.releaseFactories() in setup()
> and in tearDown() .  This setup() call is to clear configured Factory info
> from previous tests ( which taints the first test of the current running
> TestCase ).  The tearDown() call is to prevent interference w/ subsequent
> TestCases.
>
> In an offlist conversation, Sean and I recently came to the agreement that
> AbstractJsfTestCase should be calling FactoryFinder.releaseFactories()
> .  This is important because AbstractJsfTestCase.setup() configures each
> Factory, and AFAIK, it was interfering w/ his tests, which were children of
> AbstractJsfTestCase.  Sean ended up putting this in
> AbstractJsfTestCase.setup(), probably for the same reason it needs to now
> go in some of ours.
>
> @ Sean or Wendy or Craig - can one of you please put
> FactoryFinder.releaseFactories() in AbstractJsfTestCase.tearDown() also ?


In the current code, FactoryFinder.releaseFactories() is called at the
*beginning* of the setUp() method.  Doesn't that make calling it in
tearDown() redundant, because every test (including the first one) is going
to call it as part of the setup process anyway?

Dennis Byrne
>
>
> Craig