Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Feature/zend test load module #3942

Closed
wants to merge 9 commits into from

3 participants

@blanchonvincent

The goal is to have an helper for write unit tests on module :

@blanchonvincent

@Ocramius can read this PR ? thank you :)

library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((1 lines not shown))
+<?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 Zend\Test\PHPUnit\Util;
+
+use PHPUnit_Framework_TestCase;
+use Zend\Mvc\Service;
+use Zend\ServiceManager\ServiceManager;
+use Zend\Stdlib\Exception;
+
+abstract class ModuleLoader extends PHPUnit_Framework_TestCase
@Ocramius Collaborator
Ocramius added a note

Why abstract? And why extending a PHPUnit_Framework_TestCase?

Look my unit test, the goal was provide a "loadModule" method with direct assertion without usage of "new"

@Ocramius Collaborator
Ocramius added a note

@blanchonvincent perfectly clear what the intent was, I think that isn't an optimal approach (especially since it's an util, not a test case).

@Ocramius agree with you, it was just a draft :) i will change that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((12 lines not shown))
+use Zend\Mvc\Service;
+use Zend\ServiceManager\ServiceManager;
+use Zend\Stdlib\Exception;
+
+abstract class ModuleLoader extends PHPUnit_Framework_TestCase
+{
+ /**
+ * @var ServiceManager
+ */
+ protected $serviceManager;
+
+ /**
+ * Load module
+ * @param string $moduleName
+ */
+ public function loadModule($moduleName)
@Ocramius Collaborator
Ocramius added a note

What about passing this as constructor param?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((35 lines not shown))
+ /**
+ * Load modules
+ * @param array $modules
+ */
+ public function loadModules(array $modules)
+ {
+ $modulesPath = array();
+ $modulesList = array();
+
+ foreach ($modules as $key => $module) {
+ if (is_numeric($key)) {
+ $modulesList[] = $module;
+ continue;
+ }
+ $modulesList[] = $key;
+ $modulesPath[$key] = $module;
@Ocramius Collaborator
Ocramius added a note

This is not really needed. The ModuleManager already recognizes modules passed in $name => $path format as far as I know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((66 lines not shown))
+ {
+ $smConfig = isset($configuration['service_manager']) ? $configuration['service_manager'] : array();
+ $this->serviceManager = new ServiceManager(new Service\ServiceManagerConfig($smConfig));
+ $this->serviceManager->setService('ApplicationConfig', $configuration);
+ $this->serviceManager->get('ModuleManager')->loadModules();
+ }
+
+ /**
+ * Get an instance of a module class by the module name
+ *
+ * @param string $moduleName
+ * @return mixed
+ */
+ public function getModule($moduleName)
+ {
+ return $this->serviceManager->get('ModuleManager')->getModule($moduleName);
@Ocramius Collaborator
Ocramius added a note

Assumes the $this->serviceManager is set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((76 lines not shown))
+ * @param string $moduleName
+ * @return mixed
+ */
+ public function getModule($moduleName)
+ {
+ return $this->serviceManager->get('ModuleManager')->getModule($moduleName);
+ }
+
+ /**
+ * Get the array of module names that this manager should load.
+ *
+ * @return array
+ */
+ public function getModules()
+ {
+ return $this->serviceManager->get('ModuleManager')->getModules();
@Ocramius Collaborator
Ocramius added a note

Same here: assumption that $this->serviceManager is set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/PHPUnit/Util/ModuleLoader.php
((86 lines not shown))
+ *
+ * @return array
+ */
+ public function getModules()
+ {
+ return $this->serviceManager->get('ModuleManager')->getModules();
+ }
+
+ /**
+ * Get the service manager
+ * @var ServiceManager
+ */
+ public function getServiceManager()
+ {
+ if (null === $this->serviceManager) {
+ throw new Exception\LogicException('You must load modules before to have access to the service manager');
@Ocramius Collaborator
Ocramius added a note

Instead of throwing, instantiate one with provided module config (see my previous suggestion of having a constructor and making this NOT a test case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius
Collaborator

@blanchonvincent the idea looks good so far, BUT there's way too much public API in my opinion.

You should probably have something like:

  • __construct($modules, $modulePaths) (or $applicationConfig)
  • getServiceManager()
  • getModuleManager()
  • getApplication()

No other public API is needed in my opinion (that's already probably too much) :)

Usage would be:

class MyTest extends PHPUnit_Framework_TestCase
{
    public function testFoo()
    {
        $moduleLoader = new \Zend\Test\PHPUnit\Util\ModuleLoader(array(
            'modules' => array('Application')
        ));

        $this->assertInstanceOf(
            'Zend\ServiceManager\ServiceLocatorInterface',
            $moduleLoader->getServiceManager()
        );
        $this->assertInstanceOf(
            'Zend\ModuleManager\ModuleManager',
            $moduleLoader->getModuleManager()
        );
        $this->assertInstanceOf(
            'Zend\Mvc\ApplicationInterface',
            $moduleLoader->getApplication()
        );
    }
}
@blanchonvincent

@Ocramius I did not know if I had to make a external class (like you) or internal class with PHPUnit_Framework_TestCase integrated (like me).
Your solution look more natural and easy to understand for new user.

@Ocramius
Collaborator

@blanchonvincent it's not really about ease of use... I'm mainly thinking of something that doesn't enforce inheritance from another test case (we got so many already :( )

@blanchonvincent

@Ocramius yeah, you are right

@Ocramius
Collaborator

@blanchonvincent ping again once you think you've sorted it out, and thank you for picking this =)

@blanchonvincent

@Ocramius second draft, look better, i will add unit tests again later

library/Zend/Test/Util/ModuleLoader.php
((26 lines not shown))
+ {
+ if (!isset($modules['modules'])) {
+ $modulesPath = array();
+ $modulesList = array();
+
+ foreach ($modules as $key => $module) {
+ if (is_numeric($key)) {
+ $modulesList[] = $module;
+ continue;
+ }
+ $modulesList[] = $key;
+ $modulesPath[$key] = $module;
+ }
+ $configuration = array(
+ 'module_listener_options' => array(
+ 'module_paths' => $modulesPath,
@Ocramius Collaborator
Ocramius added a note

As before: if a module is provided as name => path, then the module manager should already be able to handle that :)

@Ocramius if you write in your config :

'modules' => array(
    'Foo' => '/path/to/my/foo',
),

?

Currently in "modules" key, only name is accepted : https://github.com/zendframework/zf2/blob/master/library/Zend/ModuleManager/ModuleManager.php#L80

@Ocramius Collaborator
Ocramius added a note

@EvanDotPro ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Test/Util/ModuleLoader.php
((12 lines not shown))
+use Zend\ServiceManager\ServiceManager;
+
+class ModuleLoader
+{
+ /**
+ * @var ServiceManager
+ */
+ protected $serviceManager;
+
+ /**
+ * Load list of modules or application configuration
+ * @param array $modules
+ */
+ public function __construct(array $modules)
+ {
+ if (!isset($modules['modules'])) {
@Ocramius Collaborator
Ocramius added a note

@Ocramius Ok, but i don't understand how i can use this to improve the ModuleLoader ?

@Ocramius Collaborator
Ocramius added a note

@blanchonvincent you're basically merging config here :)

@Ocramius i improve this part, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius
Collaborator

Looks much better, thank you! :)

@weierophinney weierophinney was assigned
@weierophinney

@blanchonvincent I tried merging, but I'm missing the file tests/ZendTest/Test/_files/cache/module-config-cache.phpunit.php (notified in ModuleLoaderTest::testCanLoadModulesFromConfig).

@blanchonvincent

@weierophinney you must only create the "cache" directory in "tests/ZendTest/Test/_files/". I add the directory in the versioning.

@weierophinney

@blanchonvincent Do not write files inside the tests directory; write them to the system temporary directory. (We need to do this in part to allow developers to run the test suite when installed via their system package manager; if we write inside the tests directory, they may not have permissions. Plus, it's cleaner.)

@blanchonvincent

@weierophinney sorry, i changed with the system temporary directory.

@weierophinney

@blanchonvincent I ran into issues running the test suite multiple times, as the test suite is not cleaning out the cache files. Interestingly, when I implemented that logic... a new test started failing consistently, AbstractControllerTestCaseTest::testApplicationClassAndTestRestoredConsoleFlag now fails on the very first assertion of Console::isConsole. Trying to track it down...

@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney [#3942] Clear cache files
- Clears module cache files on setup and teardown
35e077e
@weierophinney weierophinney referenced this pull request from a commit
@weierophinney weierophinney [#3942] CS fixes
- trailing whitespace, braces
b319113
@weierophinney

Tracked it down, and all tests are running. Thanks, @blanchonvincent -- merged to develop for release with 2.2.0.

@Ocramius
Collaborator

:+1:!

@blanchonvincent

@weierophinney @Ocramius Thank you both for your work

@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#3942] Clear cache files
- Clears module cache files on setup and teardown
af23c18
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney [#3942] CS fixes
- trailing whitespace, braces
d6a5f0e
@ghost Unknown referenced this pull request from a commit
@weierophinney weierophinney Merge branch 'feature/3942' into develop
Close #3942
35fc241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
86 library/Zend/Test/Util/ModuleLoader.php
@@ -0,0 +1,86 @@
+<?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 Zend\Test\Util;
+
+use Zend\Mvc\Service;
+use Zend\ServiceManager\ServiceManager;
+
+class ModuleLoader
+{
+ /**
+ * @var ServiceManager
+ */
+ protected $serviceManager;
+
+ /**
+ * Load list of modules or application configuration
+ * @param array $modules
+ */
+ public function __construct(array $configuration)
+ {
+ if (!isset($configuration['modules'])) {
+ $modules = $configuration;
+ $configuration = array(
+ 'module_listener_options' => array(
+ 'module_paths' => array(),
+ ),
+ 'modules' => array(),
+ );
+ foreach ($modules as $key => $module) {
+ if (is_numeric($key)) {
+ $configuration['modules'][] = $module;
+ continue;
+ }
+ $configuration['modules'][] = $key;
+ $configuration['module_listener_options']['module_paths'][$key] = $module;
+ }
+ }
+
+ $smConfig = isset($configuration['service_manager']) ? $configuration['service_manager'] : array();
+ $this->serviceManager = new ServiceManager(new Service\ServiceManagerConfig($smConfig));
+ $this->serviceManager->setService('ApplicationConfig', $configuration);
+ $this->serviceManager->get('ModuleManager')->loadModules();
+ }
+
+ /**
+ * Get the application
+ * @return Zend\Mvc\Application
+ */
+ public function getApplication()
+ {
+ return $this->getServiceManager()->get('Application');
+ }
+
+ /**
+ * Get the module manager
+ * @return Zend\ModuleManager\ModuleManager
+ */
+ public function getModuleManager()
+ {
+ return $this->getServiceManager()->get('ModuleManager');
+ }
+
+ /**
+ * Get module
+ * @return mixed
+ */
+ public function getModule($moduleName)
+ {
+ return $this->getModuleManager()->getModule($moduleName);
+ }
+
+ /**
+ * Get the service manager
+ * @var ServiceManager
+ */
+ public function getServiceManager()
+ {
+ return $this->serviceManager;
+ }
+}
View
86 tests/ZendTest/Test/PHPUnit/Util/ModuleLoaderTest.php
@@ -0,0 +1,86 @@
+<?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\Test\PHPUnit\Util;
+
+use PHPUnit_Framework_TestCase;
+use Zend\Test\Util\ModuleLoader;
+
+class ModuleLoaderTest extends PHPUnit_Framework_TestCase
+{
+ public function testCanLoadModule()
+ {
+ require_once __DIR__ . '/../../_files/Baz/Module.php';
+
+ $loader = new ModuleLoader(array('Baz'));
+ $baz = $loader->getModule('Baz');
+ $this->assertTrue($baz instanceof \Baz\Module);
+ }
+
+ public function testCanNotLoadModule()
+ {
+ $this->setExpectedException('Zend\ModuleManager\Exception\RuntimeException', 'could not be initialized');
+ $loader = new ModuleLoader(array('FooBaz'));
+ }
+
+ public function testCanLoadModuleWithPath()
+ {
+ $loader = new ModuleLoader(array('Baz' => __DIR__ . '/../../_files/Baz'));
+ $baz = $loader->getModule('Baz');
+ $this->assertTrue($baz instanceof \Baz\Module);
+ }
+
+ public function testCanLoadModules()
+ {
+ require_once __DIR__ . '/../../_files/Baz/Module.php';
+ require_once __DIR__ . '/../../_files/modules-path/with-subdir/Foo/Module.php';
+
+ $loader = new ModuleLoader(array('Baz', 'Foo'));
+ $baz = $loader->getModule('Baz');
+ $this->assertTrue($baz instanceof \Baz\Module);
+ $foo = $loader->getModule('Foo');
+ $this->assertTrue($foo instanceof \Foo\Module);
+ }
+
+ public function testCanLoadModulesWithPath()
+ {
+ $loader = new ModuleLoader(array(
+ 'Baz' => __DIR__ . '/../../_files/Baz',
+ 'Foo' => __DIR__ . '/../../_files/modules-path/with-subdir/Foo',
+ ));
+
+ $fooObject = $loader->getServiceManager()->get('FooObject');
+ $this->assertTrue($fooObject instanceof \stdClass);
+ }
+
+ public function testCanLoadModulesFromConfig()
+ {
+ $config = include __DIR__ . '/../../_files/application.config.php';
+ $loader = new ModuleLoader($config);
+ $baz = $loader->getModule('Baz');
+ $this->assertTrue($baz instanceof \Baz\Module);
+ }
+
+ public function testCanGetService()
+ {
+ $loader = new ModuleLoader(array('Baz' => __DIR__ . '/../../_files/Baz'));
+
+ $this->assertInstanceOf(
+ 'Zend\ServiceManager\ServiceLocatorInterface',
+ $loader->getServiceManager()
+ );
+ $this->assertInstanceOf(
+ 'Zend\ModuleManager\ModuleManager',
+ $loader->getModuleManager()
+ );
+ $this->assertInstanceOf(
+ 'Zend\Mvc\ApplicationInterface',
+ $loader->getApplication()
+ );
+ }
+}
View
3  tests/ZendTest/Test/_files/application.config.php
@@ -1,11 +1,12 @@
<?php
+
return array(
'modules' => array(
'Baz',
),
'module_listener_options' => array(
'config_cache_enabled' => true,
- 'cache_dir' => __DIR__ . '/cache',
+ 'cache_dir' => sys_get_temp_dir(),
'config_cache_key' => 'phpunit',
'config_static_paths' => array(),
'module_paths' => array(
Something went wrong with that request. Please try again.