Refactor InstallerNonDefaultDatabaseDriverTest to avoid asserting content of settings.php

Created on 14 June 2023, almost 2 years ago
Updated 20 February 2024, about 1 year ago

Problem/Motivation

Noted in Introduce database driver extensions and autoload database drivers' dependencies Fixed , https://git.drupalcode.org/project/drupal/-/merge_requests/3169#note_182014.

The InstallerNonDefaultDatabaseDriverTest asserts on actual strings of the settings.php file, which is brittle.

Steps to reproduce

Proposed resolution

Refactor to assert on arrays instead (from connection info?)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component
Database 

Last updated 3 days ago

  • Maintained by
  • 🇳🇱Netherlands @daffie
Created by

🇮🇹Italy mondrake 🇮🇹

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mondrake
  • First commit to issue fork.
  • 🇮🇹Italy mondrake 🇮🇹
  • 🇮🇹Italy mondrake 🇮🇹
  • Pipeline finished with Failed
    6 days ago
    Total: 195s
    #472419
  • Pipeline finished with Failed
    5 days ago
    Total: 525s
    #472926
  • 🇮🇹Italy mondrake 🇮🇹

    From https://www.php.net/manual/en/function.include.php

    When a file is included, the code it contains inherits the variable scope of the line on which the include occurs. Any variables available at that line in the calling file will be available within the called file, from that point forward. However, all functions and classes defined in the included file have the global scope.

    So in this case, $databases is a local variable of InstallerNonDefaultDatabaseDriverTest::getInstalledDatabaseSettings() coming from the evaluation of the code in the included settings.php file.

  • 🇳🇱Netherlands daffie

    When a file is included, the code it contains inherits the variable scope of the line on which the include occurs. Any variables available at that line in the calling file will be available within the called file, from that point forward. However, all functions and classes defined in the included file have the global scope.

    Now I get it. Thanks for the explanation. Could you remove the assert(isset($databases)); from the PR and add a comment with your explanation? After that it is RTBC for me.

  • 🇮🇹Italy mondrake 🇮🇹

    I had to add the assert since PHPStan was complaining more or less for the same reason of your comment - PHPStan cannot determine what's included in the include (pun intended), so we need to inform it that a variable with that name is existing at that point.

    Once the parent is committed I will rebase and add a comment.

  • Pipeline finished with Canceled
    3 days ago
    Total: 118s
    #474620
  • 🇮🇹Italy mondrake 🇮🇹

    Done.

  • 🇳🇱Netherlands daffie

    All code change look good to me.
    The test has been improved.
    For me it is RTBC.

  • Pipeline finished with Success
    3 days ago
    Total: 1753s
    #474621
Production build 0.71.5 2024