Problem/Motivation

UrlGeneratorTrait::setUrlGenerator sets linkGenerator property instead of urlGenerator property.
Looks like a copy-paste error.

Proposed resolution

Fix

Remaining tasks

Review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#12 url-generator-fooey-2315555-12_fail.patch1.23 KBundertext
#12 url-generator-fooey-2315555-12.patch1.92 KBundertext
#9 url-generator-fooey-2315555-9.patch1.92 KBundertext
#3 url-generator-fooey.pass_.patch1.58 KBlarowlan
#3 url-generator-fooey.fail_.patch1.07 KBlarowlan
url-generator-fooey.1.patch521 byteslarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

Yups that's write. Thanks for the fix. Do we need phpunit for this?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Quickfix

Good catch!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
FileSize
url-generator-fooey.fail_.patch1.07 KB
url-generator-fooey.pass_.patch1.58 KB

looking for red/green

oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
CreditAttribution: oriol_e9g commented
Status: Needs review » Needs work

The test should fail :(

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs work » Needs review
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

RTBC if green

The last submitted patch, 3: url-generator-fooey.fail_.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/lib/Drupal/Core/Routing/UrlGeneratorTrait.php
@@ -62,7 +62,7 @@ protected function getUrlGenerator() {
    */
   public function setUrlGenerator(UrlGeneratorInterface $generator) {
-    $this->linkGenerator = $generator;
+    $this->urlGenerator = $generator;
 
     return $this;
   }

    If we write a test we should add a proper dedicated test for the trait (note there is a phpunit helper method to create a mock from a trait. 

  2. +++ b/core/tests/Drupal/Tests/Core/Controller/ControllerBaseTest.php
@@ -58,4 +58,22 @@ public function testGetConfig() {
+    $url_generator = $this->getMockBuilder('\Drupal\Core\Routing\UrlGeneratorInterface')
+      ->disableOriginalConstructor()
+      ->getMock();
+

    you can just use getMock()

undertext’s picture

undertext CreditAttribution: undertext commented
Status: Needs work » Needs review
FileSize
url-generator-fooey-2315555-9.patch1.92 KB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

Thank you for updating the tests. Can you upload the test only patch?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
  1. +++ b/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTraitTest.php
@@ -0,0 +1,34 @@
+ * Contains Drupal\Tests\Core\Routing\UrlGeneratorTraitTest.
+ */
+
+namespace Drupal\Tests\Core\Routing;
+
+use Drupal\Tests\UnitTestCase;

    cool! 

  2. +++ b/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTraitTest.php
@@ -0,0 +1,34 @@
+ *
+ * @group Routing
+ */
+class UrlGeneratorTraitTest extends UnitTestCase {
+
+  /**
+   * Tests the urlGenerator method.
+   */
+  public function testGetUrlGenerator() {

    Let's use @coversDefaultClass and @covers 

  3. +++ b/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTraitTest.php
@@ -0,0 +1,34 @@
+    $url_generator = $this->getMockBuilder('\Drupal\Core\Routing\UrlGeneratorInterface')->getMock();

    you can use getMock('Drupal\Core\Routing\UrlGeneratorInterface'); directly.

undertext’s picture

undertext CreditAttribution: undertext commented
FileSize
url-generator-fooey-2315555-12.patch1.92 KB
url-generator-fooey-2315555-12_fail.patch1.23 KB

New patch and test-only patch

Status: Needs review » Needs work

The last submitted patch, 12: url-generator-fooey-2315555-12_fail.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs work » Reviewed & tested by the community

Yay!! thank you for the test only patch.

alexpott’s picture

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

Committed 8e6b32a and pushed to 8.0.x. Thanks!

diff --git a/core/lib/Drupal/Core/Routing/UrlGeneratorTrait.php b/core/lib/Drupal/Core/Routing/UrlGeneratorTrait.php
index ec0ba86..06d0445 100644
--- a/core/lib/Drupal/Core/Routing/UrlGeneratorTrait.php
+++ b/core/lib/Drupal/Core/Routing/UrlGeneratorTrait.php
@@ -1,15 +1,12 @@
 <?php
 
 /**
- * @file Contains Drupal\Core\Routing\UrlGeneratorTrait.
+ * @file
+ * Contains Drupal\Core\Routing\UrlGeneratorTrait.
  */
 
 namespace Drupal\Core\Routing;
 
-
-use Drupal\Core\Utility\LinkGeneratorInterface;
-use Drupal\Core\Routing\UrlGeneratorInterface;
-
 /**
  * Wrapper methods for the Url Generator.
  *

Minor fixes on commit.

  • alexpott committed 8e6b32a on 8.0.x 
    Issue #2315555 by undertext, larowlan: Fixed UrlGeneratorTrait::...
m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Issue tags: +#ams2014contest

Status: Fixed » Closed (fixed)

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