Skip to content
Snippets Groups Projects

Improve performance of functional tests by caching Drupal installations

Open Improve performance of functional tests by caching Drupal installations
issue/drupal-2900208:2900208-cache-installs into 11.x
10 unresolved threads
Open Dave Reid requested to merge issue/drupal-2900208:2900208-cache-installs into 11.x
10 unresolved threads

Closes #2900208

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
core/tests/Drupal/Tests/BrowserTestBase.php
588 /**
589 * Determines a proper file name for SQL dump.
590 */
591 protected function initDumpFile() {
592 $class = get_class($this);
593 $modules = [];
594 while ($class) {
595 if (property_exists($class, 'modules')) {
596 $modules = array_merge($modules, $class::$modules);
597 }
598 $class = get_parent_class($class);
599 }
600 sort($modules);
601 array_unique($modules);
602 $cache_dir = getenv('BROWSERTEST_CACHE_DIR') ?: sys_get_temp_dir() . '/test_dumps/' . \Drupal::VERSION;
603 is_dir($cache_dir) || mkdir($cache_dir, 0777, TRUE);
  • Guy Elsmore-Paddock
    Guy Elsmore-Paddock @guypaddock ·

    Recently, we've been experiencing sporadic failures related to this line. The error is:

    mkdir(): File exists

    I suspect that if two tests happen to get to this line at the same time (with concurrency enabled), there's a race condition in which the is_dir() check returns FALSE and one of the tests creates the folder while the other fails to create it because it already exists.

  • A
    Alex Pott @alexpott ·
    Maintainer

    Yeah making directories is a known race in PHP. In order for this to be safe you need to do something like:

        // Create the build/artifacts directory if necessary.
    if (!is_dir($cache_dir) && !@mkdir($cache_dir, 0777, TRUE) && !is_dir($cache_dir)) {
      throw new \RuntimeException('Unable to create directory: ' . $cache_dir);
    }
  • Please register or sign in to reply
  • Andrei Ivnitskii added 1522 commits

    added 1522 commits

    Compare with previous version

  • Andrei Ivnitskii added 1 commit

    added 1 commit

    • 98c7152a - Fix "Access to an undefined property"

    Compare with previous version

  • Andrei Ivnitskii added 1 commit

    added 1 commit

    • f4118590 - Revert "Fix "Access to an undefined property""

    Compare with previous version

  • Andrei Ivnitskii added 1 commit

    added 1 commit

    Compare with previous version

  • Jonathan Smith added 107 commits

    added 107 commits

    • ccdf544b...8705b9ab - 101 commits from branch project:11.x
    • 88476053 - Move from patch to MR.
    • eec84fc1 - Support namespaced mysql driver.
    • a6c5ccc5 - Improve exception message to include the database driver value.
    • a51950af - Fix "Access to an undefined property"
    • 393b051e - Revert "Fix "Access to an undefined property""
    • a5294fc8 - Added suggestion from #71

    Compare with previous version

    Toggle commit list
  • Jonathan Smith
    Jonathan Smith @jonathan1055 ·

    110 commits behind, so I applied a /rebase comment.

  • Jonathan Smith added 1 commit

    added 1 commit

    • c1371305 - Move definition of dumpFile from BrowserTestBase to FunctionalTestSetupTrait

    Compare with previous version

  • Jonathan Smith added 1 commit

    added 1 commit

    • 56db4354 - Add BROWSERTEST_CACHE_DB to phpunit.xml.dist

    Compare with previous version

  • Alex Pott @alexpott started a thread on the diff
    • core/tests/Drupal/Tests/BrowserTestBase.php
    564 $modules = [];
    565 while ($class) {
    566 if (property_exists($class, 'modules')) {
    567 $modules = array_merge($modules, $class::$modules);
    568 }
    569 $class = get_parent_class($class);
    570 }
    571 if (!empty($this->defaultTheme)) {
    572 $modules[] = $this->defaultTheme;
    573 }
    574 $modules[] = $this->profile;
    575 sort($modules);
    576 array_unique($modules);
    577 $cache_dir = getenv('BROWSERTEST_CACHE_DIR') ?: sys_get_temp_dir() . '/test_dumps/' . \Drupal::VERSION;
    578 is_dir($cache_dir) || mkdir($cache_dir, 0777, TRUE);
    579 $this->dumpFile = $cache_dir . '/_' . md5(implode('-', $modules)) . '.sql';
    • A
      Alex Pott @alexpott ·
      Maintainer

      Given md5 causes some security scanners to have kittens and this is not performance critical code lets use Crypt::hashBase64() or just hash('sha256'... if you like.

    • A
      Alex Pott @alexpott ·
      Maintainer

      Alternatively if you want to use a non cryptographic hash... then we should borrow from other code... eg. crc32(implode(':', array_keys($this->container->getParameter('container.modules'))));

      Suggested change
      579 $this->dumpFile = $cache_dir . '/_' . md5(implode('-', $modules)) . '.sql';
      579 $this->dumpFile = $cache_dir . '/_' . crc32(implode(':', $modules)) . '.sql';
    • C
      catch @catch ·
      Maintainer

      We should use xxHash

    • Please register or sign in to reply
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    69 69 */
    70 70 protected bool $usesSuperUserAccessPolicy;
    71 71
    72 /**
    73 * Path to SQL dump file.
    74 *
    75 * @var string
    76 */
    77 protected $dumpFile;
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    742 749 return $database_types;
    743 750 }
    744 751
    752 /**
    753 * Installs Drupal using SQL dump.
    754 */
    755 protected function installDrupalFromDump() {
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    807 $this->container = $this->kernel->getContainer();
    808
    809 // The value comes with the dump from previous installation.
    810 \Drupal::configFactory()->getEditable('system.file')
    811 ->set('path.temporary', $this->tempFilesDirectory)
    812 ->save();
    813 \Drupal::service('file_system')->prepareDirectory($this->tempFilesDirectory, FileSystemInterface::MODIFY_PERMISSIONS | FileSystemInterface::CREATE_DIRECTORY);
    814
    815 // Manually create the private directory.
    816 \Drupal::service('file_system')->prepareDirectory($this->privateFilesDirectory, FileSystemInterface::CREATE_DIRECTORY);
    817 }
    818
    819 /**
    820 * Dumps database structure and contents of test site.
    821 */
    822 protected function dumpDatabase() {
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    826 $pass = $connection_info['default']['password'];
    827 $db = $connection_info['default']['database'];
    828 $host = $connection_info['default']['host'];
    829
    830 switch ($connection_info['default']['driver']) {
    831 case 'mysql':
    832 case 'Drupal\mysql\Driver\Database\mysql':
    833 $tables = \Drupal::database()
    834 ->query("SHOW TABLES LIKE '$this->databasePrefix%'")
    835 ->fetchCol();
    836 $tables_param = implode(' ', $tables);
    837 exec("mysqldump -u$user -p$pass -h $host $db $tables_param | sed 's/$this->databasePrefix/default_db_prefix_/' > {$this->dumpFile}");
    838 break;
    839
    840 default:
    841 throw new \LogicException("The database driver {$connection_info['default']['driver']} is not supported yet.");
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    833 $tables = \Drupal::database()
    834 ->query("SHOW TABLES LIKE '$this->databasePrefix%'")
    835 ->fetchCol();
    836 $tables_param = implode(' ', $tables);
    837 exec("mysqldump -u$user -p$pass -h $host $db $tables_param | sed 's/$this->databasePrefix/default_db_prefix_/' > {$this->dumpFile}");
    838 break;
    839
    840 default:
    841 throw new \LogicException("The database driver {$connection_info['default']['driver']} is not supported yet.");
    842 }
    843 }
    844
    845 /**
    846 * Restores database structure and contents of test site.
    847 */
    848 protected function restoreDatabase() {
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
    848 protected function restoreDatabase() {
    849 $connection_info = Database::getConnectionInfo('default');
    850
    851 $user = $connection_info['default']['username'];
    852 $pass = $connection_info['default']['password'];
    853 $db = $connection_info['default']['database'];
    854 $host = $connection_info['default']['host'];
    855
    856 switch ($connection_info['default']['driver']) {
    857 case 'mysql':
    858 case 'Drupal\mysql\Driver\Database\mysql':
    859 exec("sed 's/default_db_prefix_/$this->databasePrefix/' $this->dumpFile | mysql -u$user -p$pass -h $host $db");
    860 break;
    861
    862 default:
    863 throw new \LogicException("The database driver {$connection_info['default']['driver']} is not supported yet.");
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/tests/Drupal/Tests/BrowserTestBase.php
    547 $this->installDrupalFromDump();
    548 }
    549 else {
    550 $this->installDrupalFromProfile();
    551 $this->dumpDatabase();
    552 }
    553 }
    554 else {
    555 $this->installDrupalFromProfile();
    556 }
    557 }
    558
    559 /**
    560 * Determines a proper file name for SQL dump.
    561 */
    562 protected function initDumpFile() {
  • Stephen Mustgrave
    Stephen Mustgrave @smustgrave started a thread on the diff
    • core/tests/Drupal/Tests/BrowserTestBase.php
    570 }
    571 if (!empty($this->defaultTheme)) {
    572 $modules[] = $this->defaultTheme;
    573 }
    574 $modules[] = $this->profile;
    575 sort($modules);
    576 array_unique($modules);
    577 $cache_dir = getenv('BROWSERTEST_CACHE_DIR') ?: sys_get_temp_dir() . '/test_dumps/' . \Drupal::VERSION;
    578 is_dir($cache_dir) || mkdir($cache_dir, 0777, TRUE);
    579 $this->dumpFile = $cache_dir . '/_' . md5(implode('-', $modules)) . '.sql';
    580 }
    581
    582 /**
    583 * Installs Drupal using installation profile.
    584 */
    585 protected function installDrupalFromProfile() {
    Please register or sign in to reply