Problem/Motivation
FunctionalTestSetupTrait.php should copy default.settings.php and services.yml from the assets folder, since copying it from sites/default/ is more volatile, being dependent on the composer scaffold configuration. My observations is that people will develop phpunit tests within an existing site's codebase, so if the default.settings.php file is not in place unnecessary DX hurdle and stifles learning by example.
My logic is that, for sites/default/default.settings.php to be in place the scaffold MUST have run. If the scaffold has run then we assume the assets folder is intact. Therefore, simply copying the file from the assets folder is the same process but removes one step that is vulnerable to snowflake setups.
Steps to reproduce
In composer.json scaffold settings:
"extra": {
"drupal-scaffold": {
"locations": {
"web-root": "web/"
},
"file-mapping": {
"[web-root]/sites/default/default.settings.php": false,
"[web-root]/sites/default/default.services.yml": false
}
},
Proposed resolution
Change the line to copy the file from the asset folder. Every bootstrapped test in Drupal relies on this so an additional test would be redundant.
Remaining tasks
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
Issue fork drupal-3440837
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:
- 3440837-test-setup-should
changes, plain diff MR !7481
Comments
Comment #2
simeComment #4
simeComment #5
simeComment #6
smustgrave commentedLittle outside my wheelhouse but the change does make sense from the standpoint of tests s. And leaning on it didn't break anything
Comment #7
simeComment #8
alexpottI guess someone could be relying on the current behaviour. It wouldn't be right but you never know. Therefore I think we should have a change record for this.
Comment #9
simeComment #10
simeI've created a change record and put it in draft.
https://www.drupal.org/node/3446122
Comment #14
catchStarted committing this, then realised I wasn't sure which version to backport it to, then decided to go with 10.4.x and 11.x (i.e. 11.1). Updated the CR to reflect this. Committed/pushed to 11.x and cherry-picked to 10.4.x, thanks!
Comment #16
quietone commentedThe CR exists and is published.