You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Brian Stansberry <be...@yahoo.com> on 2005/05/30 06:17:20 UTC

[logging] LoadTest

I've been testing my new implementation of
LogFactoryImpl and it's looking good except it fails
the "testInContainer" test of o.a.c.l.LoadTest.  This
test sets up a parent-last (aka child-first)
classloader hierarchy that has JCL and a class that
calls JCL in the child.  JCL is also visible in the
parent.  It then performs various tests, setting the
thread context class loader to various values and
trying to log.

This test is designed to fail if logging succeeds when
the TCCL is set to the system classloader or the
parent classloader.  

The old implementation of LogFactoryImpl does throw a
LogConfigurationException in this situation, as
discovery finds an adapter in the parent that is
binary incompatible with the LogFactoryImpl in the
child.

The new implementation does not throw an LCE, as it
detects the incompatibility and retries loading the
adapter using the LogFactoryImpl's classloader.  But,
because logging succeeds, this unit test does not
pass.

It is my interpretation that this unit test is really
checking for *consistent* behavior of the old
implementation, not for *correct* behavior.  It would
catch things like a change to the old implementation
that caused it to forget to try the TCCL.  But IMHO an
implementation that was specifically designed to
handle this situation shouldn't cause a unit test
failure.

Any thoughts on this one?  Have I missed something?

BTW, LoadTest is not invoked by the ant test target.

Regards,

Brian



		
__________________________________ 
Do you Yahoo!? 
Yahoo! Small Business - Try our new Resources site
http://smallbusiness.yahoo.com/resources/

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] LoadTest

Posted by robert burrell donkin <rd...@apache.org>.
On Mon, 2005-05-30 at 18:40 +1200, Simon Kitching wrote:
> On Sun, 2005-05-29 at 21:17 -0700, Brian Stansberry wrote:
> > I've been testing my new implementation of
> > LogFactoryImpl and it's looking good except it fails
> > the "testInContainer" test of o.a.c.l.LoadTest.  This
> > test sets up a parent-last (aka child-first)
> > classloader hierarchy that has JCL and a class that
> > calls JCL in the child.  JCL is also visible in the
> > parent.  It then performs various tests, setting the
> > thread context class loader to various values and
> > trying to log.
> > 
> > This test is designed to fail if logging succeeds when
> > the TCCL is set to the system classloader or the
> > parent classloader.  
> > 
> > The old implementation of LogFactoryImpl does throw a
> > LogConfigurationException in this situation, as
> > discovery finds an adapter in the parent that is
> > binary incompatible with the LogFactoryImpl in the
> > child.
> 
> I suspect it actually fails because LogFactoryImpl finds Log4JLogger in
> the parent, but log4j classes are not in the parent. 

i think that this may depend on the classpath used to run the test.

> It is a fundamental
> implication of the logging design that the XXLogger must be able to load
> the library it maps to via normal java classloader lookup (ie no using
> the context classloader). It's not "binary incompatibility", I think,
> but a complete failure to link Log4JLogger to its referenced classes.

i suspect that this is a test for exotic context classloader
configurations. i think that the mode of failure depends on the system
classpath used to run the test. (for example, maybe it was intended that
log4j would be defined by the system classloader) unless the intended
configuration is known, i think it'd be hard to work out the exact
diagnosis...

> > The new implementation does not throw an LCE, as it
> > detects the incompatibility and retries loading the
> > adapter using the LogFactoryImpl's classloader.  But,
> > because logging succeeds, this unit test does not
> > pass.
> > 
> > It is my interpretation that this unit test is really
> > checking for *consistent* behavior of the old
> > implementation, not for *correct* behavior.  It would
> > catch things like a change to the old implementation
> > that caused it to forget to try the TCCL.  But IMHO an
> > implementation that was specifically designed to
> > handle this situation shouldn't cause a unit test
> > failure.
> 
> I agree that the test is not relevant to your new code. It was really
> documenting the behaviour of the old system, and could be updated.

i suspect that the majority opinion was that JCL should refuse to run in
containers which didn't obey the rules. therefore, this was a legitimate
test. however, the current consensus is that (with hindsight) this
original decision was wrong. it was a semantic bug. i really cannot
think that any code could reasonably rely on JCL throwing a runtime
exception whenever exotic context classloader configurations are
encountered.

i agree that it should be updated. 

> An updated test really ought to verify that when context=system, logging
> falls back to jdk14 (when running with java14) or to SimpleLog. But if
> this is too much work, just asserting that logging still works would be
> ok by me.

+1

> > BTW, LoadTest is not invoked by the ant test target.
> 
> Then that should probably be fixed too. Nicely spotted!

+1

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] LoadTest

Posted by Simon Kitching <sk...@apache.org>.
On Sun, 2005-05-29 at 21:17 -0700, Brian Stansberry wrote:
> I've been testing my new implementation of
> LogFactoryImpl and it's looking good except it fails
> the "testInContainer" test of o.a.c.l.LoadTest.  This
> test sets up a parent-last (aka child-first)
> classloader hierarchy that has JCL and a class that
> calls JCL in the child.  JCL is also visible in the
> parent.  It then performs various tests, setting the
> thread context class loader to various values and
> trying to log.
> 
> This test is designed to fail if logging succeeds when
> the TCCL is set to the system classloader or the
> parent classloader.  
> 
> The old implementation of LogFactoryImpl does throw a
> LogConfigurationException in this situation, as
> discovery finds an adapter in the parent that is
> binary incompatible with the LogFactoryImpl in the
> child.

I suspect it actually fails because LogFactoryImpl finds Log4JLogger in
the parent, but log4j classes are not in the parent. It is a fundamental
implication of the logging design that the XXLogger must be able to load
the library it maps to via normal java classloader lookup (ie no using
the context classloader). It's not "binary incompatibility", I think,
but a complete failure to link Log4JLogger to its referenced classes.

> The new implementation does not throw an LCE, as it
> detects the incompatibility and retries loading the
> adapter using the LogFactoryImpl's classloader.  But,
> because logging succeeds, this unit test does not
> pass.
> 
> It is my interpretation that this unit test is really
> checking for *consistent* behavior of the old
> implementation, not for *correct* behavior.  It would
> catch things like a change to the old implementation
> that caused it to forget to try the TCCL.  But IMHO an
> implementation that was specifically designed to
> handle this situation shouldn't cause a unit test
> failure.

I agree that the test is not relevant to your new code. It was really
documenting the behaviour of the old system, and could be updated.

An updated test really ought to verify that when context=system, logging
falls back to jdk14 (when running with java14) or to SimpleLog. But if
this is too much work, just asserting that logging still works would be
ok by me.

> BTW, LoadTest is not invoked by the ant test target.

Then that should probably be fixed too. Nicely spotted!

Regards,

Simon



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org