Problem/Motivation

The GenericModuleTestBase class makes only one drupalGet() request and submits no forms, and does not use cache or sessions.

Therefore it could be converted to a kernel test.

Steps to reproduce

Proposed resolution

- Add a new base class
- Convert the empty subclasses to kernel tests.

For follow-on issues:

- convert the non-empty child classes
- deprecated the Functional base class

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3617556-nr-bot_xrh0x8lc.txt91 bytesneeds-review-queue-bot
#8 3617556-nr-bot_xnl_utx2.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3617556

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joachim created an issue. See original summary.

joachim’s picture

joachim commented
Status: Active » Needs review

This makes a new base class that's a kernel test, and converts all the child classes which are empty.

I figure the non-empty child classes should be done as follow-ups, and once all core child classes are converted, we deprecate the Functional base class.

joachim opened merge request !16741

catch’s picture

catch
he/him
Primary language English
commented

Looks like one remaining test failure:

    
    1) /builds/core/lib/Drupal/Core/ImageToolkit/ImageToolkitManager.php:70
    Using null as an array offset is deprecated, use an empty string instead
    
    Triggered by:
    
    * Drupal\Tests\media\Kernel\GenericTest::testModuleGenericIssues (4 times)
      /builds/core/modules/system/tests/src/Kernel/Module/GenericModuleTestBase.php:58
catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Test suite performance
joachim’s picture

joachim commented

> 1) /builds/core/lib/Drupal/Core/ImageToolkit/ImageToolkitManager.php:70
Using null as an array offset is deprecated, use an empty string instead

I'm guessing that missing config.

Maybe we should always install the module's default config, since it's what the previous functional tests would do.

catch’s picture

catch
he/him
Primary language English
commented

Always installing the module's default config sounds good - that's part of the goal of the test really.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3617556-nr-bot_xnl_utx2.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

joachim’s picture

joachim commented
Status: Needs work » Needs review

Rebased.

dcam’s picture

dcam commented

You're right, there is a missing config that the toolkit manager is trying to load, system.image.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3617556-nr-bot_xrh0x8lc.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

joachim’s picture

joachim commented
Status: Needs work » Needs review
dcam’s picture

dcam commented
Status: Needs review » Needs work

I double-checked the diff between the Functional and Kernel versions of the GenericModuleTestBase. I realized that the Functional version was updated last week. I left two suggestions on the MR to bring the Kernel version up to date with that change. Please check it.

joachim’s picture

joachim commented
Status: Needs work » Needs review

Done. Thanks for the review!

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

No problem. This one looks good to me.

joachim’s picture

joachim commented
Issue summary: View changes

  • catch committed 4e690fc3 on main 
    task: #3617556 convert GenericModuleTestBase (and empty child classes)...

  • catch committed 231a3fa7 on 11.x 
    task: #3617556 convert GenericModuleTestBase (and empty child classes)...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Ran a before/after:

time ddev exec phpunit -c core core/modules/taxonomy/tests/src/Functional/GenericTest.php 
PHPUnit 12.5.34 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.9
Configuration: /var/www/html/core/phpunit.xml

.                                                                   1 / 1 (100%)

HTML output was generated.
https://drupal-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_taxonomy_Functional_GenericTest-2-39735035.html


Time: 00:03.676, Memory: 10.00 MB

OK (1 test, 5 assertions)

real	0m4.525s
user	0m0.281s
sys	0m0.097s

vs.

time ddev exec phpunit -c core core/modules/taxonomy/tests/src/Kernel/GenericTest.php 
PHPUnit 12.5.34 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.5.9
Configuration: /var/www/html/core/phpunit.xml

.                                                                   1 / 1 (100%)

HTML output was generated.
https://drupal-dev.ddev.site/sites/simpletest/browser_output/Drupal_Tests_taxonomy_Kernel_GenericTest-3-24796512.html


Time: 00:02.670, Memory: 10.00 MB

OK (1 test, 9 assertions)

real	0m3.523s
user	0m0.274s
sys	0m0.098s

So approximately 1s faster per test.

Code changes all look good here. Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

joachim’s picture

joachim commented

Filed the followup: #3624021: convert remaining subclasses of GenericModuleTestBase to kernel tests, and deprecate GenericModuleTestBase.

Status: Fixed » Closed (fixed)

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