Problem/Motivation

Maybe it's time to have a common ancestor class across suite test base classes for Drupal Core:

  • Drupal\Tests\UnitTestCase
  • Drupal\KernelTests\KernelTestBase
  • Drupal\Tests\BrowserTestBase
  • Drupal\BuildTests\Framework\BuildTestBase

Steps to reproduce

Proposed resolution

Introduce a DrupalTestCase class that is the base class of all the base classes above.

In 99%. of the cases nothing changes. If you are writing a Drupal unit, kernel, build, functional, or functional-javascript test, you will keep extending the current test base classes, that do all the setup needed. That means 100% of contrib and custom tests. Only if you are developing a test for one of the Drupal\Component libraries, you should extend directly PHPUnit\Framework\TestCase as it's forbidden to mix Core code with Component code. However, since you may be running the Component tests with the same phpunit.xml configuration as the rest of Drupal tests, then you will benefit from the PHPUnit extensions that Drupal provides. These extensions have related methods that are included in DrupalTestCaseTrait ; if it's necessary to leverage on those methods, then you'll have to use the trait explicitly.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3590917

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:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Issue summary: View changes
Status: Active » Needs review
mondrake’s picture

Issue summary: View changes
kingdutch’s picture

Status: Needs review » Reviewed & tested by the community

Change makes sense. As a recent contributor to the test scaffolding, understanding the different traits can be difficult and they seem mostly a historical artifact. I also found it particularly difficult to trace different properties to different shared traits.

Having the shared base class makes it explicit what is the absolute minimum to test anything related to Drupal, with the traits being able to offer re-usable shared logic useful for other things (such as the error handling left in DrupalTestCaseTrait).

I discussed the PHPStan baseline addition with mondrake since it's usually a red flag for me. However, in this case BuildTestBase needs its own implementation because it requires more complex logic. So in this case adding the deprecation to the baseline and not adding any attributes that it's expected is the way to go. #1792310: Wrong DRUPAL_ROOT with non-standard code structure (possibly with #3590337: Introduce an AppContext object to keep track of the configured Drupal application context) is a good candidate to resolve and reconcile the class needing its own implementation.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I read the issue, the MR and updated credit.

The comment added to the test trait is dense and I think needs rewording to plain English. I have tried that but I need to know more about what the last sentence is for. I have left a comment in the MR.

mondrake’s picture

Status: Needs work » Needs review

I rewrote the docblock, hopefully for the better. Thanks @quietone. Not a native English speaker so further input greatly appreciated.

quietone’s picture

Status: Needs review » Reviewed & tested by the community

@mondrake, thanks for improving the doc block. It is much better!

I am restoring RTBC

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new91 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.

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

rebased and updated baseline

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new91 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.

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

rebased

catch’s picture

Status: Reviewed & tested by the community » Needs work

This looks good to me but unfortunately the deprecations need updating for 11.4 -> 11.5

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

Well, actually we are moving an already deprecated method from the trait to the base class. Not adding any deprecation of their own here.

catch’s picture

Status: Reviewed & tested by the community » Fixed

That's a very good point.

Committed/pushed to main, thanks!

This doesn't apply to 11.x, since there are no concrete test changes I think we can probably skip the backport so moving to fixed. But we could re-open for backport if we want to.

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.

  • catch committed fc3581f4 on main
    feat: #3590917 Introduce a common ancestor class to all Core base test...

Status: Fixed » Closed (fixed)

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