Merge "Use MediaWikiTestCase methods for tempdir in unit tests"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 12 Feb 2015 07:57:04 +0000 (07:57 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 12 Feb 2015 07:57:04 +0000 (07:57 +0000)
tests/phpunit/MediaWikiTestCase.php
tests/phpunit/includes/GlobalFunctions/GlobalTest.php
tests/phpunit/includes/api/ApiTestCaseUpload.php
tests/phpunit/includes/api/ApiUploadTest.php
tests/phpunit/includes/filebackend/FileBackendTest.php
tests/phpunit/includes/parser/NewParserTest.php
tests/phpunit/includes/upload/UploadBaseTest.php

index 2b46fc3..e49c391 100644 (file)
@@ -214,8 +214,11 @@ abstract class MediaWikiTestCase extends PHPUnit_Framework_TestCase {
 
        }
 
-       protected function tearDown() {
+       protected function addTmpFiles( $files ) {
+               $this->tmpFiles = array_merge( $this->tmpFiles, (array)$files );
+       }
 
+       protected function tearDown() {
                $this->called['tearDown'] = true;
                // Cleaning up temporary files
                foreach ( $this->tmpFiles as $fileName ) {
index 2bfabe4..331fb3b 100644 (file)
@@ -7,7 +7,7 @@ class GlobalTest extends MediaWikiTestCase {
        protected function setUp() {
                parent::setUp();
 
-               $readOnlyFile = tempnam( wfTempDir(), "mwtest_readonly" );
+               $readOnlyFile = $this->getNewTempFile();
                unlink( $readOnlyFile );
 
                $this->setMwGlobals( array(
@@ -22,16 +22,6 @@ class GlobalTest extends MediaWikiTestCase {
                ) );
        }
 
-       protected function tearDown() {
-               global $wgReadOnlyFile;
-
-               if ( file_exists( $wgReadOnlyFile ) ) {
-                       unlink( $wgReadOnlyFile );
-               }
-
-               parent::tearDown();
-       }
-
        /**
         * @dataProvider provideForWfArrayDiff2
         * @covers ::wfArrayDiff2
@@ -312,46 +302,42 @@ class GlobalTest extends MediaWikiTestCase {
         * @covers ::wfDebugMem
         */
        public function testDebugFunctionTest() {
+               $debugLogFile = $this->getNewTempFile();
 
-               global $wgDebugLogFile, $wgDebugTimestamps;
-
-               $old_log_file = $wgDebugLogFile;
-               $wgDebugLogFile = tempnam( wfTempDir(), 'mw-' );
-               # @todo FIXME: $wgDebugTimestamps should be tested
-               $old_wgDebugTimestamps = $wgDebugTimestamps;
-               $wgDebugTimestamps = false;
+               $this->setMwGlobals( array(
+                       'wgDebugLogFile' => $debugLogFile,
+                       # @todo FIXME: $wgDebugTimestamps should be tested
+                       'wgDebugTimestamps' => false
+               ) );
 
                wfDebug( "This is a normal string" );
-               $this->assertEquals( "This is a normal string\n", file_get_contents( $wgDebugLogFile ) );
-               unlink( $wgDebugLogFile );
+               $this->assertEquals( "This is a normal string\n", file_get_contents( $debugLogFile ) );
+               unlink( $debugLogFile );
 
                wfDebug( "This is nöt an ASCII string" );
-               $this->assertEquals( "This is nöt an ASCII string\n", file_get_contents( $wgDebugLogFile ) );
-               unlink( $wgDebugLogFile );
+               $this->assertEquals( "This is nöt an ASCII string\n", file_get_contents( $debugLogFile ) );
+               unlink( $debugLogFile );
 
                wfDebug( "\00305This has böth UTF and control chars\003" );
                $this->assertEquals(
                        " 05This has böth UTF and control chars \n",
-                       file_get_contents( $wgDebugLogFile )
+                       file_get_contents( $debugLogFile )
                );
-               unlink( $wgDebugLogFile );
+               unlink( $debugLogFile );
 
                wfDebugMem();
                $this->assertGreaterThan(
                        1000,
-                       preg_replace( '/\D/', '', file_get_contents( $wgDebugLogFile ) )
+                       preg_replace( '/\D/', '', file_get_contents( $debugLogFile ) )
                );
-               unlink( $wgDebugLogFile );
+               unlink( $debugLogFile );
 
                wfDebugMem( true );
                $this->assertGreaterThan(
                        1000000,
-                       preg_replace( '/\D/', '', file_get_contents( $wgDebugLogFile ) )
+                       preg_replace( '/\D/', '', file_get_contents( $debugLogFile ) )
                );
-               unlink( $wgDebugLogFile );
-
-               $wgDebugLogFile = $old_log_file;
-               $wgDebugTimestamps = $old_wgDebugTimestamps;
+               unlink( $debugLogFile );
        }
 
        /**
index 7e51339..d4d9651 100644 (file)
@@ -21,12 +21,6 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
                $this->clearFakeUploads();
        }
 
-       protected function tearDown() {
-               $this->clearTempUpload();
-
-               parent::tearDown();
-       }
-
        /**
         * Helper function -- remove files and associated articles by Title
         *
@@ -105,7 +99,7 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
         * @return bool
         */
        function fakeUploadFile( $fieldName, $fileName, $type, $filePath ) {
-               $tmpName = tempnam( wfTempDir(), "" );
+               $tmpName = $this->getNewTempFile();
                if ( !file_exists( $filePath ) ) {
                        throw new Exception( "$filePath doesn't exist!" );
                }
@@ -132,7 +126,7 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
        }
 
        function fakeUploadChunk( $fieldName, $fileName, $type, & $chunkData ) {
-               $tmpName = tempnam( wfTempDir(), "" );
+               $tmpName = $this->getNewTempFile();
                // copy the chunk data to temp location:
                if ( !file_put_contents( $tmpName, $chunkData ) ) {
                        throw new Exception( "couldn't copy chunk data to $tmpName" );
@@ -153,15 +147,6 @@ abstract class ApiTestCaseUpload extends ApiTestCase {
                );
        }
 
-       function clearTempUpload() {
-               if ( isset( $_FILES['file']['tmp_name'] ) ) {
-                       $tmp = $_FILES['file']['tmp_name'];
-                       if ( file_exists( $tmp ) ) {
-                               unlink( $tmp );
-                       }
-               }
-       }
-
        /**
         * Remove traces of previous fake uploads
         */
index 7fdefb6..b4b1bf3 100644 (file)
@@ -103,7 +103,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                try {
                        $randomImageGenerator = new RandomImageGenerator();
-                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, wfTempDir() );
+                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, $this->getNewTempDirectory() );
                } catch ( Exception $e ) {
                        $this->markTestIncomplete( $e->getMessage() );
                }
@@ -143,7 +143,6 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                // clean up
                $this->deleteFileByFilename( $fileName );
-               unlink( $filePath );
        }
 
        /**
@@ -152,7 +151,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
        public function testUploadZeroLength( $session ) {
                $mimeType = 'image/png';
 
-               $filePath = tempnam( wfTempDir(), "" );
+               $filePath = $this->getNewTempFile();
                $fileName = "apiTestUploadZeroLength.png";
 
                $this->deleteFileByFileName( $fileName );
@@ -180,7 +179,6 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                // clean up
                $this->deleteFileByFilename( $fileName );
-               unlink( $filePath );
        }
 
        /**
@@ -192,7 +190,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                try {
                        $randomImageGenerator = new RandomImageGenerator();
-                       $filePaths = $randomImageGenerator->writeImages( 2, $extension, wfTempDir() );
+                       $filePaths = $randomImageGenerator->writeImages( 2, $extension, $this->getNewTempDirectory() );
                } catch ( Exception $e ) {
                        $this->markTestIncomplete( $e->getMessage() );
                }
@@ -251,8 +249,6 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                // clean up
                $this->deleteFileByFilename( $fileName );
-               unlink( $filePaths[0] );
-               unlink( $filePaths[1] );
        }
 
        /**
@@ -264,7 +260,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                try {
                        $randomImageGenerator = new RandomImageGenerator();
-                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, wfTempDir() );
+                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, $this->getNewTempDirectory() );
                } catch ( Exception $e ) {
                        $this->markTestIncomplete( $e->getMessage() );
                }
@@ -333,7 +329,6 @@ class ApiUploadTest extends ApiTestCaseUpload {
                // clean up
                $this->deleteFileByFilename( $fileNames[0] );
                $this->deleteFileByFilename( $fileNames[1] );
-               unlink( $filePaths[0] );
        }
 
        /**
@@ -349,7 +344,7 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                try {
                        $randomImageGenerator = new RandomImageGenerator();
-                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, wfTempDir() );
+                       $filePaths = $randomImageGenerator->writeImages( 1, $extension, $this->getNewTempDirectory() );
                } catch ( Exception $e ) {
                        $this->markTestIncomplete( $e->getMessage() );
                }
@@ -417,7 +412,6 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                // clean up
                $this->deleteFileByFilename( $fileName );
-               unlink( $filePath );
        }
 
        /**
@@ -431,16 +425,14 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                $chunkSize = 1048576;
                // Download a large image file
-               // ( using RandomImageGenerator for large files is not stable )
+               // (using RandomImageGenerator for large files is not stable)
+               // @todo Don't download files from wikimedia.org
                $mimeType = 'image/jpeg';
                $url = 'http://upload.wikimedia.org/wikipedia/commons/'
                        . 'e/ed/Oberaargletscher_from_Oberaar%2C_2010_07.JPG';
-               $filePath = wfTempDir() . '/Oberaargletscher_from_Oberaar.jpg';
+               $filePath = $this->getNewTempDirectory() . '/Oberaargletscher_from_Oberaar.jpg';
                try {
-                       // Only download if the file is not avaliable in the temp location:
-                       if ( !is_file( $filePath ) ) {
-                               copy( $url, $filePath );
-                       }
+                       copy( $url, $filePath );
                } catch ( Exception $e ) {
                        $this->markTestIncomplete( $e->getMessage() );
                }
@@ -564,7 +556,5 @@ class ApiUploadTest extends ApiTestCaseUpload {
 
                // clean up
                $this->deleteFileByFilename( $fileName );
-               // don't remove downloaded temporary file for fast subquent tests.
-               //unlink( $filePath );
        }
 }
index 9558cc7..b40d2d2 100644 (file)
@@ -13,14 +13,13 @@ class FileBackendTest extends MediaWikiTestCase {
        private $multiBackend;
        /** @var FSFileBackend */
        public $singleBackend;
-       private $filesToPrune = array();
        private static $backendToUse;
 
        protected function setUp() {
                global $wgFileBackends;
                parent::setUp();
                $uniqueId = time() . '-' . mt_rand();
-               $tmpPrefix = wfTempDir() . '/filebackend-unittest-' . $uniqueId;
+               $tmpDir = $this->getNewTempDirectory();
                if ( $this->getCliArg( 'use-filebackend' ) ) {
                        if ( self::$backendToUse ) {
                                $this->singleBackend = self::$backendToUse;
@@ -51,8 +50,8 @@ class FileBackendTest extends MediaWikiTestCase {
                                'lockManager' => LockManagerGroup::singleton()->get( 'fsLockManager' ),
                                'wikiId' => wfWikiID(),
                                'containerPaths' => array(
-                                       'unittest-cont1' => "{$tmpPrefix}-localtesting-cont1",
-                                       'unittest-cont2' => "{$tmpPrefix}-localtesting-cont2" )
+                                       'unittest-cont1' => "{$tmpDir}/localtesting-cont1",
+                                       'unittest-cont2' => "{$tmpDir}/localtesting-cont2" )
                        ) );
                }
                $this->multiBackend = new FileBackendMultiWrite( array(
@@ -65,21 +64,20 @@ class FileBackendTest extends MediaWikiTestCase {
                                        'name' => 'localmultitesting1',
                                        'class' => 'FSFileBackend',
                                        'containerPaths' => array(
-                                               'unittest-cont1' => "{$tmpPrefix}-localtestingmulti1-cont1",
-                                               'unittest-cont2' => "{$tmpPrefix}-localtestingmulti1-cont2" ),
+                                               'unittest-cont1' => "{$tmpDir}/localtestingmulti1-cont1",
+                                               'unittest-cont2' => "{$tmpDir}/localtestingmulti1-cont2" ),
                                        'isMultiMaster' => false
                                ),
                                array(
                                        'name' => 'localmultitesting2',
                                        'class' => 'FSFileBackend',
                                        'containerPaths' => array(
-                                               'unittest-cont1' => "{$tmpPrefix}-localtestingmulti2-cont1",
-                                               'unittest-cont2' => "{$tmpPrefix}-localtestingmulti2-cont2" ),
+                                               'unittest-cont1' => "{$tmpDir}/localtestingmulti2-cont1",
+                                               'unittest-cont2' => "{$tmpDir}/localtestingmulti2-cont2" ),
                                        'isMultiMaster' => true
                                )
                        )
                ) );
-               $this->filesToPrune = array();
        }
 
        private static function baseStorePath() {
@@ -214,7 +212,7 @@ class FileBackendTest extends MediaWikiTestCase {
         * @dataProvider provider_testStore
         */
        public function testStore( $op ) {
-               $this->filesToPrune[] = $op['src'];
+               $this->addTmpFiles( $op['src'] );
 
                $this->backend = $this->singleBackend;
                $this->tearDownFiles();
@@ -224,7 +222,6 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->backend = $this->multiBackend;
                $this->tearDownFiles();
                $this->doTestStore( $op );
-               $this->filesToPrune[] = $op['src']; # avoid file leaking
                $this->tearDownFiles();
        }
 
@@ -275,27 +272,15 @@ class FileBackendTest extends MediaWikiTestCase {
                $tmpName = TempFSFile::factory( "unittests_", 'txt' )->getPath();
                $toPath = self::baseStorePath() . '/unittest-cont1/e/fun/obj1.txt';
                $op = array( 'op' => 'store', 'src' => $tmpName, 'dst' => $toPath );
-               $cases[] = array(
-                       $op, // operation
-                       $tmpName, // source
-                       $toPath, // dest
-               );
+               $cases[] = array( $op );
 
                $op2 = $op;
                $op2['overwrite'] = true;
-               $cases[] = array(
-                       $op2, // operation
-                       $tmpName, // source
-                       $toPath, // dest
-               );
+               $cases[] = array( $op2 );
 
-               $op2 = $op;
-               $op2['overwriteSame'] = true;
-               $cases[] = array(
-                       $op2, // operation
-                       $tmpName, // source
-                       $toPath, // dest
-               );
+               $op3 = $op;
+               $op3['overwriteSame'] = true;
+               $cases[] = array( $op3 );
 
                return $cases;
        }
@@ -948,18 +933,14 @@ class FileBackendTest extends MediaWikiTestCase {
         * @dataProvider provider_testConcatenate
         */
        public function testConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus ) {
-               $this->filesToPrune[] = $op['dst'];
-
                $this->backend = $this->singleBackend;
                $this->tearDownFiles();
                $this->doTestConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus );
-               $this->filesToPrune[] = $op['dst']; # avoid file leaking
                $this->tearDownFiles();
 
                $this->backend = $this->multiBackend;
                $this->tearDownFiles();
                $this->doTestConcatenate( $op, $srcs, $srcsContent, $alreadyExists, $okStatus );
-               $this->filesToPrune[] = $op['dst']; # avoid file leaking
                $this->tearDownFiles();
        }
 
@@ -983,7 +964,7 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->assertGoodStatus( $status,
                        "Creation of source files succeeded ($backendName)." );
 
-               $dest = $params['dst'];
+               $dest = $params['dst'] = $this->getNewTempFile();
                if ( $alreadyExists ) {
                        $ok = file_put_contents( $dest, 'blah...blah...waahwaah' ) !== false;
                        $this->assertEquals( true, $ok,
@@ -1029,8 +1010,6 @@ class FileBackendTest extends MediaWikiTestCase {
        public static function provider_testConcatenate() {
                $cases = array();
 
-               $rand = mt_rand( 0, 2000000000 ) . time();
-               $dest = wfTempDir() . "/randomfile!$rand.txt";
                $srcs = array(
                        self::baseStorePath() . '/unittest-cont1/e/file1.txt',
                        self::baseStorePath() . '/unittest-cont1/e/file2.txt',
@@ -1055,7 +1034,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        'lkaem;a',
                        'legma'
                );
-               $params = array( 'srcs' => $srcs, 'dst' => $dest );
+               $params = array( 'srcs' => $srcs );
 
                $cases[] = array(
                        $params, // operation
@@ -1761,16 +1740,13 @@ class FileBackendTest extends MediaWikiTestCase {
                $fileCContents = 'eigna[ogmewt 3qt g3qg flew[ag';
 
                $tmpNameA = TempFSFile::factory( "unittests_", 'txt' )->getPath();
-               file_put_contents( $tmpNameA, $fileAContents );
                $tmpNameB = TempFSFile::factory( "unittests_", 'txt' )->getPath();
-               file_put_contents( $tmpNameB, $fileBContents );
                $tmpNameC = TempFSFile::factory( "unittests_", 'txt' )->getPath();
+               $this->addTmpFiles( array( $tmpNameA, $tmpNameB, $tmpNameC ) );
+               file_put_contents( $tmpNameA, $fileAContents );
+               file_put_contents( $tmpNameB, $fileBContents );
                file_put_contents( $tmpNameC, $fileCContents );
 
-               $this->filesToPrune[] = $tmpNameA; # avoid file leaking
-               $this->filesToPrune[] = $tmpNameB; # avoid file leaking
-               $this->filesToPrune[] = $tmpNameC; # avoid file leaking
-
                $fileA = "$base/unittest-cont1/e/a/b/fileA.txt";
                $fileB = "$base/unittest-cont1/e/a/b/fileB.txt";
                $fileC = "$base/unittest-cont1/e/a/b/fileC.txt";
@@ -2434,16 +2410,10 @@ class FileBackendTest extends MediaWikiTestCase {
        }
 
        function tearDownFiles() {
-               foreach ( $this->filesToPrune as $file ) {
-                       if ( is_file( $file ) ) {
-                               unlink( $file );
-                       }
-               }
                $containers = array( 'unittest-cont1', 'unittest-cont2', 'unittest-cont-bad' );
                foreach ( $containers as $container ) {
                        $this->deleteFiles( $container );
                }
-               $this->filesToPrune = array();
        }
 
        private function deleteFiles( $container ) {
index 713c32d..3ce3e1f 100644 (file)
@@ -480,16 +480,16 @@ class NewParserTest extends MediaWikiTestCase {
         */
        protected function getUploadDir() {
                if ( $this->keepUploads ) {
+                       // Don't use getNewTempDirectory() as this is meant to persist
                        $dir = wfTempDir() . '/mwParser-images';
 
                        if ( is_dir( $dir ) ) {
                                return $dir;
                        }
                } else {
-                       $dir = wfTempDir() . "/mwParser-" . mt_rand() . "-images";
+                       $dir = $this->getNewTempDirectory();
                }
 
-               // wfDebug( "Creating upload directory $dir\n" );
                if ( file_exists( $dir ) ) {
                        wfDebug( "Already exists!\n" );
 
index f23b264..63ad8c0 100644 (file)
@@ -93,7 +93,7 @@ class UploadBaseTest extends MediaWikiTestCase {
 
        // Helper used to create an empty file of size $size.
        private function createFileOfSize( $size ) {
-               $filename = tempnam( wfTempDir(), "mwuploadtest" );
+               $filename = $this->getNewTempFile();
 
                $fh = fopen( $filename, 'w' );
                ftruncate( $fh, $size );
@@ -118,7 +118,6 @@ class UploadBaseTest extends MediaWikiTestCase {
                $filename = $this->createFileOfSize( 100 );
                $this->upload->initializePathInfo( basename( $filename ) . '.txt', $filename, 100 );
                $result = $this->upload->verifyUpload();
-               unlink( $filename );
 
                $this->assertEquals(
                        array( 'status' => UploadBase::OK ),