Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In #2868273: Missing a test for table TRUNCATE while in transaction it was discussed to introduce a $connection property to the DatabaseTestBase
base class so that extending classes can use it in the test code without having to implement it in each class.
Proposed resolution
Move the logic from the DeleteTruncateTest
to the DatabaseTestBase
class.
Remaining tasks
Do it.
User interface changes
none
API changes
none
Data model changes
none
Comment | File | Size | Author |
---|---|---|---|
#2 | 2953385-2.patch | 3.8 KB | mondrake |
Comments
Comment #2
mondrakePatch.
Comment #3
Berdirconfused why this isn't using $this->connection now ;)
Comment #4
mondrake#3
addSampleData()
is a static ... so unless we change it to non-static,$this
will not be in scope :)Did not want to change it, but apparently this is not used anywhere else. It could also be a protected, actually.
Comment #5
mondrake#3, #4,
actually no:
it's more complicated, we cannot change easily to non-static.
Comment #6
volegerit is possible to use
$this->container->get('database');
here?
Comment #7
mondrake#6: the database connection code in the
KernelTestBase
class thatDatabaseTestBase
extends from uses direct calls to theDatabase
methods rather than going through the service container. That's the reason I was going for it too.Comment #8
voleger#7 ok
So maybe it would be better to store connection instance in a single static property?
Comment #9
mondrake@voleger re #8 why do you think it would be better? I cannot say the full consequences of that, but
a) the
::setUp
method will anyway reset it for every test, which I am afraid may cause issues in running multiple tests in parallelb) it will not solve #3 / #4 because
Drupal\system\Tests\Database\DatabaseWebTestBase
andDrupal\Tests\system\Functional\Database\DatabaseTestBase
are not extending fromDrupal\KernelTests\Core\Database\DatabaseTestBase
, so the property will not be initialisedComment #10
volegerfor now, it's ok for me.
+1 for RTBC
I'm waiting this issue to unblock #2939387: Replace all calls to db_query [core/test/../Database]
Comment #11
joshmillerComment #12
alexpottCommitted and pushed 872ddb8101 to 8.6.x and bdfce71e7c to 8.5.x. Thanks!
Backported to 8.5.x as this is a test only change.