The test container should be used instead of calling \Drupal directly.

Issue fork easy_breadcrumb-3613131

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

loopduplicate created an issue. See original summary.

loopduplicate opened merge request !178

loopduplicate’s picture

loopduplicate commented
Assigned: loopduplicate » Unassigned
Status: Active » Needs review
csakiistvan’s picture

csakiistvan
Primary language Hungarian
Location Budapest, Hungary
commented
Assigned: Unassigned » csakiistvan
csakiistvan’s picture

csakiistvan
Primary language Hungarian
Location Budapest, Hungary
commented
Assigned: csakiistvan » Unassigned
Status: Needs review » Reviewed & tested by the community

Environment

  • Drupal: 11.4.4
  • PHP: 8.5.5
  • Database: MariaDB 10.11.16
  • DDEV: v1.25.2
  • Easy Breadcrumb: 2.x-dev

Prerequisites

  • Drupal 11.4.4 site with drupal/easy_breadcrumb:2.x-dev installed via Composer.
  • PHPUnit available through the site's Composer install (vendor/bin/phpunit).
  • Functional test environment variables set when running the test:
SIMPLETEST_BASE_URL=http://web
SIMPLETEST_DB=mysql://db:db@db/db
BROWSERTEST_OUTPUT_DIRECTORY=/tmp
  • Note: this is a test-code cleanup, so it is not reproducible in the browser. The verification is the test run plus the absence of static \Drupal calls in the test.

Steps

  1. Confirm the current state of tests/src/Functional/EasyBreadcrumbInstallUninstallTest.php: it resolves services through the static \Drupal::moduleHandler() and \Drupal::service() helpers instead of the test container.
  2. Run the functional test before the change and note the result.
vendor/bin/phpunit -c web/core/phpunit.xml.dist \
  web/modules/contrib/easy_breadcrumb/tests/src/Functional/EasyBreadcrumbInstallUninstallTest.php
  1. Apply the fix from MR !178: every service lookup in testInstallUninstallReinstall() now goes through $this->container->get() (module_handler, module_installer, config.factory) instead of the static \Drupal helpers.
  2. Rebuild caches: ddev drush cr
  3. Run the same functional test again and compare the result.
  4. Run coding standards on the changed file:
vendor/bin/phpcs --standard=Drupal,DrupalPractice \
  web/modules/contrib/easy_breadcrumb/tests/src/Functional/EasyBreadcrumbInstallUninstallTest.php

Expected results

  • The test passes both before and after the change, with the same 11 assertions — behaviour is unchanged.
  • After the change the test file contains no static \Drupal:: calls; all ten service lookups use the test container.
  • phpcs reports no errors or warnings for the changed file.

Actual results

Before the fix the test file contained ten static \Drupal::moduleHandler() / \Drupal::service() calls and the test passed with 1 test and 11 assertions. After applying MR !178 the same test still passes with 1 test and 11 assertions, the static calls are gone (ten $this->container->get() lookups in their place), and phpcs with the Drupal and DrupalPractice standards is clean. The 17 deprecation notices reported by PHPUnit come from unrelated modules in the test site and appear identically before and after the change.

Testing produced with the assistance of an LLM.

loopduplicate’s picture

loopduplicate commented

rebased with Gitlab

  • loopduplicate committed 81d9a731 on 2.x 
    Issue #3613131 Uses the test container instead of calling \Drupal...
loopduplicate’s picture

loopduplicate commented
Status: Reviewed & tested by the community » Fixed

Thanks @csakiistvan for the automated report. How can the rest of the Drupal community learn about your process for using AI to review tickets? Maybe you could show us on the AI Learner's channel/podcast? Just an idea.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

loopduplicate’s picture

loopduplicate commented

Oh shoot, @csakiistvan, I didn't get your name into the commit message. I did update the contribution records so you get credit. I apologize for that. I'm a bit rusty with the process.

csakiistvan’s picture

csakiistvan
Primary language Hungarian
Location Budapest, Hungary
commented

thank you for taking care of it @loopduplicate :)

Status: Fixed » Closed (fixed)

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