Remove ZendTest from Composer #4744

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

bakura10 commented Jun 28, 2013

No description provided.

@@ -3,17 +3,12 @@
* Setup autoloading
*/
-if (file_exists(__DIR__ . '/../vendor/autoload.php')) {
- include_once __DIR__ . '/../vendor/autoload.php';
@weierophinney

weierophinney Jun 28, 2013

Owner

We'll still need to pull in the Composer autoloader for things like doctrine/annotations, the proxymanager, etc. The primary point is that in addition to this, we also need to use the StandardAutoloader to autoload the test classes.

tests/_autoload.php
-}
+include_once __DIR__ . '/../vendor/autoload.php';
+
+require_once __DIR__ . '/../library/Zend/Loader/StandardAutoloader.php';
@Ocramius

Ocramius Jun 28, 2013

Member

Not needed \o/

tests/_autoload.php
+$loader = new Zend\Loader\StandardAutoloader(
+ array(
+ Zend\Loader\StandardAutoloader::LOAD_NS => array(
+ 'Zend' => __DIR__ . '/../library/Zend',
@Ocramius

Ocramius Jun 28, 2013

Member

Not needed

tests/_autoload.php
+ 'Zend' => __DIR__ . '/../library/Zend',
+ 'ZendTest' => __DIR__ . '/ZendTest',
+ ),
+ ));
@Ocramius

Ocramius Jun 28, 2013

Member

Closing parenthetical misaligned with opening one

Owner

weierophinney commented Jun 28, 2013

Perfect, @bakura10 -- thanks!

weierophinney added a commit that referenced this pull request Jun 28, 2013

weierophinney added a commit that referenced this pull request Jun 28, 2013

@ghost ghost assigned weierophinney Jun 28, 2013

What's the reason why the ZendTest namespace is removed? We are inheriting some ZendTest classes in our testsuite and relying on Composer autoloading.

Contributor

bakura10 replied Jul 2, 2013

Since a few release, test folder is not included when you download ZF 2 through composer (you need to explicitly add the --prefer-source option to composer so that it retrieves the tests also). However, this was problematic when you were dumping an optimized class map autoloader because Composer assumed there was a ZendTest folder, but didn't find the files, and crashed.

Owner

weierophinney replied Jul 2, 2013

@arjanvdbos -- the namespace is still present, we're simply using the StandardAutoloader in order to load that namespace. You can do similarly in your own test suite if you need to use any of our test assets -- just point the autoloader at the test directory.

@weierophinney -- I’ll understand, but the downside of this approach is that we always manually have to modify the autoloading when we add a dependency and consuming their tests. It would be nice when Composer implement a autoload-dev, which imho would be a nicer solution. See Added an autoload-dev section.

Contributor

gerardroche commented on 3bf60a3 Aug 30, 2013

Doesn't this break autoloading without using composer? i.e. clone zf2, cd into tests dir, run phpunit ZendTest/Escaper/EscaperTest.php and you get errors:

PHP Fatal error:  Class 'Zend\Loader\StandardAutoloader' not found in /path/to/zf2/tests/_autoload.php on line 8

I had to fix it by adding back what was removed.

Owner

weierophinney replied Sep 3, 2013

@gerardroche We require that you run a composer.phar install in order to run unit tests. As such, the StandardAutoloader will be autoloadable, allowing us to configure it.

We went this route because it makes the following more straight-forward for contributors:

  • The PHPUnit version(s) we support will be immediately available
  • Autoloading is simpler
  • Any dependencies on 3rd party libraries are installed and autoloadable

So, no, no fix is needed -- just install the composer dependencies to ensure you have what is needed to run the tests.

Contributor

gerardroche replied Sep 3, 2013

@weierophinney The thing is I didn't need composer or any dependencies to get what I needed done. I only needed to test one component so I reverted this change and added a file_exists for __DIR__ . '/../vendor/autoload.php' because it was spitting out a php warning about the file not existing.

Can we not have both? i.e. you can run tests without composer or dependencies but all bets are off if you do.

Owner

weierophinney replied Sep 3, 2013

@gerardroche No, we cannot have both, as that would mean we then have to maintain more code to allow tests to work. Right now, the situation is:

composer.phar install
cd tests
../vendor/bin/phpunit ZendTest/SomeComponent/

We do not need to worry about PHPUnit incompatibilities, autoloading issues due to include_path issues, etc. It's a single toolchain for everybody.

Contributor

gerardroche replied Sep 3, 2013

:) cool, thanks.

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