Hi, thanks very much for the patch! I will check it out the next days and apply. I am very happy with that because i am on a mac box and I think the others use mostly linux.
However, for legal reasons I would appreciate if you could simply create a new issue in jira and attach your patch there too. https://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&mode=hide&sorter/order=DESC&sorter/field=priority&resolution=-1&pid=12310690&fixfor=12313917 Reason is, there is a box you'll need to check that you allow us to use your work. Thanks in advance! Christian On Thu, Jan 28, 2010 at 9:18 AM, Tommy Montgomery <[email protected]> wrote: > Hi there, > > I recently noticed that this project has been pulled from the death trap > known as PHP 4, which makes me very happy. I also noticed the cry for help > for unit tests, so I thought I'd show some gratitude and write a few tests. > Unfortunately, a few of the tests are linux-specific, so I patched them up > and made them os-agnostic. Attached is the patch. > > Mostly it was using PHP_EOL instead of hardcoding \n. A few of the PDO > appender tests used posix_* functions, so I just made those conditional. I > also made the tabs/spaces consistent on a few files that I touched, because > that kind of stuff drives me crazy. :) > > Thanks, > Tommy > > Index: src/test/php/appenders/LoggerAppenderEchoTest.php > =================================================================== > --- src/test/php/appenders/LoggerAppenderEchoTest.php (revision 903977) > +++ src/test/php/appenders/LoggerAppenderEchoTest.php (working copy) > @@ -24,7 +24,7 @@ > */ > > class LoggerAppenderEchoTest extends PHPUnit_Framework_TestCase { > - > + > public function testEcho() { > $appender = new LoggerAppenderEcho("myname "); > > @@ -32,14 +32,13 @@ > $appender->setLayout($layout); > $appender->activateOptions(); > $event = new LoggerLoggingEvent("LoggerAppenderEchoTest", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > - > + > + $expected = "ERROR - testmessage" . PHP_EOL; > ob_start(); > $appender->append($event); > - $v = ob_get_contents(); > - ob_end_clean(); > + $actual = ob_get_clean(); > > - $e = "ERROR - testmessage\n"; > - self::assertEquals($v, $e); > - } > - > + self::assertEquals($expected, $actual); > + } > + > } > Index: src/test/php/appenders/LoggerAppenderPDOTest.php > =================================================================== > --- src/test/php/appenders/LoggerAppenderPDOTest.php (revision 903977) > +++ src/test/php/appenders/LoggerAppenderPDOTest.php (working copy) > @@ -29,6 +29,10 @@ > > /** To start with an empty database for each single test. */ > public function setUp() { > + if(!extension_loaded('pdo_sqlite')) { > + self::markTestSkipped("Please install 'pdo_sqlite' in order to > run this test"); > + } > + > if (file_exists(self::file)) unlink(self::file); > } > > @@ -39,12 +43,8 @@ > > /** Tests new-style logging using prepared statements and the default > SQL definition. */ > public function testSimpleWithDefaults() { > - if(!extension_loaded('pdo_sqlite')) { > - self::markTestSkipped("Please install 'pdo_sqlite' > in order to run this test"); > - } > - > // Log event > - $event = new LoggerLoggingEvent("LoggerAppenderPDOTest", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > + $event = new LoggerLoggingEvent("LoggerAppenderPDOTest", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > $appender = new LoggerAppenderPDO("myname"); > $appender->setDSN(self::dsn); > $appender->activateOptions(); > @@ -62,7 +62,9 @@ > self::assertEquals('TEST', $row[1]); // %c = category > self::assertEquals('ERROR', $row[2]); // %p = priority > self::assertEquals('testmessage', $row[3]); // %m = message > - self::assertEquals(posix_getpid(), $row[4]); // %t = thread > + if (function_exists('posix_getpid')) { > + self::assertEquals(posix_getpid(), $row[4]); // %t = thread > + } > self::assertEquals('NA', $row[5]); // %F = file, NA due to phpunit > magic > self::assertEquals('NA', $row[6]); // %L = line, NA due to phpunit > magic > } > @@ -70,22 +72,18 @@ > > /** Tests new style prepared statment logging with customized SQL. */ > public function testCustomizedSql() { > - if(!extension_loaded('pdo_sqlite')) { > - self::markTestSkipped("Please install 'pdo_sqlite' in order to > run this test"); > - } > - > // Prepare appender > - $appender = new LoggerAppenderPDO("myname"); > + $appender = new LoggerAppenderPDO("myname"); > $appender->setDSN(self::dsn); > $appender->setTable('unittest2'); > $appender->setInsertSql("INSERT INTO unittest2 (file, line, thread, > timestamp, logger, level, message) VALUES (?,?,?,?,?,?,?)"); > $appender->setInsertPattern("%F,%L,%t,%d,%c,%p,%m"); > - $appender->activateOptions(); > + $appender->activateOptions(); > > // Action! > $event = new LoggerLoggingEvent("LoggerAppenderPDOTest2", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > - $appender->append($event); > - > + $appender->append($event); > + > // Check > $db = new PDO(self::dsn); > $result = $db->query("SELECT * FROM unittest2"); > @@ -93,19 +91,17 @@ > self::assertTrue(is_object($row)); > self::assertEquals("NA", $row->file); // "NA" due to phpunit magic > self::assertEquals("NA", $row->line); // "NA" due to phpunit magic > - self::assertEquals(posix_getpid(), $row->thread); > + if (function_exists('posix_getpid')) { > + self::assertEquals(posix_getpid(), $row->thread); > + } > self::assertEquals(1, preg_match('/^\d\d\d\d-\d\d-\d\d > \d\d:\d\d:\d\d.\d\d\d$/', $row->timestamp)); > self::assertEquals('TEST', $row->logger); > self::assertEquals('ERROR', $row->level); > self::assertEquals('testmessage', $row->message); > } > - > + > /** Tests old-style logging using the $sql variable. */ > public function testOldStyle() { > - if(!extension_loaded('pdo_sqlite')) { > - self::markTestSkipped("Please install 'pdo_sqlite' in order to > run this test"); > - } > - > // Create table with different column order > $db = new PDO(self::dsn); > $db->exec('CREATE TABLE unittest3 (ts timestamp, level varchar(32), > msg varchar(64))'); > @@ -141,5 +137,5 @@ > $appender->setDSN($dsn); > $appender->setCreateTable(true); > $appender->activateOptions(); > - } > + } > } > Index: src/test/php/appenders/LoggerAppenderRollingFileTest.php > =================================================================== > --- src/test/php/appenders/LoggerAppenderRollingFileTest.php (revision > 903977) > +++ src/test/php/appenders/LoggerAppenderRollingFileTest.php (working > copy) > @@ -6,16 +6,16 @@ > * The ASF licenses this file to You under the Apache License, Version 2.0 > * (the "License"); you may not use this file except in compliance with > * the License. You may obtain a copy of the License at > - * > + * > * http://www.apache.org/licenses/LICENSE-2.0 > - * > + * > * Unless required by applicable law or agreed to in writing, software > * distributed under the License is distributed on an "AS IS" BASIS, > * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > * See the License for the specific language governing permissions and > * limitations under the License. > - * > - * @category tests > + * > + * @category tests > * @package log4php > * @subpackage appenders > * @license http://www.apache.org/licenses/LICENSE-2.0 Apache License, > Version 2.0 > @@ -24,100 +24,102 @@ > */ > > class LoggerAppenderRollingFileTest extends PHPUnit_Framework_TestCase { > - /** Directory for temporary files. > - * > - * @var string > - */ > - private $dir; > - > - protected function setUp() { > - $this->dir = dirname(__FILE__) . '/../../../../target/phpunit'; > - �...@mkdir($this->dir); > - �...@unlink($this->dir.'/TEST-rolling.txt'); > - @unlink($this->dir.'/TEST-rolling.txt.1'); > - �...@unlink($this->dir.'/TEST-rolling.txt.2'); > - } > - > - public function testMaxFileSize() { > - $appender = new LoggerAppenderRollingFile("mylogger"); > - > - $e = $appender->setMaxFileSize('1KB'); > - self::assertEquals($e, '1024'); > - > - $e = $appender->setMaxFileSize('2KB'); > - self::assertEquals($e, '2048'); > - > - $e = $appender->setMaxFileSize('1MB'); > - self::assertEquals($e, '1048576'); > - > - $e = $appender->setMaxFileSize('3MB'); > - self::assertEquals($e, '3145728'); > - > - $e = $appender->setMaxFileSize('1GB'); > - self::assertEquals($e, '1073741824'); > - > - $e = $appender->setMaxFileSize('10000'); > - self::assertEquals($e, '10000'); > - > - $e = $appender->setMaxFileSize('BLUB'); > - self::assertEquals($e, '10000'); > - > - $e = $appender->setMaxFileSize('100.5'); > - self::assertEquals($e, '100'); > - > - $e = $appender->setMaxFileSize('1000.6'); > - self::assertEquals($e, '1000'); > - > - $e = $appender->setMaxFileSize('1.5MB'); > - self::assertEquals($e, '1048576'); > - } > - > - public function testSetFileName() { > - $appender = new LoggerAppenderRollingFile("mylogger"); > - > $appender->setFileName($this->dir.'/../././phpunit/doesnotexist.log'); > - $expandedFileName = self::readAttribute($appender, > 'expandedFileName'); > - self::assertEquals(1, > preg_match('/\/target\/phpunit\/doesnotexist.log$/', $expandedFileName)); > - } > - > - public function testSimpleLogging() { > - $layout = new LoggerLayoutSimple(); > - > - $appender = new LoggerAppenderRollingFile("mylogger"); > + /** Directory for temporary files. > + * > + * @var string > + */ > + private $dir; > + > + protected function setUp() { > + $this->dir = dirname(__FILE__) . > '/../../../../target/phpunit'; > + @mkdir($this->dir); > + @unlink($this->dir.'/TEST-rolling.txt'); > + @unlink($this->dir.'/TEST-rolling.txt.1'); > + @unlink($this->dir.'/TEST-rolling.txt.2'); > + } > + > + public function testMaxFileSize() { > + $appender = new LoggerAppenderRollingFile("mylogger"); > + > + $e = $appender->setMaxFileSize('1KB'); > + self::assertEquals($e, '1024'); > + > + $e = $appender->setMaxFileSize('2KB'); > + self::assertEquals($e, '2048'); > + > + $e = $appender->setMaxFileSize('1MB'); > + self::assertEquals($e, '1048576'); > + > + $e = $appender->setMaxFileSize('3MB'); > + self::assertEquals($e, '3145728'); > + > + $e = $appender->setMaxFileSize('1GB'); > + self::assertEquals($e, '1073741824'); > + > + $e = $appender->setMaxFileSize('10000'); > + self::assertEquals($e, '10000'); > + > + $e = $appender->setMaxFileSize('BLUB'); > + self::assertEquals($e, '10000'); > + > + $e = $appender->setMaxFileSize('100.5'); > + self::assertEquals($e, '100'); > + > + $e = $appender->setMaxFileSize('1000.6'); > + self::assertEquals($e, '1000'); > + > + $e = $appender->setMaxFileSize('1.5MB'); > + self::assertEquals($e, '1048576'); > + } > + > + public function testSetFileName() { > + $appender = new LoggerAppenderRollingFile("mylogger"); > + > $appender->setFileName($this->dir.'/../././phpunit/doesnotexist.log'); > + $expandedFileName = self::readAttribute($appender, > 'expandedFileName'); > + > + $expectedFilePattern = '/' . > implode(preg_quote(DIRECTORY_SEPARATOR, '/'), array('target', 'phpunit', > 'doesnotexist\.log')) . '$/'; > + self::assertEquals(preg_match($expectedFilePattern, > $expandedFileName), 1, $expandedFileName); > + } > + > + public function testSimpleLogging() { > + $layout = new LoggerLayoutSimple(); > + > + $appender = new LoggerAppenderRollingFile("mylogger"); > $appender->setFileName($this->dir.'/TEST-rolling.txt'); > $appender->setLayout($layout); > $appender->setMaximumFileSize('1KB'); > $appender->setMaxBackupIndex(2); > $appender->activateOptions(); > - > - $event = new LoggerLoggingEvent('LoggerAppenderFileTest', > - new > Logger('mycategory'), > - > LoggerLevel::getLevelWarn(), > - "my > message123"); > - $i = 0; > - $b = true; > - while($b) { > - if($i == 1000) { > - $b = false; > - } > - $appender->append($event); > - $i++; > - } > - > - $event = new LoggerLoggingEvent('LoggerAppenderFileTest', > - new > Logger('mycategory'), > - > LoggerLevel::getLevelWarn(), > - "my > messageXYZ"); > - > - $appender->append($event); > - > + > + $event = new LoggerLoggingEvent('LoggerAppenderFileTest', > + > new Logger('mycategory'), > + > LoggerLevel::getLevelWarn(), > + > "my message123"); > + $i = 0; > + $b = true; > + while($b) { > + if($i == 1000) { > + $b = false; > + } > + $appender->append($event); > + $i++; > + } > + > + $event = new LoggerLoggingEvent('LoggerAppenderFileTest', > + > new Logger('mycategory'), > + > LoggerLevel::getLevelWarn(), > + > "my messageXYZ"); > + > + $appender->append($event); > + > $appender->close(); > - > + > $file = $this->dir.'/TEST-rolling.txt'; > $data = file($file); > $line = $data[count($data)-1]; > $e = "WARN - my messageXYZ".PHP_EOL; > self::assertEquals($e, $line); > - > + > $file = $this->dir.'/TEST-rolling.txt.1'; > $data = file($file); > $line = $data[count($data)-1]; > @@ -125,7 +127,7 @@ > foreach($data as $r) { > self::assertEquals($e, $r); > } > - > + > $file = $this->dir.'/TEST-rolling.txt.2'; > $data = file($file); > $line = $data[count($data)-1]; > @@ -133,17 +135,17 @@ > foreach($data as $r) { > self::assertEquals($e, $r); > } > - > + > if(file_exists($this->dir.'/TEST-rolling.txt.3')) { > - self::assertTrue(false); > + self::assertTrue(false); > } > - } > - > - protected function tearDown() { > - @unlink($this->dir.'/TEST-rolling.txt'); > - @unlink($this->dir.'/TEST-rolling.txt.1'); > - @unlink($this->dir.'/TEST-rolling.txt.2'); > - �...@rmdir($this->dir); > - } > - > + } > + > + protected function tearDown() { > + @unlink($this->dir.'/TEST-rolling.txt'); > + @unlink($this->dir.'/TEST-rolling.txt.1'); > + @unlink($this->dir.'/TEST-rolling.txt.2'); > + @rmdir($this->dir); > + } > + > } > Index: src/test/php/LoggerAppenderTest.php > =================================================================== > --- src/test/php/LoggerAppenderTest.php (revision 903977) > +++ src/test/php/LoggerAppenderTest.php (working copy) > @@ -40,7 +40,7 @@ > $appender->doAppend($event); > $v = ob_get_contents(); > ob_end_clean(); > - $e = "FATAL - testmessage\n"; > + $e = "FATAL - testmessage" . PHP_EOL; > self::assertEquals($e, $v); > > $event = new LoggerLoggingEvent("LoggerAppenderEchoTest", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > @@ -48,7 +48,7 @@ > $appender->doAppend($event); > $v = ob_get_contents(); > ob_end_clean(); > - $e = "ERROR - testmessage\n"; > + $e = "ERROR - testmessage" . PHP_EOL; > self::assertEquals($e, $v); > > $event = new LoggerLoggingEvent("LoggerAppenderEchoTest", new > Logger("TEST"), LoggerLevel::getLevelWarn(), "testmessage"); > @@ -56,7 +56,7 @@ > $appender->doAppend($event); > $v = ob_get_contents(); > ob_end_clean(); > - $e = "WARN - testmessage\n"; > + $e = "WARN - testmessage" . PHP_EOL; > self::assertEquals($e, $v); > > $event = new LoggerLoggingEvent("LoggerAppenderEchoTest", new > Logger("TEST"), LoggerLevel::getLevelInfo(), "testmessage"); > Index: src/test/php/renderers/LoggerRendererMapTest.php > =================================================================== > --- src/test/php/renderers/LoggerRendererMapTest.php (revision 903977) > +++ src/test/php/renderers/LoggerRendererMapTest.php (working copy) > @@ -79,6 +79,6 @@ > $v = ob_get_contents(); > ob_end_clean(); > > - self::assertEquals("ERROR - test1,test2,test3\n", $v); > + self::assertEquals("ERROR - test1,test2,test3" . PHP_EOL, $v); > } > } > Index: src/test/php/layouts/LoggerLayoutXmlTest.php > =================================================================== > --- src/test/php/layouts/LoggerLayoutXmlTest.php (revision 903977) > +++ src/test/php/layouts/LoggerLayoutXmlTest.php (working copy) > @@ -36,7 +36,7 @@ > > "<log4php:message><![CDATA[testmessage]]></log4php:message>".PHP_EOL. > "<log4php:locationInfo class=\"LoggerLoggingEvent\" > file=\"NA\" line=\"NA\" " . > "method=\"getLocationInformation\" />".PHP_EOL. > - "</log4php:event>\n".PHP_EOL; > + "</log4php:event>".PHP_EOL . PHP_EOL; > > self::assertEquals($v, $e); > } > @@ -52,7 +52,7 @@ > > "<log4php:message><![CDATA[testmessage]]></log4php:message>".PHP_EOL. > "<log4php:locationInfo class=\"LoggerLoggingEvent\" > file=\"NA\" line=\"NA\" " . > "method=\"getLocationInformation\" />".PHP_EOL. > - "</log4php:event>\n".PHP_EOL; > + "</log4php:event>".PHP_EOL . PHP_EOL; > > self::assertEquals($v, $e); > } > Index: src/test/php/layouts/LoggerLayoutSimpleTest.php > =================================================================== > --- src/test/php/layouts/LoggerLayoutSimpleTest.php (revision 903977) > +++ src/test/php/layouts/LoggerLayoutSimpleTest.php (working copy) > @@ -24,14 +24,14 @@ > */ > > class LoggerLayoutSimpleTest extends PHPUnit_Framework_TestCase { > - > + > public function testSimpleLayout() { > $event = new LoggerLoggingEvent("LoggerLayoutSimpleTest", new > Logger("TEST"), LoggerLevel::getLevelError(), "testmessage"); > > $layout = new LoggerLayoutSimple(); > - $v = $layout->format($event); > - $e = "ERROR - testmessage\n"; > - self::assertEquals($v, $e); > - } > - > + $actual = $layout->format($event); > + $expected = "ERROR - testmessage" . PHP_EOL; > + self::assertEquals($expected, $actual); > + } > + > } > Index: src/main/php/appenders/LoggerAppenderEcho.php > =================================================================== > --- src/main/php/appenders/LoggerAppenderEcho.php (revision 903977) > +++ src/main/php/appenders/LoggerAppenderEcho.php (working copy) > @@ -43,17 +43,17 @@ > class LoggerAppenderEcho extends LoggerAppender { > /** boolean used internally to mark first append */ > private $firstAppend = true; > - > + > public function __construct($name = '') { > - parent::__construct($name); > - $this->requiresLayout = true; > - $this->firstAppend = true; > + parent::__construct($name); > + $this->requiresLayout = true; > + $this->firstAppend = true; > } > > public function __destruct() { > - $this->close(); > - } > - > + $this->close(); > + } > + > public function activateOptions() { > $this->closed = false; > } > @@ -64,7 +64,7 @@ > echo $this->layout->getFooter(); > } > } > - $this->closed = true; > + $this->closed = true; > } > > public function append(LoggerLoggingEvent $event) { > Index: src/main/php/appenders/LoggerAppenderRollingFile.php > =================================================================== > --- src/main/php/appenders/LoggerAppenderRollingFile.php (revision > 903977) > +++ src/main/php/appenders/LoggerAppenderRollingFile.php (working > copy) > @@ -151,7 +151,7 @@ > // realpath() fails if the argument does not exist so the > filename is separated. > $this->expandedFileName = realpath(dirname($fileName)); > if ($this->expandedFileName === false) throw new > Exception("Directory of $fileName does not exist!"); > - $this->expandedFileName .= '/'.basename($fileName); > + $this->expandedFileName .= DIRECTORY_SEPARATOR . > basename($fileName); > } > > > >
