Updated: Comment #0

Problem/Motivation

drupal_get_profile() returns 'standard' in DrupalUnitTestBase tests. This can lead to the Standard profile config being installed by DrupalUnitTestBase::installConfig(). As an example, $this->installConfig(array('filter')); will install the 'Full HTML' format from Standard profile. This breaks encapsulation of tests.

The reason for this is that drupal_get_profile() falls back to 'standard' if Settings::get('install_profile') is empty. Inside a DrupalUnitTestBase (almost) no settings are set at all, and 'install_profile' is not either.

Proposed resolution

Remove the fallback to 'standard' in drupal_get_profile().

Remaining tasks

User interface changes

None.

API changes

drupal_get_profile() no longer performs a (non-sensical) fallback to 'standard'.

CommentFileSizeAuthor
#5 2234771-4-drupal-get-profile-no-fallback.patch1.91 KBtstoeckler
#5 2234771-1-4-interdiff.txt1.82 KBtstoeckler
#1 drupal-get-profile-no-fallback.patch372 byteststoeckler
#1 dutb-standard-profile.patch807 byteststoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Needs review
FileSize
dutb-standard-profile.patch807 bytes
drupal-get-profile-no-fallback.patch372 bytes

Here's a patch for either of the two approaches.

I personally think B makes a lot of sense, but I'm not sure how badly it will break stuff.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Title: drupal_get_profile() returns 'standard' in DrupalUnitTestBase tests » drupal_get_profile() should not fall back to 'standard'
Component: simpletest.module » base system
Issue summary: View changes

All right, let's do B, then.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Yup, the fix for drupal_get_profile() makes more sense to me.

All other code throughout core was recently adjusted to no longer assume that there is always an installation profile, because the early installer screens do not have one (until a profile is selected).

I think this is RTBC, but can we quickly add a test method to the DrupalUnitTestBaseTest (of Simpletest) with basically just one assertion that drupal_get_profile() returns no installation profile?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Looks like the phpDoc of drupal_get_profile() needs an adjustment, too.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
FileSize
2234771-1-4-interdiff.txt1.82 KB
2234771-4-drupal-get-profile-no-fallback.patch1.91 KB

Thanks for the review. Makes a lot of sense, here we go.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Looks good, but needs a change notice. According to http://drupalcontrib.org/api/drupal/drupal!includes!common.inc/function/... this is actually called from places outside of core.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs work » Needs review
Issue tags: -Needs change record

Added a draft change record: https://drupal.org/node/2235431

There's no example code as there really is no use-case of contrib code that will have to change for this. At least I couldn't think of one.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Just added some example code with the help of @tim.plunkett in IRC.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Awesome, thanks.

Committed and pushed to 8.x.

  • Commit 28f177c on 8.x by webchick: 
    Issue #2234771 by tstoeckler: Drupal_get_profile() should not fall back...

Status: Fixed » Closed (fixed)

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