Refactor all usages of drupal_render()/Renderer::render() that break #2450993

RTBC, I reviewed all interdiffs already.

Looks pretty great in general!

1. +++ b/core/modules/simpletest/src/Tests/KernelTestBaseTest.php
   @@ -291,11 +293,11 @@ function testEnableModulesTheme() {
      /**
   diff --git a/core/modules/simpletest/src/Tests/SimpleTestTest.php b/core/modules/simpletest/src/Tests/SimpleTestTest.php
   
   diff --git a/core/modules/simpletest/src/Tests/SimpleTestTest.php b/core/modules/simpletest/src/Tests/SimpleTestTest.php
   index 2514d9d..479ca39 100644
   
   index 2514d9d..479ca39 100644
   --- a/core/modules/simpletest/src/Tests/SimpleTestTest.php
   
   --- a/core/modules/simpletest/src/Tests/SimpleTestTest.php
   +++ b/core/modules/simpletest/src/Tests/SimpleTestTest.php
   
   +++ b/core/modules/simpletest/src/Tests/SimpleTestTest.php
   +++ b/core/modules/simpletest/src/Tests/SimpleTestTest.php
   @@ -24,7 +24,7 @@ class SimpleTestTest extends WebTestBase {
   
   @@ -24,7 +24,7 @@ class SimpleTestTest extends WebTestBase {
       *
       * @var array
       */
   -  public static $modules = array('simpletest', 'test_page_test');
   +  public static $modules = ['simpletest'];
    
      /**
       * The results array that has been parsed by getTestResults().
   

   This change is really not obvious ...

2. +++ b/core/tests/Drupal/Tests/Core/Form/EventSubscriber/FormAjaxSubscriberTest.php
   @@ -64,9 +58,8 @@ protected function setUp() {
   -    $this->renderer = $this->getMock('\Drupal\Core\Render\RendererInterface');
        $this->stringTranslation = $this->getStringTranslationStub();
   
   @@ -171,9 +164,21 @@ public function testOnExceptionBrokenPostRequest() {
   +    $renderer = $this->getMockBuilder('Drupal\Core\Render\Renderer')
   +      ->disableOriginalConstructor()
   +      ->getMock();
   +    $renderer->expects($this->once())
          ->method('renderRoot')
   -      ->willReturn($rendered_output);
   +      ->with()
   +      ->willReturnCallback(function (&$elements) use ($rendered_output) {
   +        $elements['#attached'] = [];
   +        return $rendered_output;
   +      });
   +    $container = new ContainerBuilder();
   +    $container->set('renderer', $renderer);
   +    \Drupal::setContainer($container);
    
   

   This is a bit confusing, why can we no longer mock the interface? ... I think it is actually a step backward, as we seem to call out to the global container now, but haven't earlier

few things are filed as child issues in related

Back to RTBC, there are still some inconsistencies e.g. rendering vs. not rendering in Ajax responses, but that had been present before, such this is not making it worse, but better, because early rendering is moved to rely on the real renderRoot or renderPlain functions that should be used.

I still don't get why we need to mock the actual instance of the class

The question is why:

+ $renderer = $this->getMockBuilder('Drupal\Core\Render\Renderer')

and not

+ $renderer = $this->getMockBuilder('Drupal\Core\Render\RendererInterface')

It should not matter in difference ...

Spin-off of #2450993: Rendered Cache Metadata created during the main controller request gets lost and hence a hard blocker for that one.

Therefore: Critical.

Committed 68d4f0b and pushed to 8.0.x. Thanks!

diff --git a/core/tests/Drupal/Tests/Core/Form/EventSubscriber/FormAjaxSubscriberTest.php b/core/tests/Drupal/Tests/Core/Form/EventSubscriber/FormAjaxSubscriberTest.php
index 007781a..8d0f6bb 100644
--- a/core/tests/Drupal/Tests/Core/Form/EventSubscriber/FormAjaxSubscriberTest.php
+++ b/core/tests/Drupal/Tests/Core/Form/EventSubscriber/FormAjaxSubscriberTest.php
@@ -166,9 +166,7 @@ public function testOnExceptionBrokenPostRequest() {
     $rendered_output = 'the rendered output';
     // CommandWithAttachedAssetsTrait::getRenderedContent() will call the
     // renderer service via the container.
-    $renderer = $this->getMockBuilder('Drupal\Core\Render\Renderer')
-      ->disableOriginalConstructor()
-      ->getMock();
+    $renderer = $this->getMock('Drupal\Core\Render\RendererInterface');
     $renderer->expects($this->once())
       ->method('renderRoot')
       ->with()

Yep indeed this is better. Fixed on commit and ran tests with PHPUnit.

If you remove the mocked renderer service in the container, then this happens:

Yeah I'm not stupid ... the thing is, once you mock in a hard coded way, there is less guarantee that you can write alternative implementations one day.

Closed as duplicate #2509606: book_node_view() breaks node render caching
but other 2 issues still needs review

@Wim Leers - Child issues in #2509534: [Meta] document or refactor calls to drupal_render()