[jira] Commented: (LOG4PHP-102) LoggerLayoutPattern fails tests

["Ivan Habunek (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LOG4PHP-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12845084#action_12845084 ] 

Ivan Habunek commented on LOG4PHP-102:
--------------------------------------

I am contributing two possible solutions to this problem (both fix the above issue):

The first solution is to rewrite the code to use PHP string manipulation methods, as stated in a @todo. This makes the spacePad() method obsolete (and with it the global variable holding spaces) and so fixes the problem. Also, the code is much shorter and easier to read. However, I did some basic testing, and it seems that the str_pad method is not as fast as the spacePad() method. This patch can be found in LoggerPatternConverter.patch.rewrite.txt

The second solution is to use a local member variable to hold the spaces array instead of a global variable (it is not used anywhere else in the project). This solution has no performance impact. The patch can be found in LoggerPatternConverter.patch.quickfix.txt

Personally, I would prefer to use the first solution, because it is neater, but I don't have a good way to measure how this will affect overall performance of log4php. Can any of the project developers take a look at this issue?

I am also contributing a new set of tests, because the ones currently in LoggerLayoutPatternTest class are a bit thin. The new tests include a more complicated pattern (as requested in a TODO) and testing of all log levels. The patch can be found in LoggerLayoutPatternTest.patch.txt 

> LoggerLayoutPattern fails tests
> -------------------------------
>
>                 Key: LOG4PHP-102
>                 URL: https://issues.apache.org/jira/browse/LOG4PHP-102
>             Project: Log4php
>          Issue Type: Bug
>          Components: Code
>    Affects Versions: 2.0
>            Reporter: Ivan Habunek
>            Priority: Minor
>         Attachments: LoggerLayoutPatternTest.patch.txt, LoggerPatternConverter.patch.quickfix.txt, LoggerPatternConverter.patch.rewrite.txt
>
>
> In LoggerLayoutPatternTest class, the testWarnLayout() method is commented out because it fails with the following message:
> 1) testWarnLayout(LoggerLayoutPatternTest)
> Undefined index:  log4php.LoggerPatternConverter.spaces
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/helpers/LoggerPatternConverter.php:131
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/helpers/LoggerPatternConverter.php:104
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/layouts/LoggerPatternLayout.php:216
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/test/php/layouts/LoggerLayoutPatternTest.php:45
> This seems to be related to using a global variable $GLOBALS['log4php.LoggerPatternConverter.spaces'] in LoggerPatternConverter::spacePad(). I am unsure why the global variable is not accessible in this test, but using a global variable seems the wrong approach here anyway.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.