Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Filter\File\RenameUpload: wrap move_uploaded_file to be easly mocked #3708

Closed
wants to merge 1 commit into from

4 participants

@Slamdunk

As of yet, ZendTest\Filter\File\RenameUploadTest used runkit_function_rename to test the class that wraps move_uploaded_file.

Runkit extension is powerfull, but very old and almost no CI server has it.
If you run the tests with it, they fail because of a trivial bug in a test (method getNewName doesn't exist), not shown in Travis because it has not the extension.

I moved the blocking move_uploaded_file in a separate method, that is overwritten by the ZendTest\Filter\File\RenameUploadMock, a simple subclass to test the original class.

Runkit extension is no more needed, and no test is skipped.

@prolic

:+1: great

@cgmartin

:+1: ++ much better idea

@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/3708' into develop
Forward port #3708
b47ac44
@weierophinney weierophinney was assigned
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/3708'
Close #3708
cec2aa5
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'hotfix/3708' into develop
Forward port #3708
47c9c65
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-filter
@weierophinney weierophinney Merge branch 'hotfix/3708' 617a96b
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-filter
@weierophinney weierophinney Merge branch 'hotfix/3708' into develop c3425d2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 7, 2013
  1. @Slamdunk
This page is out of date. Refresh to see the latest.
View
24 library/Zend/Filter/File/RenameUpload.php
@@ -149,22 +149,34 @@ public function filter($value)
}
$this->checkFileExists($targetFile);
+ $this->moveUploadedFile($sourceFile, $targetFile);
+ if ($isFileUpload) {
+ $uploadData['tmp_name'] = $targetFile;
+ return $uploadData;
+ }
+ return $targetFile;
+ }
+
+ /**
+ * @param string $sourceFile Source file path
+ * @param string $targetFile Target file path
+ * @throws \Zend\Filter\Exception\RuntimeException
+ * @return boolean
+ */
+ protected function moveUploadedFile($sourceFile, $targetFile)
+ {
ErrorHandler::start();
$result = move_uploaded_file($sourceFile, $targetFile);
$warningException = ErrorHandler::stop();
if (!$result || null !== $warningException) {
throw new Exception\RuntimeException(
- sprintf("File '%s' could not be renamed. An error occurred while processing the file.", $value),
+ sprintf("File '%s' could not be renamed. An error occurred while processing the file.", $sourceFile),
0, $warningException
);
}
- if ($isFileUpload) {
- $uploadData['tmp_name'] = $targetFile;
- return $uploadData;
- }
- return $targetFile;
+ return $result;
}
/**
View
9 tests/Bootstrap.php
@@ -93,12 +93,3 @@
* Unset global variables that are no longer needed.
*/
unset($zfRoot, $zfCoreLibrary, $zfCoreTests, $path);
-
-/**
- * Internal PHP function mocks
- * To be used with runkit_function_rename()
- */
-function move_uploaded_file_mock($source, $dest)
-{
- return rename($source, $dest);
-}
View
25 tests/ZendTest/Filter/File/RenameUploadMock.php
@@ -0,0 +1,25 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Filter\File;
+
+use Zend\Filter\File\RenameUpload;
+
+class RenameUploadMock extends RenameUpload
+{
+ /**
+ * @param string $sourceFile Source file path
+ * @param string $targetFile Target file path
+ * @return boolean
+ */
+ protected function moveUploadedFile($sourceFile, $targetFile)
+ {
+ return rename($sourceFile, $targetFile);
+ }
+}
View
126 tests/ZendTest/Filter/File/RenameUploadTest.php
@@ -28,13 +28,6 @@ class RenameUploadTest extends \PHPUnit_Framework_TestCase
protected $_filesPath;
/**
- * Original testfile
- *
- * @var string
- */
- protected $_origFile;
-
- /**
* Testfile
*
* @var string
@@ -69,26 +62,19 @@ class RenameUploadTest extends \PHPUnit_Framework_TestCase
*/
public function setUp()
{
- $this->_filesPath = dirname(__DIR__) . DIRECTORY_SEPARATOR . '_files' . DIRECTORY_SEPARATOR;
- $this->_origFile = $this->_filesPath . 'original.file';
- $this->_oldFile = $this->_filesPath . 'testfile.txt';
- $this->_newFile = $this->_filesPath . 'newfile.xml';
- $this->_newDir = $this->_filesPath . DIRECTORY_SEPARATOR . '_testDir2';
- $this->_newDirFile = $this->_newDir . DIRECTORY_SEPARATOR . 'testfile.txt';
-
- if (file_exists($this->_origFile)) {
- unlink($this->_origFile);
- }
+ $this->_filesPath = dirname(__DIR__) . DIRECTORY_SEPARATOR . '_files' . DIRECTORY_SEPARATOR . 'RenameUploadTest';
+ $this->_newDir = $this->_filesPath . DIRECTORY_SEPARATOR . '_testDir2';
- if (file_exists($this->_newFile)) {
- unlink($this->_newFile);
- }
+ $this->tearDown();
- if (file_exists($this->_newDirFile)) {
- unlink($this->_newDirFile);
- }
+ mkdir($this->_filesPath);
+ mkdir($this->_newDir);
+
+ $this->_oldFile = $this->_filesPath . '/testfile.txt';
+ $this->_newFile = $this->_filesPath . '/newfile.xml';
+ $this->_newDirFile = $this->_newDir . '/testfile.txt';
- copy($this->_oldFile, $this->_origFile);
+ touch($this->_oldFile);
}
/**
@@ -98,28 +84,23 @@ public function setUp()
*/
public function tearDown()
{
- if (!file_exists($this->_oldFile)) {
- copy($this->_origFile, $this->_oldFile);
- }
-
- if (file_exists($this->_origFile)) {
- unlink($this->_origFile);
- }
+ $this->removeDir($this->_newDir);
+ $this->removeDir($this->_filesPath);
+ }
- if (file_exists($this->_newFile)) {
- unlink($this->_newFile);
+ protected function removeDir($dir)
+ {
+ if (! is_dir($dir)) {
+ return;
}
- if (file_exists($this->_newDirFile)) {
- unlink($this->_newDirFile);
+ foreach (glob($dir . DIRECTORY_SEPARATOR . '*') as $file) {
+ if (is_file($file)) {
+ unlink($file);
+ }
}
- if (function_exists("runkit_function_rename")
- && function_exists('move_uploaded_file_orig')
- ) {
- runkit_function_rename('move_uploaded_file', 'move_uploaded_file_mock');
- runkit_function_rename('move_uploaded_file_orig', 'move_uploaded_file');
- }
+ rmdir($dir);
}
/**
@@ -139,25 +120,6 @@ public function testThrowsExceptionWithNonUploadedFile()
}
/**
- * Mock the move_uploaded_file() function with rename() functionality
- *
- * @return void
- */
- protected function setUpMockMoveUploadedFile()
- {
- if (!function_exists("runkit_function_rename")
- || !ini_get('runkit.internal_override')
- ) {
- $this->markTestSkipped(
- 'move_uploaded_file cannot be unit tested without runkit module'
- );
- return;
- }
- runkit_function_rename('move_uploaded_file', 'move_uploaded_file_orig');
- runkit_function_rename('move_uploaded_file_mock', 'move_uploaded_file');
- }
-
- /**
* @return void
*/
public function testOptions()
@@ -185,9 +147,7 @@ public function testOptions()
*/
public function testStringConstructorParam()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload($this->_newFile);
+ $filter = new RenameUploadMock($this->_newFile);
$this->assertEquals($this->_newFile, $filter->getTarget());
$this->assertEquals($this->_newFile, $filter($this->_oldFile));
$this->assertEquals('falsefile', $filter('falsefile'));
@@ -198,9 +158,7 @@ public function testStringConstructorParam()
*/
public function testStringConstructorWithFilesArray()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload($this->_newFile);
+ $filter = new RenameUploadMock($this->_newFile);
$this->assertEquals($this->_newFile, $filter->getTarget());
$this->assertEquals(
array(
@@ -220,9 +178,7 @@ public function testStringConstructorWithFilesArray()
*/
public function testArrayConstructorParam()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload(array(
+ $filter = new RenameUploadMock(array(
'target' => $this->_newFile,
));
$this->assertEquals($this->_newFile, $filter->getTarget());
@@ -246,9 +202,7 @@ public function testConstructTruncatedTarget()
*/
public function testTargetDirectory()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload($this->_newDir);
+ $filter = new RenameUploadMock($this->_newDir);
$this->assertEquals($this->_newDir, $filter->getTarget());
$this->assertEquals($this->_newDirFile, $filter($this->_oldFile));
$this->assertEquals('falsefile', $filter('falsefile'));
@@ -259,9 +213,7 @@ public function testTargetDirectory()
*/
public function testOverwriteWithExistingFile()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload(array(
+ $filter = new RenameUploadMock(array(
'target' => $this->_newFile,
'overwrite' => true,
));
@@ -269,7 +221,7 @@ public function testOverwriteWithExistingFile()
copy($this->_oldFile, $this->_newFile);
$this->assertEquals($this->_newFile, $filter->getTarget());
- $this->assertEquals($this->_newFile, $filter->filter($this->_oldFile));
+ $this->assertEquals($this->_newFile, $filter($this->_oldFile));
}
/**
@@ -277,9 +229,7 @@ public function testOverwriteWithExistingFile()
*/
public function testCannotOverwriteExistingFile()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload(array(
+ $filter = new RenameUploadMock(array(
'target' => $this->_newFile,
'overwrite' => false,
));
@@ -291,7 +241,7 @@ public function testCannotOverwriteExistingFile()
$this->setExpectedException(
'Zend\Filter\Exception\InvalidArgumentException', 'already exists'
);
- $this->assertEquals($this->_newFile, $filter->filter($this->_oldFile));
+ $this->assertEquals($this->_newFile, $filter($this->_oldFile));
}
/**
@@ -299,17 +249,15 @@ public function testCannotOverwriteExistingFile()
*/
public function testGetRandomizedFile()
{
- $this->setUpMockMoveUploadedFile();
-
- $filter = new FileRenameUpload(array(
+ $filter = new RenameUploadMock(array(
'target' => $this->_newFile,
'randomize' => true,
));
$this->assertEquals($this->_newFile, $filter->getTarget());
$this->assertTrue($filter->getRandomize());
- $fileNoExt = $this->_filesPath . 'newfile';
- $this->assertRegExp('#' . $fileNoExt . '_.{13}\.xml#', $filter->getNewName($this->_oldFile));
+ $fileNoExt = $this->_filesPath . '/newfile';
+ $this->assertRegExp('#' . $fileNoExt . '_.{13}\.xml#', $filter($this->_oldFile));
}
/**
@@ -317,17 +265,15 @@ public function testGetRandomizedFile()
*/
public function testGetRandomizedFileWithoutExtension()
{
- $this->setUpMockMoveUploadedFile();
-
- $fileNoExt = $this->_filesPath . 'newfile';
- $filter = new FileRenameUpload(array(
+ $fileNoExt = $this->_filesPath . '/newfile';
+ $filter = new RenameUploadMock(array(
'target' => $fileNoExt,
'randomize' => true,
));
$this->assertEquals($fileNoExt, $filter->getTarget());
$this->assertTrue($filter->getRandomize());
- $this->assertRegExp('#' . $fileNoExt . '_.{13}#', $filter->getNewName($this->_oldFile));
+ $this->assertRegExp('#' . $fileNoExt . '_.{13}#', $filter($this->_oldFile));
}
/**
Something went wrong with that request. Please try again.