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

Command icon 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:

Comments

sime created an issue. See original summary.

sime’s picture

Issue summary: View changes

sime’s picture

Title: Test setup should copy default.settings.php from assets » Test setup should copy default.settings.php/services.yml from assets
Issue summary: View changes
sime’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Little outside my wheelhouse but the change does make sense from the standpoint of tests s. And leaning on it didn't break anything

sime’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

I 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.

sime’s picture

Assigned: Unassigned » sime
sime’s picture

Status: Needs work » Reviewed & tested by the community

I've created a change record and put it in draft.
https://www.drupal.org/node/3446122

  • catch committed d1eede1c on 10.4.x
    Issue #3440837 by sime: Test setup should copy default.settings.php/...

  • catch committed f862f883 on 11.x
    Issue #3440837 by sime: Test setup should copy default.settings.php/...
catch’s picture

Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

Started 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

Issue tags: -Needs change record

The CR exists and is published.