Part of #2066993: Use magic methods to sync container property to \Drupal::getContainer in functional tests

CommentFileSizeAuthor
#14 2066999-13.patch117.56 KBxen
#13 2066999-13.patch117.56 KBxen
#11 2066999-10.patch117.56 KBxen
#11 2066999-10.txt104.57 KBxen
#5 2066999-5.patch111.48 KBbloniasz75
#4 2066999.patch111.39 KBjuanolalla

Comments

nonprofit’s picture

nonprofit commented
Assigned: Unassigned » nonprofit
nonprofit’s picture

nonprofit commented
Assigned: nonprofit » Unassigned
juanolalla’s picture

juanolalla commented
Assigned: Unassigned » juanolalla

I'm doing it.

juanolalla’s picture

juanolalla commented
Assigned: juanolalla » Unassigned
Status: Active » Needs review
StatusFileSize
new 2066999.patch111.39 KB

I've done these replacements:

\Drupal::service( with $this->container->get(
\Drupal::config( with $this->container->get('config.factory')->get(
\Drupal::state() with $this->container->get('state')
\Drupal::moduleHandler() with $this->container->get('module_handler')
\Drupal::urlGenerator() with $this->container->get('url_generator')
\Drupal::entityQuery( with $this->container->get('entity.query')->get(
\Drupal::typedData() with $this->container->get('typed_data')
\Drupal::TypeData() with $this->container->get('typed_data')
\Drupal::token() with $this->container->get('token')

bloniasz75’s picture

bloniasz75 commented
StatusFileSize
new 2066999-5.patch111.48 KB

Added one more

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Patch applies cleanly, and fixes all

There's 2 more left but they are in valid ones

git grep -C 2 "\Drupal::" core/modules/system/lib/Drupal/system/Tests/
core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php-15- * Tests for URL generation functions.
core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php-16- *
core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php:17: * url() calls Drupal::moduleHandler()->getImplementations(),
core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php-18- * which may issue a db query, which requires
core/modules/system/lib/Drupal/system/Tests/Common/UrlTest.php-19- * inheriting from a web test case rather than a unit test case.
--
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php-82-
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php-83-  /**
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php:84:   * Test Drupal::moduleHandler()->getImplementations() caching.
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php-85-   */
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php-86-  function testModuleImplements() {
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

git ac https://drupal.org/files/2066999-5.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  111k  100  111k    0     0  61644      0  0:00:01  0:00:01 --:--:-- 68355
error: patch failed: core/modules/system/lib/Drupal/system/Tests/Upgrade/FieldUpgradePathTest.php:122
error: core/modules/system/lib/Drupal/system/Tests/Upgrade/FieldUpgradePathTest.php: patch does not apply

Also I'm not sure about that this is really necessary.

RunePhilosof’s picture

RunePhilosof commented
Assigned: Unassigned » RunePhilosof

Working on reroll

xen’s picture

xen commented
Assigned: RunePhilosof » xen

Taking over from Rune.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

#2087751: [policy, no patch] Stop using $this->container in web tests

You might want to wait with this until that issue is discussed.

xen’s picture

xen commented
Assigned: xen » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2066999-10.txt104.57 KB
new 2066999-10.patch117.56 KB

Rerolled.

Found about 30 new occurrences, fixed those too.

The txt file is the rerolled patch before my additions.

Status: Needs review » Needs work

The last submitted patch, 2066999-10.patch, failed testing.

xen’s picture

xen commented
StatusFileSize
new 2066999-13.patch117.56 KB

That was a bit too fast. New version.

Tests in system all pass.

xen’s picture

xen commented
Status: Needs review » Needs work
StatusFileSize
new 2066999-13.patch117.56 KB

[arg, double post]

xen’s picture

xen commented
Status: Needs work » Needs review

And the right status too...

mcrittenden’s picture

mcrittenden commented
Status: Needs work » Needs review
Issue tags: -Needs reroll

Tags

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Postponed

Again, see #2087751: [policy, no patch] Stop using $this->container in web tests, we should not continue with this until we agreed on that issue.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes
Status: Postponed » Needs work

Setting to NW because #2066993: Use magic methods to sync container property to \Drupal::getContainer in functional tests is go.

Also bumping to 8.1.x.

snehi’s picture

snehi commented
Assigned: Unassigned » snehi
snehi’s picture

snehi commented
Assigned: snehi » Unassigned

Any final decision can we start with the patch because it will need a huge effort may be some error can be faced.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Parent issue: » #2066993: Use magic methods to sync container property to \Drupal::getContainer in functional tests
alexpott’s picture

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

I think per module is the wrong scope for this issue see #2066993-21: Use magic methods to sync container property to \Drupal::getContainer in functional tests also we need to answer a question about why we should do this.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.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.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.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.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.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.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.

smustgrave’s picture

smustgrave commented
Status: Needs work » Closed (outdated)

Appears most of these have already been completed. Closing out but if you feel it's still an issue please reopen.

smustgrave’s picture

smustgrave commented
Issue tags: +Bug Smash Initiative