In #2337191: Split up EntityManager into many services the entity manager was refactored and splitted into many services.
In the refactor you forgot to add the return statement in EntityManager::clearDisplayModeInfo(). For confirmation see
EntityDisplayRepository::clearDisplayModeInfo() and what is stated by the interface: 

  /**
   * Clears the gathered display mode info.
   *
   * @return $this
   */
  public function clearDisplayModeInfo();
CommentFileSizeAuthor
#20 interdiff-18-20.txt2.04 KBkishor_kolekar
#20 missing_return-2605904-20.patch1.82 KBkishor_kolekar
#18 interdiff_12-18.txt1.62 KBswatichouhan012
#18 2605904-18.patch1.83 KBswatichouhan012
#12 missing_return-2605904-12.patch1.49 KBjmikii
#12 missing_return-2605904-12-test-only.patch657 bytesjmikii
#11 missing_return-2605904-11.patch864 bytesvacho
#2 missing_return-2605904-2.patch638 byteswillzyx

Comments

willzyx created an issue. See original summary.

willzyx’s picture

willzyx commented
StatusFileSize
new missing_return-2605904-2.patch638 bytes
willzyx’s picture

willzyx commented
Version: 8.1.x-dev » 8.0.x-dev

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should 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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should 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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should 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.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work
Issue tags: +Needs tests, +Novice

Good catch, now that we have a legacy test for this method, we could assert quite easily that it returns itself again in \Drupal\Tests\Core\Entity\EntityManagerTest::testClearDisplayModeInfo.

vacho’s picture

vacho commented
Version: 8.6.x-dev » 8.8.x-dev
StatusFileSize
new missing_return-2605904-11.patch864 bytes

Only updating this patch. (rerolled)

jmikii’s picture

jmikii commented
Status: Needs work » Needs review
Issue tags: -Needs tests +DevDaysCluj
StatusFileSize
new missing_return-2605904-12-test-only.patch657 bytes
new missing_return-2605904-12.patch1.49 KB

Modified the test to check the return output

The last submitted patch, 12: missing_return-2605904-12-test-only.patch, failed testing. View results

rosinegrean’s picture

rosinegrean commented
Issue tags: -DevDaysCluj +DevDaysTransylvania

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.

joachim’s picture

joachim commented
Status: Needs review » Reviewed & tested by the community
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/Entity/EntityManager.php
@@ -593,7 +593,7 @@ public function getFormModeOptionsByBundle($entity_type_id, $bundle) {
   public function clearDisplayModeInfo() {
     @trigger_error('EntityManagerInterface::clearDisplayModeInfo() is deprecated in Drupal 8.0.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityDisplayRepositoryInterface::clearDisplayModeInfo() instead. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
-    $this->container->get('entity_display.repository')->clearDisplayModeInfo();
+    return $this->container->get('entity_display.repository')->clearDisplayModeInfo();
   }

    This method needs to return an instance of EntityManager. This would return that entity display repository service. 

  2. +++ b/core/tests/Drupal/Tests/Core/Entity/EntityManagerTest.php
@@ -454,7 +454,7 @@ public function testGetFormModeOptionsByBundle() {
     $this->entityDisplayRepository->clearDisplayModeInfo()->shouldBeCalled()->willReturn([]);

    This should return $this->entityDisplayRepository. 

  3. +++ b/core/tests/Drupal/Tests/Core/Entity/EntityManagerTest.php
@@ -454,7 +454,7 @@ public function testGetFormModeOptionsByBundle() {
+    $this->assertEquals([], $this->entityManager->clearDisplayModeInfo());

    We should assert that it equals $this->entityManager.

swatichouhan012’s picture

swatichouhan012 commented
Status: Needs work » Needs review
StatusFileSize
new 2605904-18.patch1.83 KB
new interdiff_12-18.txt1.62 KB

Hi, i have updated patch according comment #17, kindly review.

berdir’s picture

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

You need to change the willReturn() call above to $this->entityManager, and now you don't call the method anymore inside EntityManager.

kishor_kolekar’s picture

kishor_kolekar commented
Status: Needs work » Needs review
StatusFileSize
new missing_return-2605904-20.patch1.82 KB
new interdiff-18-20.txt2.04 KB

kindly review new patch.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Yeah, that's better. The willReturn() is correct although it technically doesn't matter anymore as we ignore that return.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 96a7c8f714 to 8.9.x and c9d2c9bbec to 8.8.x. Thanks!

I guess it is important to have EntityManager work the same as the EntityDisplayRepositoty...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.9.x-dev » 8.8.x-dev

  • alexpott committed 96a7c8f on 8.9.x 
    Issue #2605904 by jmikii, swatichouhan012, kishor_kolekar, willzyx,...

  • alexpott committed 96e5ba8 on 8.8.x 
    Issue #2605904 by jmikii, swatichouhan012, kishor_kolekar, willzyx,...

Status: Fixed » Closed (fixed)

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