Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The process of migrating to Twig was/is largely held up due to each patch requiring extensive manual testing and comparison of before/after markup.
It's obvious that originally the intention was to minimise the amount of work required by not implementing automated tests for every theme function/template but in hindsight, it's doubtful that we even saved that much time overall and we now have a lot less to show for our efforts than if we'd taken the time to write tests upfront and skipped the repetitive and painstaking manual HTML reviews.
The only time we *ever* want to change rendered HTML in core is when we're fully aware of what we're doing and why, so the current lack of test coverage is a bit scary.
This meta issue is about testing actual rendered markup strings, not the underlying structure of the render system.
We have essentially zero test coverage of our render element types. What we have can be found in /core/modules/system/lib/Drupal/system/Tests/Common/RenderElementTypesTest.php.
Tests for the theme functions are interspersed with other tests so it is very hard to see what is being tested and what is not. The testing seems to be done in a very non-standard way for theme functions.
A few that I found:
- image styles /core/modules/image/lib/Drupal/image/Tests/ImageThemeFunctionTest.php
- item lists /core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php
In light of #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system it makes sense to me (in the context of testing rendered markup) to only test the array('#theme') form of theme hooks and not try to validate _theme() directly.
Comments
Comment #1
star-szrAdding a related issue.
Comment #2
joelpittetWe need a bit more guidance on the scope of this issue as it seems quite broad.
There are over ~136 core twig templates and ~56 render elements (#type).
Comment #3
thedavidmeister CreditAttribution: thedavidmeister commented@joelpittet - that is the scope, yes, it is broad but this is just because of the number of different templates we have. There are no templates that should not have tests to ensure the rendered HTML is consistent when the template is refactored.
Comment #4
joelpittet@thedavidmeister what do we gain from all that busy work? Sorry to be pessimistic but that sounds like a lot of work for little gain. We can do it but we really need a carrot worth reaching for.
Also for things that have poor templates or cores is different from stable/classy things get tricky. Example block.html.twig template in classy has an inner div whereas everywhere else it doesn't.
This can go in any time but it really needs an ROI, IMO, FTW.
Comment #5
joelpittet