-
Notifications
You must be signed in to change notification settings - Fork 195
Support for setting PHP settings through configuration #193
Changes from all commits
0282f2f
4c73602
6851310
4ae7ff6
7bd9632
bade4e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
/** | ||
* Zend Framework (http://framework.zend.com/) | ||
* | ||
* @see http://github.com/zendframework/zend-expressive for the canonical source repository | ||
* @copyright Copyright (c) 2015 Zend Technologies USA Inc. (http://www.zend.com) | ||
* @license https://github.com/zendframework/zend-expressive/blob/master/LICENSE.md New BSD License | ||
*/ | ||
|
||
namespace Zend\Expressive\Container\Exception; | ||
|
||
use RuntimeException; | ||
|
||
/** | ||
* Exception indicating that setting of some PHP configuration option has failed. | ||
*/ | ||
class PhpSettingsFailureException extends RuntimeException implements ExceptionInterface | ||
{ | ||
public static function forOption($name, $value) | ||
{ | ||
return new self("Setting PHP configuration '$name' with a value of '$value' has failed"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,68 @@ public function setUp() | |
$this->factory = new ApplicationFactory(); | ||
} | ||
|
||
protected function getContainer($config = null, $router = null, $finalHandler = null, $emitter = null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Maks3w What do you think about this approach, of having a helper method for configuring container based on the test specificity? Test cases now look more cleaner. Maybe these should be applied to all the other other ApplicationFactory tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I saw too a helper or many helper methods are needed I disagree with this proposal because add too much logic. The problem with add logic branches in a test case (if, for, ...) is difficult to trust is bugfree. What test method test getContainer works as expected? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this would allow us to have only one place for managing and maintaining container configuration and it also solves code duplication in all the test cases in this class. I agree though that it is somewhat unreliable due to its complexity, but after all, every test case which consumes will fail if it does not work as expected. |
||
{ | ||
if (null !== $config) { | ||
$this->container | ||
->has('config') | ||
->willReturn(true); | ||
|
||
$this->container | ||
->get('config') | ||
->willReturn($config); | ||
} else { | ||
$this->container | ||
->has('config') | ||
->willReturn(false); | ||
} | ||
|
||
if (null !== $router) { | ||
$this->container | ||
->has('Zend\Expressive\Router\RouterInterface') | ||
->willReturn(true); | ||
$this->container | ||
->get('Zend\Expressive\Router\RouterInterface') | ||
->will(function () use ($router) { | ||
return $router->reveal(); | ||
}); | ||
} else { | ||
$this->container | ||
->has('Zend\Expressive\Router\RouterInterface') | ||
->willReturn(false); | ||
} | ||
|
||
if (null !== $finalHandler) { | ||
$this->container | ||
->has('Zend\Expressive\FinalHandler') | ||
->willReturn(true); | ||
$this->container | ||
->get('Zend\Expressive\FinalHandler') | ||
->willReturn($finalHandler); | ||
} else { | ||
$this->container | ||
->has('Zend\Expressive\FinalHandler') | ||
->willReturn(false); | ||
} | ||
|
||
if (null !== $emitter) { | ||
$this->container | ||
->has('Zend\Diactoros\Response\EmitterInterface') | ||
->willReturn(true); | ||
$this->container | ||
->get('Zend\Diactoros\Response\EmitterInterface') | ||
->will(function () use ($emitter) { | ||
return $emitter->reveal(); | ||
}); | ||
} else { | ||
$this->container | ||
->has('Zend\Diactoros\Response\EmitterInterface') | ||
->willReturn(false); | ||
} | ||
|
||
return $this->container; | ||
} | ||
|
||
public function assertRoute($spec, array $routes) | ||
{ | ||
$this->assertTrue(array_reduce($routes, function ($found, $route) use ($spec) { | ||
|
@@ -65,6 +127,49 @@ public function getRouterFromApplication(Application $app) | |
return $r->getValue($app); | ||
} | ||
|
||
/** | ||
* @expectedException \Zend\Expressive\Exception\InvalidArgumentException | ||
*/ | ||
public function testInvalidPhpSettingsRaisesException() | ||
{ | ||
$config = ['php_settings' => 'invalid']; | ||
|
||
$this->factory->__invoke($this->getContainer($config)->reveal()); | ||
} | ||
|
||
/** | ||
* @expectedException \Zend\Expressive\Container\Exception\PhpSettingsFailureException | ||
*/ | ||
public function testWhenSettingPhpConfigurationFailsExceptionIsRaised() | ||
{ | ||
$config = [ | ||
'php_settings' => [ | ||
'invalid_option' => 1, | ||
], | ||
]; | ||
|
||
$this->factory->__invoke($this->getContainer($config)->reveal()); | ||
} | ||
|
||
public function testFactorySetsPhpSettingsFromConfig() | ||
{ | ||
$config = [ | ||
'php_settings' => [ | ||
'display_errors' => 1, | ||
'date.timezone' => 'Europe/Belgrade', | ||
], | ||
]; | ||
|
||
$this->iniSet('display_errors', 0); | ||
$this->iniSet('date.timezone', 'UTC'); | ||
|
||
$this->factory->__invoke($this->getContainer($config)->reveal()); | ||
|
||
foreach ($config['php_settings'] as $name => $value) { | ||
$this->assertEquals($value, ini_get($name)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this foreach should be wrapped in a try/finally where try is this loop and finally is the tear down loop |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Maks3w Is this in line with what you've mentioned about having only one loop? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PHPUnit assertions throw exceptions when assertion fail. So ini_restore won't be restored when test fail so next tests in the run will be executed with a wrong environment setting. After $config $this->iniSet('display_errors', 0);
$this->iniSet('date.timezone', 'UTC'); |
||
} | ||
|
||
public function testFactoryWillPullAllReplaceableDependenciesFromContainerWhenPresent() | ||
{ | ||
$router = $this->prophesize('Zend\Expressive\Router\RouterInterface'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Maks3w What do you think about moving
$this->setPhpSettings($container);
at the very beginning of a__invoke()
method, because it doesn't need$app
at all?