Problem/Motivation

In \Drupal\Core\Test\FunctionalTestSetupTrait::installDefaultThemeFromClassProperty() we determine the default theme for an install profile by reading the it's config/install/system.theme.yml. However profiles can also be installed from config/sync so we should check there too. This allows tests using such an install profile not to have to declare a $defaultTheme property.

Proposed resolution

Check config/sync directory too.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3108021-2.patch3.7 KBalexpott
#2 3108021-2.test-only.patch2.48 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3108021-2.test-only.patch2.48 KB
3108021-2.patch3.7 KB

The last submitted patch, 2: 3108021-2.test-only.patch, failed testing. View results

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

Could we remove the $defaultTheme property from Drupal\FunctionalTests\Installer\InstallerExistingConfigSyncDriectoryProfileMismatchTest and Drupal\FunctionalTests\Installer\InstallerExistingConfigSyncDirectoryProfileHookInstall as well?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

@lauriii nope - because these use config outside the profile. I debated checking the defined config sync directory so we could but then I feared that this might erroneously pick up unrelated config so I think that that is bad idea.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs review » Reviewed & tested by the community

Makes sense. In that case #2 looks good. Thank you!

  • catch committed de6caed on 9.0.x 
    Issue #3108021 by alexpott, lauriii: Set $defaultTheme for install...

  • catch committed 0febf23 on 8.9.x 
    Issue #3108021 by alexpott, lauriii: Set $defaultTheme for install...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed de6caed and pushed to 9.0.x. Thanks! Backported to 8.9.x

  • catch committed 37c653f on 8.8.x 
    Issue #3108021 by alexpott, lauriii: Set $defaultTheme for install...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.9.x-dev » 8.8.x-dev

And to 8.8.x

Status: Fixed » Closed (fixed)

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