settings.testing.php / testing.services.yml not picked up by BrowserTestBase [#2849222]

Problem/Motivation

This is a regression in 8.3.x introduced in #2796105: Move similar methods in BrowserTestBase / WebTestBase to a trait; untangle installDrupal().

settings.testing.php and testing.services.yml are not picked up by test classes extending BrowserTestBase.

The reason is that BrowserTestBase sets $this->originalSiteDirectory while FunctionalSetupTrait expects $this->originalSite to be set.

Proposed resolution

Fix BrowserTestBAse to set $this->originalSite instead.

Note that - because this affects the setup code of tests and in particular the interaction with the host system - this cannot be tested automatically.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Comment	File	Size	Author
#4	2849222-4.patch	906 bytes	tstoeckler
#4	8.3.x: PHP 5.6 & MySQL 5.5 21,524 pass
#2	2796105-followup (1).patch	906 bytes	tstoeckler
#2	8.3.x: PHP 5.6 & MySQL 5.5 21,510 pass, 1 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

2 February 2017 at 17:09

tstoeckler created an issue. See original summary.

Comment #2

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented 2 February 2017 at 17:11

Status:	Active	» Reviewed & tested by the community
Parent issue:		» #2796105: Move similar methods in BrowserTestBase / WebTestBase to a trait; untangle installDrupal()

File	Size
2796105-followup (1).patch	906 bytes
8.3.x: PHP 5.6 & MySQL 5.5 21,510 pass, 1 fail

Uploading patch from #2796105-63: Move similar methods in BrowserTestBase / WebTestBase to a trait; untangle installDrupal() (#63) and marking RTBC per #2796105-67: Move similar methods in BrowserTestBase / WebTestBase to a trait; untangle installDrupal() (#67).

Comment #3

2 February 2017 at 18:02

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch, 2: 2796105-followup (1).patch, failed testing.

Comment #4

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented 2 February 2017 at 21:31

Status:

Needs work

» Needs review

File	Size
2849222-4.patch	906 bytes
8.3.x: PHP 5.6 & MySQL 5.5 21,524 pass

1 file was hidden/shown/deleted

File	Size
2796105-followup (1).patch	906 bytes
8.3.x: PHP 5.6 & MySQL 5.5 21,510 pass, 1 fail

Hmm... not sure what happened there. Here is a fresh one.

Comment #5

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented 3 February 2017 at 05:41

Status:

Needs review

» Reviewed & tested by the community

Comment #6

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 8 February 2017 at 16:22

Status:

Reviewed & tested by the community

» Needs review

No RTBCing your own patch, please. Thanks!

Comment #7

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 8 February 2017 at 16:25

Status:

Needs review

» Reviewed & tested by the community

I reviewed it and will RTBC - it's just correcting a variable name that wasn't adjusted with the conversion to the trait.

Comment #8

8 February 2017 at 16:33

xjm committed f2f688b on 8.4.x

Issue #2849222 by tstoeckler: settings.testing.php / testing.services....

Comment #9

8 February 2017 at 16:33

xjm committed 3f1912d on 8.3.x

Issue #2849222 by tstoeckler: settings.testing.php / testing.services....

At first I didn't see where $originalSite was documented, but as @catch pointed out it's in TestSetupTrait (not FunctionalTestSetupTrait). https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Test%21Te...

Checking the usages of both properties in HEAD:

[ibnsina:drupal | Wed 10:19:27] $ grep -r "originalSite" *
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:      'value' => $this->originalSite,
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:    $settings_testing_file = DRUPAL_ROOT . '/' . $this->originalSite . '/settings.testing.php';
core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:    $settings_services_file = DRUPAL_ROOT . '/' . $this->originalSite . '/testing.services.yml';
core/lib/Drupal/Core/Test/TestSetupTrait.php:  protected $originalSite;
core/modules/simpletest/src/KernelTestBase.php:    $settings_services_file = DRUPAL_ROOT . '/' . $this->originalSite . '/testing.services.yml';
core/modules/simpletest/src/KernelTestBase.php:    $settings_testing_file = DRUPAL_ROOT . '/' . $this->originalSite . '/settings.testing.php';
core/modules/simpletest/src/KernelTestBase.php:    $settings['test_parent_site'] = $this->originalSite;
core/modules/simpletest/src/TestBase.php:    $this->originalSite = $site_path;
core/tests/Drupal/Tests/BrowserTestBase.php:  protected $originalSiteDirectory;
core/tests/Drupal/Tests/BrowserTestBase.php:    $this->originalSiteDirectory = $kernel->findSitePath($request);

With the patch, there are no remaining instances of originalSiteDirectory.

As @catch pointed out this is an 8.3.x regression and not testable. It also actually feels like a major bug to me. So I've committed it to 8.4.x and backported it to 8.3.x as well. Thanks!