Replace the SimpleTest framework with PHPUnit.
Problem/Motivations
The SimpleTest framework no longer has the same level of maintenance than the PHP Unit framework. PHPUnit has become the standard; most frameworks use it (ZF1 & 2, Cake, Symfony2, ...) and is actively maintained, sable and works great for every codebase, scenario.
Replacing the testing framework with PHPUnit will allow better consistency in the drupal 8 codebase, by aligning ourselves with Symfony2 testing tools. Moreover, it integrates better with IDE (NetBeans, Eclipse, IntelliJ IDEA), Continuous Integration servers such as Jenkins/Hudson, and with Sonar for static code analysis as well. It has also code coverage which is a huge +.
Proposed resolution
Convert existings tests from SimpleTest to PHPUnit, in three "simple" steps:
- Unit tests can be converted quasi-verbatim, requiring some minor adjustement due to the difference of APIs. They will extend the
Drupal\Core\Testing\UnitTestBase
class. <- DONE ! - Database tests, such as UserSaveTest, which only asserts Database interaction, will extend the
Drupal\Core\Testing\DatabaseTestBase
. These tests will require some refactoring as we need to mock the needed subsystems such as config, cache, etc. <- Work is in progress in that area, see the commits. - Functional tests a.k.a our WebTestCase. These tests will requires a minimal bootstrapped environment to be run, and an internal browser, such as Guzzle. Because recreating a mocked environment like we do in DatabaseTests would take too much time, we'll bootstrap a DrupalKernel. Also as PHPUnit assumes database schemas already exists we have to write procedure to drop them in order keep a good level of isolation in the test suit. <- No work has been done yet.
Work In Progress
As of 04 January 2013, the first step has been completed, the second is work in progress and APIs are willing to change a lot to polish developer experience. The third step has not started yet. It follows every agreements in #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest except one:
- Install Drupal with (stripped down) testing profile + class-specific module(s).
When running UnitTest, we are not installing Drupal at all. For now the bootstrap only instantiate the class loader, and creates an empty container. The Kernel is neither created as well.
git clone --recursive --branch test-phpunit-1801176-sylvain http://git.drupal.org/sandbox/sun/1255586.git phpunit
cd phpunit
php phpunit.php
Profit.
Meta Issue
Related
Comment | File | Size | Author |
---|---|---|---|
#24 | coverage.png | 259.55 KB | Sylvain Lecoy |
#12 | PHPUnitPOC.patch | 102.74 KB | Sylvain Lecoy |
#4 | PHPUnit-POC-bottom-up.patch | 4.54 KB | Sylvain Lecoy |
Comments
Comment #0.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commented.
Comment #0.1
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commented;
Comment #0.2
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedadded NetBeans phpunit
Comment #0.3
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdded UPal.
Comment #1
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdding tag.
Comment #1.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdded Agile Unit Testing book page.
Comment #2
sunPrior art: #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest
I actually had plenty of discussions and conf calls with various people on this topic and a battle plan / roadmap already.
Comment #3
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedDuplicate of #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest.
Can you add the Agile Unit Testing link to your issue ? As well as some (brief) description about problems and motivations ?
Also, please direct me to work in progress if any, Moshe told me to see with you directly.
Comment #3.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedtypo.
Comment #4
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedHere is a very simple POC, adding the minimal required files to run the
BlockTemplateSuggestionsUnitTest
as a PHPUnit test instead of a SimpleTest one.Usage:
Make sure you have PHPUnit 3.7 installed (via PEAR or composer).
I put it as Need Review for curiosity sake of the testbot.
Comment #6
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedYes obviously, the PHPUnit_Framework_TestCase does not yet exists in the infrastructure :)
Comment #7
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedJust, if it is possible to have very early feedback on the approach I am taking it would be welcomed.
Comment #8
sunI'm here. Watching you. ;)
That's a good and simple start. :)
As mentioned in the meta issue, I don't really have time to properly think through this before feature freeze. And I strongly believe we need a carefully designed plan.
First things first, we should move the work into the Platform sandbox. I just granted you write access. Can you move your patch (or local branch) into a branch in there? Use this branch name:
Second, some considerations in arbitrary order:
There should be 5 base classes in total:
- 2 plain phpunit base classes, one for unit tests and one for web tests.
- 3 compatibility base classes, for UnitTestBase, DrupalUnitTestBase, and WebTestBase. (check how Upal does it)
We need to add Mink, required Guzzle components, and Goutte to core for phpunit web tests.
The top-level test suites declaration obviously does not work. We need a mechanism to discover and collect phpunit.xml files throughout the code base. (but omit files in any /vendor directory)
I expect to be able to run tests for my (contributed) module from the top-level/root Drupal directory.
We can temporarily convert test cases like you did for demo/proof purposes, but as mentioned in the meta issue, the primary objective for D8 is to only provide the infrastructure. It is too late in the release cycle to start test conversions.
As you already figured out, run-tests.sh/Simpletest needs to be adjusted to check for
instanceof Drupal\simpletest\TestBase
to prevent it from considering a phpunit test.We are blocked on d.o testing infra to install phpunit on testbots. Once that has happened, we need to look into concurrent/parallel test runner support in PIFR/Conduit. Unless these points have happened, phpunit tests are not supported in any way, and our efforts here will only mean that developers can only run them manually on their local machines.
These are the main, high-level aspects for me right now. Happy to discuss further, but as mentioned, I'd highly prefer to defer this effort post feature freeze.
Comment #9
cosmicdreams CreditAttribution: cosmicdreams commentedOMG OMG OMG, I can't tell you how excited I am about this. I am happy to write tests if shown the way. I will be following this intensely.
Comment #9.0
cosmicdreams CreditAttribution: cosmicdreams commentedAdded meta issue.
Comment #9.1
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedCorrected meta issue number.
Comment #10
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedHi sun and thank you for the Platform sandbox access, although I can't see the branch yet... Maybe I have to creates it when I push?
About the classes:
About Guzzle, and PHPUnit, should I add an entry in the composer.json like core does ? Plus the actual libraries or this will bloat the patch size?
So you suggest having a phpunit.xml per module (e.g. system/phpunit.xml, user/phpunit.xml) then provides a mechanism for discovering and collecting them ?
I don't understand what you mean by being able to run test from the top-level, you mean not from /core like it is proposed in the patch ?
OK for the general approach providing only the infrastructure. Secretly hope that it can happen for Drupal 8 :)
Comment #11
cweagansThere's already an RTBC issue open for Guzzle: #1447736: Adopt Guzzle library to replace drupal_http_request()
Not sure about the rest though.
Comment #12
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedSome work in progress with utilities functions only... They are mostly a mix of UPal and SimpleTest 8.x.
Will start working on the branch aiming to implement setUp() and tearDown() methods.
Comment #13
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedJust created the branch in your sandbox sun.
Comment #14
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedBack on tracks I am working on Mink integration. Tests are fun to write and I am progessing pretty fast.
I am creating a 'testing' module which will hold the infrastructure along with the core/lib/Drupal/Core/Testing package/namespace. This testing module will contains - for now - integrations tests which will asserts the integration with PHPUnit for Unit tests, and Mink for functional ones. It will serves as an example for developers how to use the new testing system. Later it will replaces the simpletest framework. Also it will be possible to completely moves the testing module out of the core.
Comment #15
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI have pushed a basic working test asserting the integration of Mink with Goutte driver.
Comment #16
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI am working on database fixtures (http://www.phpunit.de/manual/current/en/database.html), and it introduces a concept of UnitTest, and DatabaseTest, they are differents.
Both can use the Goutte/Mink driver (I think at the rest module who can runs without the database), that's why I think to drop the name of WebTestCase. Instead of WebTestCase and UnitTestCase, there will be now DatabaseTestCase and UnitTestCase. If we want to add Web possibilities, we'll use a Mink Session through object composition instead of object inheritance.
Comment #17
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI may requires some advices at this point on how to properly load a schema (per-class test) then load test data into it ? PHPUnit does a TRUNCATE but keep the schema between methods invocations for performances. Do we want to go this way ?
We could also load dump like we are doing in UpgradePathTest, but then again when ? On a per-test basis ? per-function basis ? What about setUp() and tearDown() ?
Comment #18
BerdirYou probably want to have a look at DrupalUnitTestBase which a) close to what you called DatabaseTestCase and b) provides methods to enable modules and install tables in a partially mocked environment.
Comment #19
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI am about to implement a functional test driven with PHPUnit and more specifically the Database test class, composed with the Goutte driver.
I am writing the new tests under the tests directory, thus a project will have the following:
Its a good practice to separate tests from production code. Also we'll be able to make the transition more smoothly as it will make simpletest working as is, without having to modify the current code, and PHPUnit to look at the classes inside the 'tests/' directory. I am also implementing the sun's idea, replacing
$this->drupalCreateUser()
with$this->getHelper('user')->createUser()
.I also strongly believe in "convention over configuration" that's why the example code assumes that tests classes will be under this directory. Also, the Helper classes will uses the following convention - for now - UserTestHelper.php.
As soon as i get the UserCreateTest functional test working with PHPUnit and Goutte, I'll provide a patch here for architectural review. In the meantime, you can check the branch on sun's Drupal sandbox where i'm committing work in progress.
Comment #20
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedBtw, thanks Berdir for pointing me out where to look :)
I have a few conceptual questions, in my last commit which aim to integrates a UserCreateTest, I have to set-up the database.
@see http://drupalcode.org/sandbox/sun/1255586.git/blob/refs/heads/test-phpun...
In the test class i'm using PDO as a sqlite memory implementation. So I am going to use this to set-up my fixture. I want also the module to be tested to implements its schema.
In #913086: Allow modules to provide default configuration for running tests, sun speaks about config, how does it relates to PHPUnit fixtures ?
Remember that I am taking two approaches in this issue:
I don't know if people are ok with the statement above so any advices/tips would be appreciated :-)
Comment #21
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI just need directions/validations actually, a chat with sun, Moshe, or anyone involved in PHPUnit on IRC/mail should help me.
Comment #22
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedIt seems that I can't inject a PDO object.
So my next question is: Do/can we mock the DB object ?
Comment #23
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedExciting work out there, I managed to completely remove the dependency upon drupal_bootstrap() and DrupalKernel. We are now on a super-minimal mocked environment.
By the way tests could not start if you were on a fresh copy of Drupal (i.e. without default/settings.php set-up). Now its working on a fresh install, in the minimal environment possible (I think).
Grab a copy of the last commit, and run command "phpunit" in the drupal/ root folder. There is 5 basic assertions, which will asserts your environment is mostly setup correctly with Mink, Guzzle, Goutte and PHPUnit + DBUnit.
Comment #24
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedSome code coverage and IDE integration to advertise...
Comment #25
jolos CreditAttribution: jolos commentedConcerning drupal & mink/behat integration it might be worth to have a look at http://drupal.org/project/drupalextension and see if there's anything you can leverage ( assuming you're not already aware of this )
Comment #25.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedadded unit test remastered issue
Comment #26
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedSure I'll have a look, I was not aware of this.
I am still on the PHPUnit part, then i'll work on the Database/Web test cases. I'd just like to have early feedback if possible because I know that working as submarine is never good. One day there is always a person who wakes up and says: this is not what we want to do :-(
I am committing really often and posting progress here, so i'd really appreciate to have early reviews as well.
EDIT: I simplified the process to be run with truely zero configuration: NO-PEAR, No-PHPUnit, No need to have a default site set-up (no settings.php required), etc. Everything is baked with the software, just do the following:
Regards
EDIT: Updated issue:
Comment #26.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdded work in progress.
Comment #27
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI encourage people following this issue to check this commit: http://drupalcode.org/sandbox/sun/1255586.git/commitdiff/711465fcd956cbc... and the one directly following.
I have factorized the code in builder object, which aims to manipulates MockObjects and provides default mocked services such as keyvalue, config.factory and then system.module, system.theme, etc.
The client use of this is the DatabaseIntegrationTest where you mock an environment very quickly. Of course you still have the full flexibility of MockBuilder since these classes extends the PHPUnit_Framework_MockObject_MockBuilder class.
Feedback appreciated !
Cheers and happy new year :)
Comment #28
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedI recently pushed the DatabaseIntegrationTest which asserts that installSchema() method works correctly with a mocked Connection object.
The principles are:
We creates a SQLite Connection object - here :sqlite::memory: - which is injected into the dependency container. This mocked connection will be then used by Drupal. We then install a particular schema from a defined module. Because I found module_list(), module_enable() was too dependant of many sub-systems (config.factory, keyvalue store, etc.) I decided to not use these functions in DatabaseTests. @see DatabaseTestBase::installSchema().
If we want to rely on module_list() and module_enable() like it is done in DrupalUnitTestCase, I think these belong to functional tests and not databases tests so I may re-introduce a WebTestBase class which performs a full-bootstrap on a minimal environment of pre-installed modules like it is done in UPal and with SimpleTests. These tests wont be as fast as UnitTests and DatabaseTests but functional tests are useful to test the interaction of systems.
The converting process should be more or less like this in the final state:
The fact that PHPUnit assumes that the database schema already exists is a strenght when it comes to performance, but a flaw when we will convert our existing test suit. Althought I don't think it is a design flaw, for instance upgrade tests would be run on a separate database setup, we have to take this difference into account when writing tests because the level of isolation between tests will not be the same. I think however that it is perfectly acceptable if we know the consequences.
Comment #29
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedIn this commit I partially converted the UserSaveTest (no-web based, only DB) to PHPUnit.
I had to mock the entity system, plugin system, and cache system as well. The test is verbose but it is a first attempt as a bottom-up approach to understand how to isolates properly systems.
I created a bunch of MockBuilder object for every subsystems such as ConfigFactory, Config, CacheFactory, etc. and fluid-style interface to configure them. Feedback is still welcome. Grab the last commit and run the test suit to see the UserSaveTest in action; the class is located at /core/modules/user/tests/Drupal/user/Tests/UserSaveTest.php.
Comment #29.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdded code to fetch the project.
Comment #30
jhedstromTrying to review this, I first needed to hack testing.inc, since my D8 instance doesn't run by itself at localhost. Also, there appeared to be one too many directories in the `DRUPAL_ROOT` definition:
After those changes, I was able to run phpunit, but no tests were found:
Edit: I meant to mention that it seems like an ideal fix would be to allow the specification of the `REMOTE_ADDR` in the phpunit.xml file.
Comment #31
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedHello jhedstrom, I greatly appreciate your feedback, thanks :)
I totally agree that the remote_addr should be parametrized in the phpunit.xml file, good point. Thanks for the bug fix as well, i'll push it.
It is wierd that you didn't found any test, are you using the HEAD ? I'll have a look on this.
Sylvain
Comment #32
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedYes actually go to phpunit.xml and uncomment the following line:
You can comment the other lines as they are not in sync with Drupal HEAD anymore (I get a Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "module_handler".)
The test suit Testing makes basic assertion about your setup. You can have a look at the User test suit which makes databases and more advanced assertions, and Core test suit which mock the filesystem and makes integration tests.
Comment #33
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAlternatively, you can grab my last commit and execute the test suit.
Comment #34
franskuipers CreditAttribution: franskuipers commentedThis issue is mentioned here #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest.
Next week I would like to investigate your branch after #1901670: Start using PHPUnit for unit tests.
Comment #35
RobLoachTo install PHPUnit with Drush Composer and run the tests, it is:
Comment #36
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedHey franskuipers thanks for giving me some attention, I guess we have some basic integration in #1901670: Start using PHPUnit for unit tests and you can focus on reviewing some mocking objects that I've proposed in this branch.
I think there will be some material to take and I'd be happy to continue working on it and/or to help on the subject. Let me know :)
Comment #36.0
Sylvain Lecoy CreditAttribution: Sylvain Lecoy commentedAdded plan
Comment #37
dawehnerWe also have functional tests and what you define as database tests (kernel tests).
I think this issue should be fixed with that.