Hi,
> I think I know why this works for you. Are you by any chance running
> phpunit only on that one test class?
no, only in dev mode i run single tests. But before committing i am
running mvn test AND phpunit . for all tests.
It still works for me. But the running order might be different on
different os. I heard from some other guy who has this failure.
> It took me a little while, but I know why this happens. The test which
> fails uses Logger::resetConfiguration() to clear the previous settings
> and appenders. However, the resetConfiguration() method does not reset
> the LoggerAppenderPool. Next time configure() and initialize() is
> called, instead of creating new appenders, the pool will use existing
> ones. In this case, the htmlLineBreaks option is left turned on by a
> previous test which uses the same appender name, and that is why the
> test fails.
>
> Just to prove the point, I implemented a method to clear the appender
> pool (LoggerAppenderPool::clear()). When I edit
> LoggerRendererMapTest::testUsage(), and add
> LoggerAppenderPool::clear() before applying the new configuration, the
> test passes as expected.
>
> In conclusion, i think that:
> * LoggerAppenderPool should have a clear() method
> * this method should be called by Logger::resetConfiguration()
>
> If you have the time, have a look at this problem, I'd like to finish
> the echo appender issue first.
Interesting. I looked into the code and think we should do something here.
First, LoggerAppenderPool is only used by Configurators, I think we
should move it in this package and not on leave it in the root
package.
Second, I don't see that LoggerAppenderPool needs to be statically
available. If we create a new LoggerAppenderPool object in each
configurators constructor and use the object then this should be
solved without any additional reset calls or clear methods.
What do you think?
Like this:
public function configure(LoggerHierarchy $hierarchy, $url = '') {
$pool = new LoggerAppenderPool();
$pool->getAppenderFromPool("name");
Cheers
Christian
