Closed (fixed)
Project:
Drupal Canvas
Version:
1.x-dev
Component:
Project management
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
15 Dec 2025 at 13:06 UTC
Updated:
26 Feb 2026 at 11:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
wim leersYes, please!
Are there more things you want to do on this MR, or is it ready for review?
Comment #3
wim leersComment #4
phenaproximaI gotta do more work on this.
Comment #5
wim leersCame up again at https://git.drupalcode.org/project/canvas/-/merge_requests/497/diffs#not....
Comment #6
wim leersAnd again at #3561392-22: Test Canvas on Drupal 11.3. I'll get this moving forward again.
Comment #8
wim leersComment #9
wim leersMost of the remaining failures are in an area of known technical debt: the kernel tests reusing the fixture-intended-for-end-to-end-tests: #3531679: Decouple kernel tests from `CanvasTestSetup`: painful to work on, and VERY slow
Fixing all those would massively increase scope here. Instead, we should let that existing issue handling the conversion from
KernelTestBasetoCanvasKernelTestBase.Comment #10
wim leersA dozen or so cannot be updated, because #3531679: Decouple kernel tests from `CanvasTestSetup`: painful to work on, and VERY slow needs to happen first for them.
But other than that: all done! And it is very high impact:
(not to mention every future kernel test will be simpler)
I continued on the path that @phenaproxima started, but I now authored >50% of the MR, so asking him to sign off.
Comment #11
phenaproximaApproved. That is indeed high-impact and a good step in the direction of long-term maintainability.
I think, if I have any outstanding concerns here, is that it's not clear why some tests enable some of the modules in Canvas's dependency, tree, but not all. Culling those $modules arrays even further is probably a good piece of clean-up for another issue. We also need to be sure that we explicitly document the limitations of
CanvasKernelTestBase:We might want to bring in certain helpful traits always, just to reduce friction and improve readability. Stuff like
ContentTypeCreationTrait,MediaTypeCreationTrait, and so on.Nonetheless, these don't need to happen right here, right now. Simply standardizing on one class is a big help.
Comment #12
wim leersTo ensure we don't regress and automatically provide this as feedback: #3572401: PHPCS: require CanvasKernelTestBase to be extended, forbid extending KernelTestBase.
Comment #13
wim leersComment #14
wim leersThanks, @phenaproxima!
WDYM? I just double-checked and the only dependency (direct or indirect) of Canvas that is missing is
path_aliasThe first bit is documented since https://git.drupalcode.org/project/canvas/-/merge_requests/555/diffs?com...
The second bit I'll document.
I personally prefer explicitness over magic, and of the >60 tests using
CanvasKernelTestBase(which are all passing ATM), less than a third actually need to install various entity schemas. See attached screenshots.If we can confirm through manual testing that installing all entity schemas always does not slow the kernel tests down significantly, then I'm game. (Or, perhaps alternatively: a
protected $installEntitySchemas = FALSEboolean that tests can override!)Comment #15
phenaproximaWell...not quite. It says it installs the dependencies, but unless you're very familiar with kernel testing, you might assume that the entity schemas are installed too (as is done in functional tests). As you said, it is better to be explicit about this than to assume whoever is reading the comment knows the quirks.
Comment #16
wim leersYep, you were right — fixed that literally while you wrote that: https://git.drupalcode.org/project/canvas/-/merge_requests/555/diffs?com... 😄
Based on your feedback in #11, I also made sure that the new base class installs the
path_aliasmodule. But doing that triggered a whole avalanche of test failures, mostly because of\Drupal\filter\FilterPermissions::permissions()generating a URL to the permission's text format, which performs a path alias lookup 🙄Still, it's only needed on <50% of the
CanvasKernelTestBase-powered kernel tests!Let's ship this, and consider expanding what the base class does in future issues. This MR is the hard one. Next ones will be simpler.
Comment #18
wim leersDone!
See you next in:
Comment #20
wim leers