3.0 phpunit #1355

Merged
merged 29 commits into from Jun 19, 2013

Projects

None yet

4 participants

@lorenzo
Member
lorenzo commented Jun 16, 2013

This is my humble approach to not using the test shell for running core tests. I had several goals in mind

  • Not depend on any App folder configuration
  • Not mess with phpunit internals by overriding classes
  • Be able to just type phpunit to run all test so that we can add groups later
  • Have a clean way to pass env variables and use them in the phpunit init script

I also made some broken tests pass during the process. There might be some problems with sqlite and postgres, but they are due to unimplemented/broken features in the database layer.

lorenzo added some commits Jun 13, 2013
@markstory
Member

I'd love to not have the test shell at all. Ideally each app/plugin could include a phpunit.xml, and Test/bootstrap.php file to setup PHPUnit. We could generate these file as part of baking new plugins/apps. I think the test shell doesn't really contribute enough to make it worth carrying forwards.

I'm a big 👍 on the general idea.

@markstory markstory commented on the diff Jun 16, 2013
lib/Cake/Test/init.php
+]);
+
+Cake\Core\Configure::write('Log.debug', [
+ 'engine' => 'Cake\Log\Engine\FileLog',
+ 'levels' => ['notice', 'info', 'debug'],
+ 'file' => 'debug',
+]);
+
+Cake\Core\Configure::write('Log.error', [
+ 'engine' => 'Cake\Log\Engine\FileLog',
+ 'levels' => ['warning', 'error', 'critical', 'alert', 'emergency'],
+ 'file' => 'error',
+]);
+
+$autoloader = new Cake\Core\ClassLoader('TestApp', dirname(__DIR__) . '/Test');
+$autoloader->register();
@markstory
markstory Jun 16, 2013 Member

You could use the classes to method calls shorter.

@markstory markstory commented on the diff Jun 16, 2013
lib/Cake/Test/init.php
+
+Cake\Core\Configure::write('Session', [
+ 'defaults' => 'php'
+]);
+
+Cake\Core\Configure::write('Log.debug', [
+ 'engine' => 'Cake\Log\Engine\FileLog',
+ 'levels' => ['notice', 'info', 'debug'],
+ 'file' => 'debug',
+]);
+
+Cake\Core\Configure::write('Log.error', [
+ 'engine' => 'Cake\Log\Engine\FileLog',
+ 'levels' => ['warning', 'error', 'critical', 'alert', 'emergency'],
+ 'file' => 'error',
+]);
@markstory
markstory Jun 16, 2013 Member

It would be nice if the core test logs didn't modify the app logs. Perhaps use sys_get_temp_dir() instead?

@lorenzo
lorenzo Jun 16, 2013 Member

That was my first attempt and failed miserably. I can give it a second try, though

@markstory markstory commented on the diff Jun 16, 2013
lib/Cake/TestSuite/Fixture/FixtureInjector.php
+ */
+namespace Cake\TestSuite\Fixture;
+
+use \Exception;
+use \Cake\TestSuite\TestCase;
+use \Cake\TestSuite\Fixture\FixtureManager;
+use \PHPUnit_Framework_AssertionFailedError;
+use \PHPUnit_Framework_Test;
+use \PHPUnit_Framework_TestSuite;
+use \PHPUnit_Framework_TestListener;
+
+/**
+ * Test listener used to inject a fixture manager in all tests that
+ * are composed inside a Test Suite
+ */
+class FixtureInjector implements \PHPUnit_Framework_TestListener {
@markstory
markstory Jun 16, 2013 Member

I wonder if there is a way to update the TestShell so we can re-use this. That might make it easier to override fewer PHPUnit internals.

@lorenzo
lorenzo Jun 16, 2013 Member

I will do that

@lorenzo
lorenzo Jun 18, 2013 Member

I think it will require some changes that I'm not sure they are worth doing now. I'd like to just use phpunit for the app too, so there would not be much gain from changing stuff now that we are going to remove soon.

@markstory
markstory Jun 19, 2013 Member

Fair enough. Do you think that changing to using phpunit for app/plugin tests is going to be a big shock for our developers? It might mean the end of the webrunner.

@markstory markstory and 1 other commented on an outdated diff Jun 16, 2013
lib/Cake/phpunit.xml.dist
+ stopOnFailure="false"
+ syntaxCheck="false"
+ bootstrap="./Test/init.php"
+ >
+ <php>
+ <ini name="memory_limit" value="-1"/>
+ <ini name="apc.enable_cli" value="1"/>
+ </php>
+
+ <testsuites>
+ <testsuite name="CakePHP Test Suite">
+ <directory>./Test/TestCase/</directory>
+ <exclude>./Test/TestCase/Model/</exclude>
+ <exclude>./Test/TestCase/Network/Email</exclude>
+ <exclude>./Test/TestCase/Network/SocketTest.php</exclude>
+ <exclude>./Test/TestCase/Error/ErrorHandlerTest.php</exclude>
@markstory
markstory Jun 16, 2013 Member

Why are the Email, Socket and ErrorHandler tests excluded?

@lorenzo
lorenzo Jun 16, 2013 Member

All the Network folder was skipped for 3.0 (including Request and Response Tests) Those tests seem very broken right now, I will remove this line and make the tests incomplete or something

@markstory
markstory Jun 17, 2013 Member

Interesting the request/response tests were passing at one point, as were the http/client tests. I'll take a look at the failing tests in Socket/Email and see if I can get them working.

@markstory
markstory Jun 17, 2013 Member

I only get one local error due to SMTP server issues. All the other tests in the AllNetwork suite are passing.

@renan
Member

No more cross database tests?

Member

The ORM layer doesn't support it yet.

@lorenzo
Member
lorenzo commented Jun 16, 2013

@renansaddam We don't have those yet

@markstory
Member

I've fixed the sqlite tests on 3.0. Creating new connections on every test method was causing issues with memory databases.

@lorenzo
Member
lorenzo commented Jun 18, 2013

Can this be merged now?

@markstory
Member

I think so, we should probably discuss removing the test shell and only using phpunit on the cakephp-core mailing list.

@lorenzo
Member
lorenzo commented Jun 19, 2013

I will send an email today

@lorenzo lorenzo merged commit 0cb0d36 into cakephp:3.0 Jun 19, 2013

1 check failed

default The Travis CI build failed
Details
@lorenzo lorenzo deleted the unknown repository branch Jun 19, 2013
@AD7six
Member
AD7six commented Jun 19, 2013

Be able to just type phpunit to run all test

Belated: yay, 👍 to that.

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