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

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

idebr created an issue. See original summary.

jonas139’s picture

Assigned: Unassigned » jonas139
JeroenT’s picture

Issue tags: +DevDaysCluj
jonas139’s picture

Status: Active » Needs review
FileSize
15.94 KB

I'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.

rosinegrean’s picture

Issue tags: -DevDaysCluj +DevDaysTransylvania
JeroenT’s picture

Assigned: jonas139 » Unassigned
jonas139’s picture

FileSize
16.01 KB

Apparently my tests were in the wrong folder (thanks @idebr for pointing out).
Now the patch does have the correct paths.

jonas139’s picture

FileSize
16.07 KB

This one is the correct one...

JeroenT’s picture

.

JeroenT’s picture

Status: Needs review » Reviewed & tested by the community

Great 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.

oknate’s picture

Looks 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.

+    $this->createTestContentType();
+    $this->entity = $this->createTestEntity();
jonas139’s picture

I was thinking the same but kept this methods because I wasn't sure this should be covered in this issue.

oknate’s picture

Ah, good point. Perhaps refactoring should be done in a separate issue.

oknate’s picture

Adding coding standards fixes.

oknate’s picture

Status: Reviewed & tested by the community » Needs review
jonas139’s picture

Looks good to me (thanks :-)).

jonas139’s picture

Status: Needs review » Reviewed & tested by the community
jenlampton’s picture

Title: Convert automated tests based on Simpletest to PHPUnit » Convert automated tests based on Simpletest to PHPUnit / Drupal 9 Deprecated Code Report

I 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.

Screenshot of the Project information section with Compatible with Drupal 9 highlighted

Below is a code sample from a module that has the badge.

"require": {
        "php": "^7.1",
       "drupal/core": "^8 || ^9"
  },

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.

core_version_requirement: ^8 || ^9

Do you want to include this change in the patch here or open a separate issue?

andrewmacpherson’s picture

To 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 a composer.json.

In the case of rabbit hole, the core_version_requirement: ^8 || ^9 will need to be added to each submodule.

jonas139’s picture

Title: Convert automated tests based on Simpletest to PHPUnit / Drupal 9 Deprecated Code Report » Convert automated tests based on Simpletest to PHPUnit
Related issues: +#3114676: Drupal 9 compatibility

I've created a sub issue to make the changes to the compatiblity

Berdir’s picture

> 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.

lolandese’s picture

Sustaining 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)

:)

John Cook’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Drupal 9 compatibility, +Novice

I've checked the patch against upgrade status. There is still an outstanding issue:

$ drush us-a rabbit_hole
 [notice] Processing /var/www/drupalvm/drupal/web/modules/contrib/rabbit_hole.
 43/43 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


================================================================================
Rabbit Hole,  8.x-1.0-beta6
Scanned on Thu, 05/28/2020 - 09:03

FILE: web/modules/contrib/rabbit_hole/rabbit_hole.info.yml

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 0    Add `core_version_requirement: ^8 || ^9` to      
                    rabbit_hole.info.yml to designate that the module is        
                    compatible with Drupal 9. See                               
                    https://drupal.org/node/3070687.                            
--------------------------------------------------------------------------------

Because of this I've set the status back to Needs work, and added the Novice tag to add the core_version_requirement option.

JeroenT’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Drupal 9 compatibility, -Novice

@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

jonas139’s picture

I agree with @JeroenT, compatibility is covered in issue #3091139: Drupal 9 compatibility

Dylan Donkersgoed’s picture

Status: Reviewed & tested by the community » Fixed

I've merged this in.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.