- ๐ฌ๐งUnited Kingdom joachim
Shouldn't this have been removed from phpunit.xml.dist?
<!-- To disable deprecation testing completely uncomment the next line. --> <!-- <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled"/> -->
Also, with Symfony PHPUnit Bridge, you could get deprecations to show a backtrace in tests. I don't think there's a way to do that with plain PHPUnit.
- ๐ฎ๐ณIndia Prashant.c Dharamshala
prashant.c โ made their first commit to this issueโs fork.
- @mondrake opened merge request.
- ๐ฎ๐นItaly mondrake ๐ฎ๐น
@diegopino maybe you can try and file an issue to change the exception to an assertion, and see your chances.
I'm afraid there's no way back to trigger_error: they are untestable under PHPUnit 10, and what's more, in PHP 8.4 we will have this Deprecate passing E_USER_ERROR to trigger_error().
- ๐บ๐ธUnited States smustgrave
Based on other tickets with stubs this looks inline with those too. Coverage doesn't appear to be lost.
- ๐บ๐ธUnited States DiegoPino
Hi Folks,
I agree exceptions are more manageable (when actually managed) and making tests happy (specially when testing a specific method) is great, but this really changes the behavior (from the user perspective) throwing an uncaught exception when rendering inline templates or templates via render elements that allow so, not giving an end user enough info and thus the chance to act on an invalid passed array as value.
With the previous user_error \Drupal\Core\Render\Renderer::doRender when calling
// Get the children of the element, sorted by weight.
$children = Element::children($elements, TRUE); // Line 462 of Renderer.phpWe would just get as output a bunch of errors that an user/admin could act on, but still rendering everything else on screen.
Now because the exception is not caught one basically gets a white page. From that perspective, what is your suggestion? Adding try/catch when calling this method in Renderer? And acting on that via the previous (or similar behavior) but on the caller (or callers)? Or is a failure to render the new expected behavior?
For some good reasons I agree with one of the comments shared here, just before the change was committed.
"Actually given that this isn't really an error, we can just ignore and carry on, then I think an assertion is more appropriate?"
Any suggestions would be appreciated. Thanks a lot
- ๐บ๐ธUnited States smustgrave
Based on other ones the new Stub looks good.
Not sure if CRs should be opened for the Stubs or not needed? But coverage doesn't appear to be lost.