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
| Comment | File | Size | Author |
|---|
Issue fork drupal-3590917
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:
- 3590917-introduce-a-common
changes, plain diff MR !15813
Comments
Comment #3
mondrakeComment #4
mondrakeComment #5
kingdutchChange 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
BuildTestBaseneeds 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.Comment #6
quietone commentedI 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.
Comment #7
mondrakeI rewrote the docblock, hopefully for the better. Thanks @quietone. Not a native English speaker so further input greatly appreciated.
Comment #8
quietone commented@mondrake, thanks for improving the doc block. It is much better!
I am restoring RTBC
Comment #9
needs-review-queue-bot commentedThe 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.
Comment #10
mondrakerebased and updated baseline
Comment #11
needs-review-queue-bot commentedThe 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.
Comment #12
mondrakerebased
Comment #13
catchThis looks good to me but unfortunately the deprecations need updating for 11.4 -> 11.5
Comment #14
mondrakeWell, actually we are moving an already deprecated method from the trait to the base class. Not adding any deprecation of their own here.
Comment #15
catchThat'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.