Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
The TestDatabase class is responsible for generating a lock ID, from which is derived the site folder name, and the database prefix. Optionally, a lock file is written (though see #2946439: TestDatabase should always lock).
However, none of this code has anything to do with the database. The database needs the prefix, but that could be passed in.
Steps to reproduce
Proposed resolution
Move the code for creating the lock to a trait such as TestSetupTrait or a new trait.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3382828
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
joachim commentedComment #4
joachim commentedWe have several instances of this sort of thing:
which only instantiates a TestDatabase for the purpose of deriving the test site path from the DB prefix.
I'm not sure where to move this code to. Any ideas?
Comment #5
mglamanWithout opening the code, my gut reaction is alongside the test user agent functions.
Comment #6
mondrakeMy 2 cents: when I worked a bit around this code in #3075608: Introduce TestRun objects and refactor TestDatabase to be testable, I remember thinking that I would rather try to get rid of TestSetupTrait, and move as much as possible of the code (prefixing, locking, etc) to TestDatabase or directly to the base test classes instead.
Comment #7
joachim commented@mondrake I don't think code for site ID and locking belongs in TestDatabase because it doesn't have anything to do with the database. The database needs to be given the prefix, but you don't the database to get the ID. And the fact that there is code that instantiates a TestDatabase *just* to get the site ID seems wrong.
@mglaman I moved the methods from TestDatabase into a trait so they can be used directly by test base classes without going to TestDatabase (which in some cases is pointlessly instantiated).
But now I'm not sure what to do in a case like this in function code.
Are you suggesting that instead of a trait, we move the code to functions in bootstrap.inc, alongside drupal_valid_test_ua() etc?
My feeling is that things like the lock ID, the site path, and DB prefix should belong to something like a specialised subclass of DrupalKernel, but I fear that's going to be another rabbithole.
Comment #8
smustgrave commentedCC failure in MR.
Did not review.
Comment #9
joachim commentedSpoke about this to Alex Pott at DrupalCon Lille, and his analysis is that the class should stay as it is, but that it's badly named.