-
Notifications
You must be signed in to change notification settings - Fork 87
It's time to add some test for the installer itself #48
Changes from all commits
2192831
1848cf5
18266fc
217c275
e2542e4
54f2e1c
9601893
9d56446
02493e7
0306888
777bfc5
8e79a94
7163f30
2b917d5
1038862
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 |
---|---|---|
|
@@ -42,27 +42,27 @@ class OptionalPackages | |
/** | ||
* @var array | ||
*/ | ||
private static $config; | ||
public static $config; | ||
|
||
/** | ||
* @var array | ||
*/ | ||
private static $composerDefinition; | ||
public static $composerDefinition; | ||
|
||
/** | ||
* @var string[] | ||
*/ | ||
private static $composerRequires; | ||
public static $composerRequires; | ||
|
||
/** | ||
* @var string[] | ||
*/ | ||
private static $composerDevRequires; | ||
public static $composerDevRequires; | ||
|
||
/** | ||
* @var string[] | ||
*/ | ||
private static $stabilityFlags; | ||
public static $stabilityFlags; | ||
|
||
/** | ||
* Install command: choose packages and provide configuration. | ||
|
@@ -162,8 +162,19 @@ public static function install(Event $event) | |
// House keeping | ||
$io->write("<info>Remove installer</info>"); | ||
|
||
// Remove composer source | ||
// Remove test dependencies | ||
unset(self::$composerDefinition['require-dev']['composer/composer']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-aurarouter']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-fastroute']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-zendrouter']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-platesrenderer']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-twigrenderer']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-expressive-zendviewrenderer']); | ||
unset(self::$composerDefinition['require-dev']['zendframework/zend-servicemanager']); | ||
unset(self::$composerDefinition['require-dev']['ocramius/proxy-manager']); | ||
unset(self::$composerDefinition['require-dev']['aura/di']); | ||
unset(self::$composerDefinition['require-dev']['mouf/pimple-interop']); | ||
unset(self::$composerDefinition['autoload-dev']['psr-4']['ExpressiveInstallerTest\\']); | ||
|
||
// Remove installer data | ||
unset(self::$composerDefinition['extra']['optional-packages']); | ||
|
@@ -189,7 +200,7 @@ public static function install(Event $event) | |
self::removeDefaultMiddleware($io, $projectRoot); | ||
} | ||
|
||
self::cleanUp($io); | ||
self::cleanUp($io, $projectRoot); | ||
} | ||
|
||
/** | ||
|
@@ -199,11 +210,13 @@ public static function install(Event $event) | |
* this one) and assets (including configuration and templates). | ||
* | ||
* @param IOInterface $io | ||
* @param string $projectRoot | ||
*/ | ||
private static function cleanUp(IOInterface $io) | ||
private static function cleanUp(IOInterface $io, $projectRoot) | ||
{ | ||
$io->write("<info>Removing Expressive installer classes and configuration</info>"); | ||
$io->write("<info>Removing Expressive installer classes, configuration, and tests</info>"); | ||
self::recursiveRmdir(__DIR__); | ||
self::recursiveRmdir($projectRoot . '/test/ExpressiveInstallerTest'); | ||
} | ||
|
||
/** | ||
|
@@ -288,7 +301,7 @@ private static function askQuestion(Composer $composer, IOInterface $io, $questi | |
* @param $packageName | ||
* @param $packageVersion | ||
*/ | ||
private static function addPackage(IOInterface $io, $packageName, $packageVersion) | ||
public static function addPackage(IOInterface $io, $packageName, $packageVersion) | ||
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. Why public? 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. nm; I see why (you call on these in the tests) |
||
{ | ||
$io->write(sprintf( | ||
" - Adding package <info>%s</info> (<comment>%s</comment>)", | ||
|
@@ -340,7 +353,7 @@ private static function addPackage(IOInterface $io, $packageName, $packageVersio | |
* @param string $target Destination. | ||
* @param bool $force whether or not to copy over an existing file. | ||
*/ | ||
private static function copyFile(IOInterface $io, $projectRoot, $source, $target, $force = false) | ||
public static function copyFile(IOInterface $io, $projectRoot, $source, $target, $force = false) | ||
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. Why public? 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. Same, it's called in the InstallerTestCase |
||
{ | ||
// Copy file | ||
if ($force === false && is_file($projectRoot . $target)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
<?php | ||
|
||
namespace ExpressiveInstallerTest; | ||
|
||
use Aura\Di\Container as AuraContainer; | ||
use ExpressiveInstaller\OptionalPackages; | ||
use Interop\Container\ContainerInterface; | ||
use Interop\Container\Pimple\PimpleInterop as PimpleInteropContainer; | ||
use Zend\Expressive; | ||
use Zend\ServiceManager\ServiceManager as ZendServiceManagerContainer; | ||
|
||
class ContainersTest extends InstallerTestCase | ||
{ | ||
protected $teardownFiles = [ | ||
'/config/container.php', | ||
'/config/autoload/routes.global.php', | ||
]; | ||
|
||
/** | ||
* @dataProvider containerProvider | ||
*/ | ||
public function testContainer( | ||
$containerOption, | ||
$routerOption, | ||
$copyFilesKey, | ||
$expectedResponseStatusCode, | ||
$expectedContainer | ||
) { | ||
// Install packages | ||
$this->installPackage( | ||
OptionalPackages::$config['questions']['container']['options'][$containerOption], | ||
$copyFilesKey | ||
); | ||
$this->installPackage( | ||
OptionalPackages::$config['questions']['router']['options'][$routerOption], | ||
$copyFilesKey | ||
); | ||
|
||
// Test container | ||
$container = $this->getContainer(); | ||
$this->assertInstanceOf(ContainerInterface::class, $container); | ||
$this->assertInstanceOf($expectedContainer, $container); | ||
$this->assertTrue($container->has(Expressive\Helper\UrlHelper::class)); | ||
$this->assertTrue($container->has(Expressive\Helper\ServerUrlHelper::class)); | ||
$this->assertTrue($container->has(Expressive\Application::class)); | ||
$this->assertTrue($container->has(Expressive\Router\RouterInterface::class)); | ||
|
||
// Test home page | ||
$response = $this->getAppResponse(); | ||
$this->assertEquals($expectedResponseStatusCode, $response->getStatusCode()); | ||
} | ||
|
||
public function containerProvider() | ||
{ | ||
// $containerOption, $routerOption, $copyFilesKey, $expectedResponseStatusCode, $expectedContainer | ||
return [ | ||
'aura-minimal' => [1, 2, 'minimal-files', 404, AuraContainer::class], | ||
'aura-full' => [1, 2, 'copy-files', 200, AuraContainer::class], | ||
'pimple-minimal' => [2, 2, 'minimal-files', 404, PimpleInteropContainer::class], | ||
'pimple-full' => [2, 2, 'copy-files', 200, PimpleInteropContainer::class], | ||
'zend-sm-minimal' => [3, 2, 'minimal-files', 404, ZendServiceManagerContainer::class], | ||
'zend-sm-full' => [3, 2, 'copy-files', 200, ZendServiceManagerContainer::class], | ||
]; | ||
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. Please add keys to each, detailing what they are testing; this makes understanding which failed simpler: return [
'aura-full-install' => [ /* ... */ ],
'aura-minimal' => [/* ... */ ],
/* etc. */
]; |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
<?php | ||
|
||
namespace ExpressiveInstallerTest; | ||
|
||
use Composer\Factory; | ||
use Composer\IO\IOInterface; | ||
use Composer\Json\JsonFile; | ||
use ExpressiveInstaller\OptionalPackages; | ||
use Interop\Container\ContainerInterface; | ||
use Prophecy\Argument; | ||
use Psr\Http\Message\ResponseInterface; | ||
use Zend\Diactoros\Response; | ||
use Zend\Diactoros\ServerRequest; | ||
use Zend\Expressive\Application; | ||
|
||
class InstallerTestCase extends \PHPUnit_Framework_TestCase | ||
{ | ||
/** | ||
* @var IOInterface | ||
*/ | ||
private $io; | ||
|
||
private $projectRoot; | ||
|
||
/** | ||
* @var ContainerInterface | ||
*/ | ||
protected $container; | ||
|
||
protected $response; | ||
|
||
protected $teardownFiles = []; | ||
|
||
public function setup() | ||
{ | ||
$this->response = null; | ||
|
||
$this->cleanup(); | ||
|
||
$this->io = $this->prophesize('Composer\IO\IOInterface'); | ||
|
||
// Get composer.json | ||
$composerFile = Factory::getComposerFile(); | ||
$json = new JsonFile($composerFile); | ||
OptionalPackages::$composerDefinition = $json->read(); | ||
|
||
$this->projectRoot = realpath(dirname($composerFile)); | ||
|
||
OptionalPackages::$config = require 'src/ExpressiveInstaller/config.php'; | ||
} | ||
|
||
public function tearDown() | ||
{ | ||
parent::tearDown(); | ||
|
||
$this->cleanup(); | ||
} | ||
|
||
public function installPackage($config, $copyFilesKey) | ||
{ | ||
/* TODO: First we need to set $composerDefinition, $composerRequires, $composerDevRequires and $stabilityFlags; | ||
// Add packages to install | ||
if (isset($config['packages'])) { | ||
foreach ($config['packages'] as $packageName) { | ||
OptionalPackages::addPackage($this->io, $packageName, $this->config['packages'][$packageName]); | ||
} | ||
}*/ | ||
|
||
// Copy files | ||
if (isset($config[$copyFilesKey])) { | ||
foreach ($config[$copyFilesKey] as $source => $target) { | ||
OptionalPackages::copyFile($this->io->reveal(), $this->projectRoot, $source, $target); | ||
} | ||
} | ||
} | ||
|
||
public function cleanup() | ||
{ | ||
foreach ($this->teardownFiles as $file) { | ||
if (is_file($this->projectRoot.$file)) { | ||
unlink($this->projectRoot.$file); | ||
} | ||
} | ||
} | ||
|
||
public function getContainer() | ||
{ | ||
if (!$this->container) { | ||
/** @var ContainerInterface $container */ | ||
$this->container = require 'config/container.php'; | ||
} | ||
|
||
return $this->container; | ||
} | ||
|
||
public function getAppResponse($path = '/') | ||
{ | ||
$container = $this->getContainer(); | ||
|
||
/** @var Application $app */ | ||
$app = $container->get('Zend\Expressive\Application'); | ||
$request = new ServerRequest([], [], 'https://example.com'.$path, 'GET'); | ||
|
||
/** @var ResponseInterface $response */ | ||
$response = $app($request, new Response()); | ||
|
||
return $response; | ||
} | ||
} |
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.
Why public?
If it's for testing, the various
assertAttribute*()
assertions can do assertions on non-public properties. That said, I've not tried on static properties; if those present an issue, I'd argue reflection is still a better way to test these.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.
Those are not being tested. They are public so I can set them from within a test. Doing it this way I don't need to repeat the code to setup the OptionalPackages class.
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.
https://github.com/zendframework/zend-expressive-skeleton/pull/48/files#diff-733c8720c49c759a0935fc5596c19e72R49
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.
Couldn't these assignments be done with the Reflection API instead? That way we protect encapsulation for normal consumption.
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.
I'm not sure how this would work with static classes.