Issue #2893117 by Mile23, Berdir: Improve HTML caching of Simpletest UI test form

core/modules/simpletest/simpletest.module

@@ -661,10 +661,6 @@ function simpletest_clean_environment() {
   else {
     drupal_set_message(t('Clear results is disabled and the test results table will not be cleared.'), 'warning');
   }
-
-  // Detect test classes that have been added, renamed or deleted.
-  \Drupal::cache()->delete('simpletest');
-  \Drupal::cache()->delete('simpletest_phpunit');
 }
 
 /**

core/modules/simpletest/simpletest.services.yml

@@ -1,4 +1,9 @@
 services:
   test_discovery:
     class: Drupal\simpletest\TestDiscovery
-    arguments: ['@app.root', '@class_loader', '@module_handler', '@?cache.discovery']
+    arguments: ['@app.root', '@class_loader', '@module_handler']
+  cache_context.test_discovery:
+    class: Drupal\simpletest\Cache\Context\TestDiscoveryCacheContext
+    arguments: ['@test_discovery', '@private_key']
+    tags:
+      - { name: cache.context}

core/modules/simpletest/src/Cache/Context/TestDiscoveryCacheContext.php

@@ -0,0 +1,94 @@
+<?php
+
+namespace Drupal\simpletest\Cache\Context;
+
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\Context\CacheContextInterface;
+use Drupal\Core\PrivateKey;
+use Drupal\Core\Site\Settings;
+use Drupal\simpletest\TestDiscovery;
+
+/**
+ * Defines the TestDiscoveryCacheContext service.
+ *
+ * Cache context ID: 'test_discovery'.
+ */
+class TestDiscoveryCacheContext implements CacheContextInterface {
+
+  /**
+   * The test discovery service.
+   *
+   * @var \Drupal\simpletest\TestDiscovery
+   */
+  protected $testDiscovery;
+
+  /**
+   * The private key service.
+   *
+   * @var \Drupal\Core\PrivateKey
+   */
+  protected $privateKey;
+
+  /**
+   * The hash of discovered test information.
+   *
+   * Services should not be stateful, but we only keep this information per
+   * request. That way we don't perform a file scan every time we need this
+   * hash. The test scan results are unlikely to change during the request.
+   *
+   * @var string
+   */
+  protected $hash;
+
+  /**
+   * Construct a test discovery cache context.
+   *
+   * @param \Drupal\simpletest\TestDiscovery $test_discovery
+   *   The test discovery service.
+   * @param \Drupal\Core\PrivateKey $private_key
+   *   The private key service.
+   */
+  public function __construct(TestDiscovery $test_discovery, PrivateKey $private_key) {
+    $this->testDiscovery = $test_discovery;
+    $this->privateKey = $private_key;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getLabel() {
+    return t('Test discovery');
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getContext() {
+    if (empty($this->hash)) {
+      $tests = $this->testDiscovery->getTestClasses();
+      $this->hash = $this->hash(serialize($tests));
+    }
+    return $this->hash;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheableMetadata() {
+    return new CacheableMetadata();
+  }
+
+  /**
+   * Hashes the given string.
+   *
+   * @param string $identifier
+   *   The string to be hashed.
+   *
+   * @return string
+   *   The hash.
+   */
+  protected function hash($identifier) {
+    return hash('sha256', $this->privateKey->get() . Settings::getHashSalt() . $identifier);
+  }
+
+}

core/modules/simpletest/src/Form/SimpletestTestForm.php

@@ -110,6 +110,10 @@ class SimpletestTestForm extends FormBase {
     ];
 
     $form['tests'] = [
+      '#cache' => [
+        'keys' => ['simpletest_ui_table'],
+        'contexts' => ['test_discovery'],
+      ],
       '#type' => 'table',
       '#id' => 'simpletest-form-table',
       '#tableselect' => TRUE,

core/modules/simpletest/src/TestDiscovery.php

@@ -6,7 +6,6 @@ use Doctrine\Common\Annotations\SimpleAnnotationReader;
 use Doctrine\Common\Reflection\StaticReflectionParser;
 use Drupal\Component\Annotation\Reflection\MockFileFinder;
 use Drupal\Component\Utility\NestedArray;
-use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Extension\ExtensionDiscovery;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\simpletest\Exception\MissingGroupException;
@@ -25,11 +24,11 @@ class TestDiscovery {
   protected $classLoader;
 
   /**
-   * Backend for caching discovery results.
+   * Statically cached list of test classes.
    *
-   * @var \Drupal\Core\Cache\CacheBackendInterface
+   * @var array
    */
-  protected $cacheBackend;
+  protected $testClasses;
 
   /**
    * Cached map of all test namespaces to respective directories.
@@ -70,14 +69,11 @@ class TestDiscovery {
    *   \Symfony\Component\ClassLoader\ApcClassLoader.
    * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
    *   The module handler.
-   * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend
-   *   (optional) Backend for caching discovery results.
    */
-  public function __construct($root, $class_loader, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache_backend = NULL) {
+  public function __construct($root, $class_loader, ModuleHandlerInterface $module_handler) {
     $this->root = $root;
     $this->classLoader = $class_loader;
     $this->moduleHandler = $module_handler;
-    $this->cacheBackend = $cache_backend;
   }
 
   /**
@@ -159,9 +155,9 @@ class TestDiscovery {
     $reader = new SimpleAnnotationReader();
     $reader->addNamespace('Drupal\\simpletest\\Annotation');
 
-    if (!isset($extension)) {
-      if ($this->cacheBackend && $cache = $this->cacheBackend->get('simpletest:discovery:classes')) {
-        return $cache->data;
+    if (!isset($extension) && empty($types)) {
+      if (!empty($this->testClasses)) {
+        return $this->testClasses;
       }
     }
     $list = [];
@@ -215,10 +211,8 @@ class TestDiscovery {
     // Allow modules extending core tests to disable originals.
     $this->moduleHandler->alter('simpletest', $list);
 
-    if (!isset($extension)) {
-      if ($this->cacheBackend) {
-        $this->cacheBackend->set('simpletest:discovery:classes', $list);
-      }
+    if (!isset($extension) && empty($types)) {
+      $this->testClasses = $list;
     }
 
     if ($types) {

core/modules/simpletest/tests/src/Kernel/Cache/Context/TestDiscoveryCacheContextTest.php

@@ -0,0 +1,53 @@
+<?php
+
+namespace Drupal\Tests\simpletest\Kernel\Cache\Context;
+
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\simpletest\Cache\Context\TestDiscoveryCacheContext;
+use Drupal\simpletest\TestDiscovery;
+
+/**
+ * @group simpletest
+ */
+class TestDiscoveryCacheContextTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['simpletest'];
+
+  /**
+   * Tests that test context hashes are unique.
+   */
+  public function testContext() {
+    // Mock test discovery.
+    $discovery = $this->getMockBuilder(TestDiscovery::class)
+      ->setMethods(['getTestClasses'])
+      ->disableOriginalConstructor()
+      ->getMock();
+    // Set getTestClasses() to return different results on subsequent calls.
+    // This emulates changed tests in the filesystem.
+    $discovery->expects($this->any())
+      ->method('getTestClasses')
+      ->willReturnOnConsecutiveCalls(
+        ['group1' => ['Test']],
+        ['group2' => ['Test2']]
+      );
+
+    // Make our cache context object.
+    $cache_context = new TestDiscoveryCacheContext($discovery, $this->container->get('private_key'));
+
+    // Generate a context hash.
+    $context_hash = $cache_context->getContext();
+
+    // Since the context stores the hash, we have to reset it.
+    $hash_ref = new \ReflectionProperty($cache_context, 'hash');
+    $hash_ref->setAccessible(TRUE);
+    $hash_ref->setValue($cache_context, NULL);
+
+    // And then assert that we did not generate the same hash for different
+    // content.
+    $this->assertNotSame($context_hash, $cache_context->getContext());
+  }
+
+}