Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The Rabbit Hole repository contains a number of webtests based on based on Simpletest but this test framework is deprecated in favor of PHPUnit.
Proposed resolution
Convert the existing webtests extending \Drupal\simpletest\WebTestBase
to PHPUnit.
Remaining tasks
- Write a patch
- Review
- Commit
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#8 | 3039164-6.patch | 16.07 KB | jonas139 |
#14 | 3039164--interdiff-6-14.txt | 3.06 KB | oknate |
#14 | 3039164-14.patch | 16.62 KB | oknate |
#22 | rabbit_hole- AFTER_cli_output_phpunt-3039164-22.txt | 15.88 KB | lolandese |
#22 | rabbit_hole- BEFORE_cli_output_phpunt-3039164-22.txt | 1.34 KB | lolandese |
Comments
Comment #2
jonas139 CreditAttribution: jonas139 at iO commentedComment #3
JeroenTComment #4
jonas139 CreditAttribution: jonas139 at iO commentedI've converted the simpletests to BrowserTests for PHPUnit and also fixed some TODO's and deprecated code.
For now I only get 1 deprecated code message about the use of getStatus() instead of getStatusCode() for the BrowserKit Response but this is out of scope for this issue.
Comment #5
rosinegrean CreditAttribution: rosinegrean at PitechPlus commentedComment #6
JeroenTComment #7
jonas139 CreditAttribution: jonas139 at iO commentedApparently my tests were in the wrong folder (thanks @idebr for pointing out).
Now the patch does have the correct paths.
Comment #8
jonas139 CreditAttribution: jonas139 at iO commentedThis one is the correct one...
Comment #9
JeroenT.
Comment #10
JeroenTGreat work on this @jonas139!
Code looks good and I tried the patch and the tests pass locally.
This is a nice improvement for this module. We should also enable automated testing.
Comment #11
oknateLooks good. A few nits. These two functions are only used once. Perhaps the functions aren't needed. Most tests just create their content types and content in the set up function.
Comment #12
jonas139 CreditAttribution: jonas139 at iO commentedI was thinking the same but kept this methods because I wasn't sure this should be covered in this issue.
Comment #13
oknateAh, good point. Perhaps refactoring should be done in a separate issue.
Comment #14
oknateAdding coding standards fixes.
Comment #15
oknateComment #16
jonas139 CreditAttribution: jonas139 at iO commentedLooks good to me (thanks :-)).
Comment #17
jonas139 CreditAttribution: jonas139 at iO commentedComment #18
jenlamptonI believe that if you add drupal 9 as an option to the
composer.json
file in this project you can get a nice little Compatible with Drupal 9 badge in the Project information section on the module page.Below is a code sample from a module that has the badge.
It also looks like it may possible to get the badge by adding the 'core_version_requirement' key in the modules info.yml file, which, in turn, will add the version to the require section of
composer.json
. Example follows.Do you want to include this change in the patch here or open a separate issue?
Comment #19
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedTo get the D9 compatibility badge, you just need to put the
core_version_requirement: ^8 || ^9
in the module info file. See Disable HTML5 validation module for a simple example; that project doesn't have acomposer.json
.In the case of rabbit hole, the
core_version_requirement: ^8 || ^9
will need to be added to each submodule.Comment #20
jonas139 CreditAttribution: jonas139 at iO commentedI've created a sub issue to make the changes to the compatiblity
Comment #21
Berdir> To get the D9 compatibility badge, you just need to put the core_version_requirement: ^8 || ^9 in the module info file
There is no automatic compatibility badge. There's just a free-text field where the maintainers can fill in whatever they want.
Comment #22
lolandese CreditAttribution: lolandese at Cognizant Technology Solutions commentedSustaining the RTBC status.
Just providing the terminal output of running PHPUnit on my local machine BEFORE and AFTER applying the #14 patch, showing it works as expected. Impressed by the amount of automated testing going on. Maybe better to show the output as attached text files.
In summary
BEFORE
PHP Fatal error: Uncaught Error: Class 'Drupal\system\Tests\Plugin\PluginTestBase' not found in /var/www/html/brown.localhost/web/modules/contrib/rabbit_hole/src/Tests/RabbitHoleBehaviorPluginTest.php:12
:(
AFTER
OK (29 tests, 205 assertions)
:)
Comment #23
John Cook CreditAttribution: John Cook at Creode commentedI've checked the patch against upgrade status. There is still an outstanding issue:
Because of this I've set the status back to Needs work, and added the Novice tag to add the
core_version_requirement
option.Comment #24
JeroenT@John Cook,
This issue is not about Drupal 9 compatibility. This is converting the simpletests to PHPUnit tests.
Drupal 9 compatibility is handled in this issue: #3091139: Drupal 9 compatibility
Comment #25
jonas139 CreditAttribution: jonas139 at iO commentedI agree with @JeroenT, compatibility is covered in issue #3091139: Drupal 9 compatibility
Comment #27
Dylan Donkersgoed CreditAttribution: Dylan Donkersgoed as a volunteer commentedI've merged this in.