Problem/Motivation

There are a couple of methods which are totally related with setting up up a functional test setup, but they are still living inside BrowserTestBase.
As seen in #2869825: Leverage JS for JS testing (using nightwatch), we need the following methods inside the trait for it to be super useful ...

  • prepareEnvironment
  • setupBaseUrl
  • installParameters

Proposed resolution

Move the methods over.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#17 interdiff.txt424 bytespritishkumar
#17 2869839-17.patch18.39 KBpritishkumar
#15 interdiff-2869839.txt1.58 KBdawehner
#15 2869839-15.patch18.41 KBdawehner
#13 interdiff-2869839.txt4.54 KBdawehner
#13 2869839-13.patch18.1 KBdawehner
#6 interdiff-2869839.txt2.76 KBdawehner
#6 2869839-6.patch19.48 KBdawehner
#4 interdiff-2869839.txt5.58 KBdawehner
#4 2869839-4.patch17.92 KBdawehner
#2 2869839-2.patch14.36 KBdawehner

Comments

dawehner created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 2869839-2.patch14.36 KB

Here is a patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs review » Needs work
Issue tags: +Novice

Nice. Only few nits:

  1. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -448,4 +450,176 @@ protected function rebuildAll() {
+  /**
...
+   * @see install_state_defaults()
+   */
+  protected function installParameters() {

    I know that the code was moved around but this is "new code" for the trait, so let's fix the docs: lacks @return. 

  2. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -448,4 +450,176 @@ protected function rebuildAll() {
+   * @throws \Exception

    Needs a comment for @throws. 

  3. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -448,4 +450,176 @@ protected function rebuildAll() {
+    $_SERVER['HTTP_USER_AGENT'] = 'Drupal command line';
+  }
+  ¶
+  /**
...
+    $callbacks = [];
+  }
+  ¶
 }

    Trailing spaces.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2869839-4.patch17.92 KB
new interdiff-2869839.txt5.58 KB

Thank you @claudiu.cristea for the super quick review!

Here is some adaption.

Status: Needs review » Needs work

The last submitted patch, 4: 2869839-4.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2869839-6.patch19.48 KB
new interdiff-2869839.txt2.76 KB

Let's fix this failure.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice

Perfect!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Nice cleanup! +1 RTBC

dawehner’s picture

dawehner
Primary language German
commented

Thanks all of you!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -448,4 +454,211 @@ protected function rebuildAll() {
+  protected function getDatabaseTypes() {
+    // Setup a fake container.
+    $container = new Container();
+    $container->set('file_system', new FileSystem(new StreamWrapperManager(), Settings::getInstance(), new LoggerChannel('test')));
+    \Drupal::setContainer($container);
+    $database_types = drupal_get_database_types();
+    \Drupal::unsetContainer();
+    return $database_types;
+  }

    Let's inline this into installParameters it's super-specialised. 

  2. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -448,4 +454,211 @@ protected function rebuildAll() {
+  protected function setupBaseUrl() {
...
+  protected function prepareEnvironment() {

    I don't think we should be adding these to WebTestBase via the Trait. Maybe we need a trait that extends the existing trait and adds them - that'd make this functionality easy to share - or just a whole new trait along side this one.

dawehner’s picture

dawehner
Primary language German
commented

Let's inline this into installParameters it's super-specialised.

Wouldn't this break potential BC?

I don't think we should be adding these to WebTestBase via the Trait.

Can you explain why?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Re #10.1 Well the problem with getDatabaseTypes is I don't think it should be API in the trait and thereby fore BrowserTestBase tests.
#10.2 Because at this point I don't think we should be adding additional API to WebTestBase and also given the number of tests in the wild WTB additions are always kinda risky.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2869839-13.patch18.1 KB
new interdiff-2869839.txt4.54 KB

#10.1
It turns out, this stuff was actually just not needed for WTB at this moment, so we don't have to add it.
#10.2
Sure. setupBaseUrl() is a new method for BTB as well, so we can mark it as private. This is not something you need to adapt.
prepareEnvironment() can make sense to override, but you know, we can trick it around by making it private in the trait but protected in BTB.

Status: Needs review » Needs work

The last submitted patch, 13: 2869839-13.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2869839-15.patch18.41 KB
new interdiff-2869839.txt1.58 KB 
#10.2
Sure. setupBaseUrl() is a new method for BTB as well, so we can mark it as private. This is not something you need to adapt.
prepareEnvironment() can make sense to override, but you know, we can trick it around by making it private in the trait but protected in BTB.

Well I fear this simply didn't worked at all, so we maybe have to go to underscore methods.

Status: Needs review » Needs work

The last submitted patch, 15: 2869839-15.patch, failed testing.

pritishkumar’s picture

pritishkumar commented
Status: Needs work » Needs review
StatusFileSize
new 2869839-17.patch18.39 KB
new interdiff.txt424 bytes

Status: Needs review » Needs work

The last submitted patch, 17: 2869839-17.patch, failed testing.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Status: Needs work » Closed (outdated)

Research show that what is in the patch here was done in #2870009: Update: Convert system functional tests to phpunit in Aug 2017. Therefor, closing as outdated.