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
Running \Drupal\KernelTests\Config\DefaultConfigTest creates a directory called : in DRUPAL_ROOT. This artefact shouldn't exist.
It gets created by image_install. It happens because file_default_scheme() depends on system config BUT file.inc is part of core.
Steps to reproduce:
- Run
Drupal\KernelTests\Config\DefaultConfigTestlocally - Check for the existence of a directory called
:in the root of the checkout. It will contain another directory calledstyles
Proposed resolution
The simplest fix here is to make it return 'public' if the configuration does not exist. Once it does then the directory is created as expected in virtual file system set up by KernelTestBase.
Remaining tasks
User interface changes
None
API changes
None.
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 2861622-10.patch | 2.18 KB | alexpott |
| #10 | 5-10-interdiff.txt | 770 bytes | alexpott |
| #5 | 2861622-5.patch | 1.43 KB | alexpott |
| #5 | 2861622-5.test-only.patch | 649 bytes | alexpott |
| #4 | 2861622-4.patch | 817 bytes | alexpott |
Comments
Comment #2
alexpottMarking this major since any file operation in kernel tests might cause files to be created in places you don't expect.
Comment #3
dawehnerOOH, I always saw that and thought I became crazy / used some bad command in the command line. Can we explain in a comment why we need some fallback? (pre installation could be a different place where this function returns the wrong output).
Comment #4
alexpottHere's a better fix that is isolated to KTB only.
Comment #5
alexpottHere's a test.
Comment #6
Mile23If we're writing the test in #5 then file_default_scheme() should be deprecated in a follow-up.
Comment #10
alexpottComment #11
alexpott@Mile23 wrt to deprecation of
file_default_scheme()see #2244513: Move the unmanaged file APIs to the file_system service (file.inc) - I don't think that it is in scope here.Comment #12
dawehnerGiven that this is a problem in the testing infrastructure (kernel tests don't boot up a real site) I think fixing it just inside tests is totally cool. The referenced issue will fix other parts.
The only small thing I was wondering, do we need a change notice as this might break some contrib tests? I guess this is such an edge case, that this is not worth it?
Comment #15
catchCommitted/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!