Problem/Motivation
As #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest is underway we need to convert all Simpletest based unittest to PhpUnit tests.
Proposed resolution
Let's create patches for each directory from the list.
Make sure you use
git diff -M 8.x
to make the file move explicit which reduces the patch size and thus the reviewers tasks.
Remaining tasks
Documentation for PHPUnit's mocks and stubs: http://phpunit.de/manual/current/en/test-doubles.html
As an example for modules see breakpoint module core/modules/breakpoint/tests/Drupal/breakpoint/Tests/
For Core or Component we already have several tests core/tests/Drupal/Tests/Core|Component
.
- If a test is in the system module, and it tests a class not in system module, then it should be moved to
core/tests/Drupal/Tests/
directory - If a class contains
Unit
in its name, remove it when converting to PHPUnit..there is no point keeping it. - If a class can be converted to use dataproviders it should
As mentioned in #1938068-11: Convert UnitTestBase to PHPUnit
Open Issues
core/modules/simpletest/lib/Drupal/simpletest/Tests/MissingDependentModuleUnitTest.php cannot be converted to phpunit since it actually tests Simpletest itself (which requires Drupal bootstrap, etc). Not sure if it deserves a separate issue, or if it should just be removed from this issue summary.
Fill in the documentation void of http://drupal.org/node/1814344 Agile Unit Testing section 'Unit Test Organization'
TODO
#1996868: Start converting image.inc to an Image component
#1938130: Convert User tests to PHPUnit
#2003800: Move drupal_check_memory_limit() and parse_size() functionality to components
#2008566: Convert PasswordHashingTest unit tests to phpunit
#2035133: Convert system module's Mail unit test to phpunit
#2042739: Convert system module's ControllerResolverTest to phpunit
#2051467: Expand and convert to phpunit tests for \Drupal\Component\Transliteration
#2130551: Convert system modules MimeTypeMatcherTest to phpunit
(And then 44 more tests from system module.)
In core / lib / Drupal / Component
Component might be a good place to aim for 100% coverage.
Really though we want to make issues to remove procedural code from classes,
and declare the dependencies (dependency injection), and then add phpunit tests in the issue to "Declare dependecies in *classWhatever*".
Fill in issue numbers if related to getting those tests in, or issue numbers for new issues created to convert tests or improve coverage. Let's try and do one issue per class.
- (todo: improve. done: convert) Datetime, DateTimePlus 83% #2003698: Convert system module's Datetime unit tests to phpunit
- (done) Graph 100% #1935908: Convert Graph tests to phpunit
- Image 100%
- PhpStorage 100% #2051385: Expand phpunit tests for \Drupal\Component\PhpStorage
- FileReadOnlyStorage 77%
- FileStorage 63%
- MTimeProtectedFastFileStorage 83%
- PhpStorageFactory 52%
- Plugin 48% #2052109: [meta] Expand phpunit tests for \Drupal\Component\Plugin classes
- Discovery, StaticDiscovery 50%
- PluginBag 56%
- PluginBase 66%
- PluginManagerBase 20%
- Transliteration, PHPTransliteration 86% #2051467: Expand and convert to phpunit tests for \Drupal\Component\Transliteration
- Utitlity 67%
- Crypt 62% #2046207: Expand phpunit tests for \Drupal\Component\Utility\Crypt
- DiffArray 100%
- Json 100%
- MapArray 100%
- NestedArray 100% #2050535: Expand phpunit tests for \Drupal\Component\Utility\NestedArray
- Random 100%
- Settings 100% #2035863: Expand phpunit tests for \Drupal\Component\Utility\Settings
- String 100%
- Tags 100%
- Timer 38% #2049817: Expand phpunit tests for \Drupal\Component\Utility\Timer
- Unicode 48% #1938670: Convert unicode.inc to \Drupal\Component\Utility\Unicode, #2051717: Expand phpunit tests for \Drupal\Component\Utility\Unicode
- Url 39% #2046245: Expand phpunit tests for \Drupal\Component\Utility\Url
- Xss 83%
In core / lib / Drupal / Core / Batch
Batch has 3 classes, but 2 are not in phpunit coverage output since code is not autoloaded as part of tests.
- BatchStorage 0% #2035825: Expand PHPUnit test coverage for Drupal\Core\Batch\BatchStorage
- BatchStorageInterface 0%
- Percentage 100% #2003736: Convert system module's Batch unit tests to phpunit
Done
#2003762: Convert system module's Uuid unit tests to phpunit
#1938184: Convert Drupal\Core\Utility\Color\ColorTest to PHPUnit
#1991078: Convert MachineNameControllerTest to PHPUnit
#1901670: Start using PHPUnit for unit tests
#1935970: Convert timer_* to a utility class and convert tests to phpunit
#1957302: Convert PathProcessorTest to PHPUnit
#2001218: Convert core/modules/views/lib/Drupal/views/Tests/PluginTypeListTest.php to phpunit
#2003342: Convert system module's database unit tests to phpunit
#2002190: Convert core/modules/search/lib/Drupal/search/Tests/SearchExpressionTest.php to phpunit
#2003568: Convert tags,attributes, diff and url validation unit tests to phpunit
#2006568: Convert filter_xss tests to unit tests
#2016299: Convert system module's JsonUnitTest to phpunit
#2042745: Convert system module's RouteTest to phpunit
#2002116: Convert core/modules/update/lib/Drupal/update/Tests/UpdateCoreUnitTest.php to phpunit
#2030173: Convert system module's ValidNumberStepUnitTest to phpunit
API changes
The UnitTest files are moved to ... tbd
====
See coder #1938066: Prepare for phpunit tests
Related issues
Comment | File | Size | Author |
---|---|---|---|
#41 | interdiff.txt | 3.09 KB | sun |
#41 | convert-to-kernel-tests-1938068-41.patch | 21.96 KB | sun |
#36 | interdiff.txt | 2.94 KB | sun |
#36 | convert-to-kernel-tests-1938068-36.patch | 22.99 KB | sun |
#34 | convert-to-kernel-tests-1938068-34.patch | 21.91 KB | sun |
Comments
Comment #0.0
clemens.tolboomFixed wrong node
Comment #0.1
clemens.tolboomAdded link to coder
Comment #0.2
clemens.tolboomAdded issue summary
Comment #1
clemens.tolboomAlready done
We need to register these.
Comment #1.0
clemens.tolboomAdded current list
Comment #1.1
franskuipers CreditAttribution: franskuipers commentedUpdated issue summary.
Comment #1.2
clemens.tolboomAdded block.
Comment #1.3
clemens.tolboomAdded block
Comment #1.4
clemens.tolboomAdded already done Unittests
Comment #2
franskuipers CreditAttribution: franskuipers commentedCommands to find files/directories with UnitTests
All files:
All directories:
Current directories with UnitTestBase classes:
Comment #2.0
franskuipers CreditAttribution: franskuipers commentedAdded User module.
Comment #2.1
clemens.tolboomadded change notice
Comment #3
franskuipers CreditAttribution: franskuipers commentedComment #3.0
franskuipers CreditAttribution: franskuipers commentedUpdated issue summary.
Comment #3.1
franskuipers CreditAttribution: franskuipers commentedUpdated remaining tasks
Comment #4
Eric_A CreditAttribution: Eric_A commentedTagging phpunit.
Comment #4.0
Eric_A CreditAttribution: Eric_A commentedAdded #1957302: Convert PathProcessorTest to PHPUnit.
Comment #4.1
Eric_A CreditAttribution: Eric_A commentedAdded tests.
Comment #4.2
Eric_A CreditAttribution: Eric_A commentedThere is an issue for user already.
Comment #4.3
Eric_A CreditAttribution: Eric_A commentedSeparate Done from To do.
Comment #4.4
Eric_A CreditAttribution: Eric_A commented#57302: php error info displayed is done.
Comment #4.5
Eric_A CreditAttribution: Eric_A commentedGrouped by module.
Comment #5
Eric_A CreditAttribution: Eric_A commentedEDIT: removed simpletest framework classes from the lists.
I've added non-system module UnitTestBase test files to the issue summary. I haven't checked if these have issues already.
Perhaps the 44 system module tests should be in a separate meta issue.
Comment #5.0
Eric_A CreditAttribution: Eric_A commentedRemoved simpletest framework classes.
Comment #5.1
ParisLiakos CreditAttribution: ParisLiakos commentedAdd Unicode DUTB
Comment #5.2
ParisLiakos CreditAttribution: ParisLiakos commentedadd transliteration test
Comment #5.3
ParisLiakos CreditAttribution: ParisLiakos commentedUpdate issues with the new image dimension one
Comment #6
clemens.tolboomUpdated the summary to make sure we get patches using
git -M
Comment #7
jhedstromWorking on some of these, I notice that tests are being moved from modules into
core/tests/...
. If they are left in place, phpunit doesn't find them because they aren't being loaded from what I can tell. Is this intentional? For instance, see #1935908: Convert Graph tests to phpunit.For instance, with
core/modules/views/lib/Drupal/views/Tests/PluginTypeListTest.php
orcore/modules/search/lib/Drupal/search/Tests/SearchExpressionInsertExtractTest.php</code, should these be moved to <code>core/tests/...
, or should we updatecore/bootstrap.php
/phpunit.xml.dist
to find these where they currently live?Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedhi! thanks for working on this!
make sure you tag them with phpunit as well!
And yes unit tests for stuff in core should move in
core/tests/
and for modulesmodule_name/tests/
Comment #9
jhedstromre: #7 msonnabaum cleared this up. For modules, tests get moved from
lib/Drupal/foo/Tests
totests/Drupal/foo/Tests
.Comment #9.0
jhedstromAdded the git -M as mentioned in #1938184-6: Convert Drupal\Core\Utility\Color\ColorTest to PHPUnit
Comment #10
jhedstromre: #8oops, crosspost there. Thanks for the info!
Comment #10.0
jhedstromUpdated issue summary.
Comment #11
jhedstromcore/modules/simpletest/lib/Drupal/simpletest/Tests/MissingDependentModuleUnitTest.php
cannot be converted to phpunit since it actually tests Simpletest itself (which requires Drupal bootstrap, etc). Not sure if it deserves a separate issue, or if it should just be removed from this issue summary.Comment #11.0
jhedstromUpdated issue summary.
Comment #11.1
jhedstromUpdated issue summary.
Comment #11.2
jhedstromUpdated issue summary.
Comment #11.3
jhedstromUpdated issue summary.
Comment #11.4
jhedstromUpdated issue summary.
Comment #11.5
jhedstromUpdated issue summary.
Comment #11.6
jhedstromUpdated issue summary.
Comment #11.7
jhedstromUpdated issue summary.
Comment #11.8
clemens.tolboomAdded comment from jhedstrom to the remaind tasks list.
Moved todo issues to top of tasks.
Comment #12
clemens.tolboomWhat is this remark about?
I currently have no clue what to do to convert one of the open tasks.
Can or Should we improve the issue summary?
Comment #13
Eric_A CreditAttribution: Eric_A commentedWhen I added that remark all non-system module UnitTestBase classes were already listed in the summary (with or without a linked issue), but not the 40+ classes from system module. See #5.
Comment #14
clemens.tolboom@Eric_A but should these be listed in the todo of the issue summary as a todo or do we need XREFs to the issues or just remove it from the summary? I dunno :p
Comment #15
ParisLiakos CreditAttribution: ParisLiakos commentedAs an example for modules (for the destination question) see breakpoint module
core/modules/breakpoint/tests/Drupal/breakpoint/Tests/
For Core or Component we already have several tests.
core/tests/Drupal/Tests/
directoryUnit
in its name, remove it when converting to PHPUnit..there is no point keeping it.Comment #15.0
ParisLiakos CreditAttribution: ParisLiakos commentedMade git diff -M better visible.
Added XREF to Agile Unit Testing http://drupal.org/node/1814344 assuming that's the official documentation page.
Comment #16
clemens.tolboom@Eric_A + @ParisLiakos it would be awesome to have an up to date issue summary ;)
Comment #16.0
clemens.tolboomadded phpunit docs link
Comment #16.1
dawehneradded another test
Comment #16.2
dawehnermoved to todo
Comment #16.3
ParisLiakos CreditAttribution: ParisLiakos commentedAdd some more info and remove link to DUTB
Comment #16.4
jhedstromUpdated issue summary.
Comment #16.5
ParisLiakos CreditAttribution: ParisLiakos commentedUpdate done issues
Comment #16.6
jhedstromUpdated issue summary.
Comment #16.7
jhedstromUpdated issue summary.
Comment #16.8
jhedstromUpdated issue summary.
Comment #16.9
jhedstromUpdated issue summary.
Comment #16.10
jhedstromUpdated issue summary.
Comment #16.11
jhedstromUpdated issue summary.
Comment #16.12
YesCT CreditAttribution: YesCT commentedorganizing.
Comment #16.13
YesCT CreditAttribution: YesCT commentedadded heading
Comment #16.14
YesCT CreditAttribution: YesCT commentedadded some more class detail.
Comment #16.15
YesCT CreditAttribution: YesCT commentedadding a section for batch, why? because alexpott explained stuff using batch as an example and I dont want to lose that knowledge. -y
Comment #16.16
YesCT CreditAttribution: YesCT commentedadded a trial issue for Settings test coverage. need to check with experienced people to see what approach to take for creating issues and describing them.
Comment #16.17
jhedstromUpdated issue summary.
Comment #16.18
jhedstromUpdated issue summary.
Comment #16.19
jhedstromUpdated issue summary.
Comment #16.20
jhedstromUpdated issue summary.
Comment #16.21
jhedstromUpdated issue summary.
Comment #16.22
jhedstromUpdated issue summary.
Comment #16.23
jhedstromUpdated issue summary.
Comment #16.24
jhedstromUpdated issue summary.
Comment #16.25
jhedstromUpdated issue summary.
Comment #16.26
jhedstromUpdated issue summary.
Comment #16.27
jhedstromUpdated issue summary.
Comment #16.28
YesCT CreditAttribution: YesCT commentedadded @ to get an overview sense of what is available and who is doing what. @ shows who it is assigned to.
Comment #16.29
jhedstromUpdated issue summary.
Comment #17
Mile23BTW: Working on this #2032697: Create PHPUnit example module if anyone wants to opine.
Also made this: http://drupalcoverage.gopagoda.com/
Far from perfect, but hopefully useful.
Comment #17.0
Mile23Updated issue summary.
Comment #18
YesCT CreditAttribution: YesCT commentedadded #2097275: Add code coverage whitelist configuration to phpunit.xml.dist to improve accuracy of results to the summary
Comment #18.0
YesCT CreditAttribution: YesCT commentedadded issue about accuracy of code coverage.
Comment #18.1
jhedstromUpdated issue summary.
Comment #18.2
jhedstromUpdated issue summary.
Comment #19
jhedstromComment #20
sunThanks a ton to everyone who is working on this herculean effort! :-)
It looks like we're missing quite a few tasks for remaining
UnitTestBase
classes though?→ Class hierarchy of UnitTestBase
Our ultimate goal should be to remove
UnitTestBase
prior to the release of Drupal 8.0.0. All modules should use PHPUnit for unit testing. For Simpletest, onlyDrupalUnitTestBase
will remain for API-level integration tests that have access to the kernel, service container, and extension system.Comment #21
Mile23Just wanted to point out that, thanks to expanded use of
@covers
and@coversDefaultClass
, the previously exciting and lovely 100% coverage for \Drupal\Component is now 38% at best.Which is good, I guess. :-)
Comment #22
sunWhat's the current status of this effort? It looks like we're very close to completion?
Out of the remaining, quite a few should simply be converted from
UnitTestBase
toKernelTestBase
, because the tests implicitly assume a preexisting "parent site" (test runner), which happens to allow them to futz with settings, services, and filesystem paths, even though they are supposed to operate in a completely empty unit test environment.It would be great to get rid of
UnitTestBase
as soon as possible.Comment #23
Mile23Improved link: Class heirarchy of UnitTestBase
https://api.drupal.org/api/drupal/core%21modules%21simpletest%21src%21Un...
Comment #24
sunTo clarify my question:
All of the remaining Child issues of this meta issue are about expanding test coverage of existing/converted phpunit tests. Are there issues to convert the remaining tests based on
UnitTestBase
that might not be linked to this parent/meta issue?Comment #25
BerdirI think not.
The attached patch converts all classes that extend directly from UnitTestBase (except KernelTestBase itself) to extend from KernelTestBase). Most are working fine and can be cleaned up nicely, but some are failing, in some cases expected (the GetFilename test) in others not, did not investigate yet.
Comment #27
sun+1 — Fixed the remaining test failures + cleaned up.
Comment #28
sunCreated the final follow-up issue: #2324789: Remove UnitTestBase
Comment #29
jibranThis class missing docs.
Comment #30
sunAdded a comment.
Comment #31
moshe weitzman CreditAttribution: moshe weitzman commentedNice cleanup Berdir and Sun.
Comment #32
tim.plunkettPatch no longer applies.
It's confusing to me that this patch is in the meta, which would imply that it is the final patch needed, yet KernelTestBase still extends UnitTestBase, and we still have #2324789: Remove UnitTestBase.
Perhaps this issue should be combined with that, properly removing UnitTestBase? Or this one needs to be temporarily renamed, or split out.
Comment #33
jibranThanks for the doc fix @sun. You going to reroll it anyway so minor nitpick.
I know this is stub class in phpunit that is why I am asking. Is it a good idea to add inheritdoc here?
Comment #34
sunRe-rolled against HEAD. Interdiff wasn't possible due to conflicts.
@jibran: Didn't add those, because it's a temporary, test-only, no-op class that implements a single interface only, and no other code can use it. The PHPDoc on the class is sufficient.
Comment #36
sunComment #37
tim.plunkettThis was skipped in the recent round of feedback.
Comment #38
sun@tim.plunkett: The vast majority of work happened in sub-issues. The patch performs the last remaining changes to complete the effort.
Intentionally created #2324789: Remove UnitTestBase as a separate issue, because it's a generic test base class. We don't have to break contrib tests immediately. The base class can be removed separately. Its removal is not necessary to complete the goal of converting all tests.
Comment #39
tim.plunkettYes, and the sky is blue and the earth is round. Thanks for explaining.
Comment #40
dawehnerThis patch looks really fine. I especially like the KV test changes, as they have a much higher readability.
Is there a reason we don't use $this->container->get('keyvalue') instanceof ... ? This makes it for example easier to read.
Totally unrelated but I realized we could improve the DX here in general: #2328129: Make KeyValueStoreInterface and StateInterface chainable
Comment #41
sunAddresses #40 + simplifies
GetFilenameUnitTest
.GetFilenameUnitTest
could be converted into a phpunit unit test, but we do not add/use phpunit tests for legacy procedural functions. Instead, phpunit tests are only added when things are converted into classes/services, which makes sense.Therefore, this is the only test that will temporarily use KTB even though it doesn't need a kernel/container. But we should get rid of
drupal_get_path()
+drupal_get_filename()
ASAP anyway.Comment #43
sunWould be great to move forward here.
Comment #44
dawehnerstill don't like vsprintf but well, no actual problem here.
Comment #45
webchickCan't find any issues in glancing through, and this has been RTBC long enough to give someone else a chance to raise them if there are any. :)
Committed and pushed to 8.x. Thanks!