Replace usage of assertAttributeEquals() that is deprecated

1. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -1028,6 +1036,9 @@ public function truncate($table, array $options = []) {
   +      throw new ConnectionInvalidException('Cannot access the database schema, the database connection is invalid');
   

   Throwing a new exception must be documented in the docblock

2. Can a connection that has been destroyed, be reopened again?

3. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -246,6 +253,7 @@ public static function open(array &$connection_options = []) {}
   +    $this->destroyed = TRUE;
   

   Can we remove the line of code $this->schema = NULL;. It is now not necessary any more.

4. +++ b/core/tests/Drupal/Tests/Core/Menu/StaticMenuLinkOverridesTest.php
   @@ -11,18 +11,6 @@
   -  /**
   -   * Tests the constructor.
   -   *
   -   * @covers ::__construct
   -   */
   -  public function testConstruct() {
   -    $config_factory = $this->getConfigFactoryStub(['core.menu.static_menu_link_overrides' => []]);
   -    $static_override = new StaticMenuLinkOverrides($config_factory);
   -
   -    $this->assertAttributeEquals($config_factory, 'configFactory', $static_override);
   -  }
   -
   

   Why are we removing this piece of code in this issue?

@mondrake: Thank you for your explanation.

1. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -246,6 +253,7 @@ public static function open(array &$connection_options = []) {}
   +    $this->destroyed = TRUE;
   

   Can you move this line next to the line $this->schema = NULL; and add some documentation why the schema must be set to NULL.

2. +++ b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
   @@ -198,7 +199,9 @@ public function testDestroy() {
   -    $this->assertAttributeEquals(NULL, 'schema', $connection);
   

   Can we still keep testing with reflection that the variable $schema has the value NULL.

I want to keep the testing for $this->schema = NULL, because it is important as you said: "the purpose is actually to null out the pointer to the schema object to allow garbage collection by PHP, so that's explicit".

As we are not removing $this->schema = NULL, the whole adding of the $destroyed parameter feels like it is out of scope for this issue. I like the functionality and that is why I am asking you to create a new issue for it.

@daffie you only get the exception if the value of $this->schema is null, so that's tested anyway. If we disagree, please let the issue in needs review so that someone else could comment, too.

Yes, we disagree and I shall leave the issue in needs review.

1. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -246,13 +253,16 @@ public static function open(array &$connection_options = []) {}
   +    $this->destroyed = TRUE;
   
   @@ -1026,9 +1036,15 @@ public function truncate($table, array $options = []) {
   +      if ($this->destroyed) {
   

   The addition of the destroyed property needs way more work. Imo we need to evaluate the destroy method and move this code to __destruct() so the object is always destructed correctly. We need to look at #843114: DatabaseConnection::__construct() and DatabaseConnection_mysql::__construct() leaks $this (Too many connections) and work out why this solution was chosen.

2. +++ b/core/tests/Drupal/Tests/Core/Database/ConnectionTest.php
   @@ -198,7 +199,9 @@ public function testDestroy() {
   -    $this->assertAttributeEquals(NULL, 'schema', $connection);
   +    $this->expectException(ConnectionInvalidException::class);
   +    $this->expectExceptionMessage('Cannot access the database schema, the database connection is being closed');
   +    $connection->schema();
   

   I think using reflection here is preferable. And not changing runtime code for testability reasons.

3. +++ b/core/tests/Drupal/Tests/Core/Menu/StaticMenuLinkOverridesTest.php
   @@ -11,18 +11,6 @@
   -  /**
   -   * Tests the constructor.
   -   *
   -   * @covers ::__construct
   -   */
   -  public function testConstruct() {
   -    $config_factory = $this->getConfigFactoryStub(['core.menu.static_menu_link_overrides' => []]);
   -    $static_override = new StaticMenuLinkOverrides($config_factory);
   -
   -    $this->assertAttributeEquals($config_factory, 'configFactory', $static_override);
   -  }
   

   +1 this test is not a test.

Opened #3128616: Replace \Drupal\Core\Database\Connection::destroy() with a proper destructor to rid us of destroy().

All changes look good to me.
The suppression of warnings for the use of assertAttributeEquals() is removed.
All instances of assertAttributeEquals() are removed.
For me it is RTBC.

@mondrake good idea - less boilerplate and it's all test code so getters have no value.

The only changes are in tests or test implementations, the changes are as minimal as possible and follow what we did in recent similar issues, I think this is ready to go in.

Committed and pushed a6e402fc01 to 9.1.x and 50a6075593 to 9.0.x. Thanks!