ViewExecutableTest::testPropertyMethods() generates a random string that is then used to test that the ViewExecutable::getUrl() method returns a URL correctly.

When the randomly generated string begins with "%/", ends with "/%", or contains "/%/" then getUrl() treats this like a placeholder and replaces it with an asterisk. Although this seems like a random enough occurrence, I've encountered it twice in one day.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pbull’s picture

Assigned: pbull » Unassigned
Status: Active » Needs review
FileSize
634 bytes

Replace "%" with "*" to prevent randomly generated URLs from containing placeholders.

pbull’s picture

Title: ViewExecutableTest::testPropertyMethods fails when randomly generater URL contains placeholders » ViewExecutableTest::testPropertyMethods fails when randomly generated URL contains placeholders
dawehner’s picture

Priority: Minor » Critical
Issue tags: +Random test failure

Random test failures are critical.

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/src/Tests/ViewExecutableTest.php
@@ -321,6 +321,8 @@ public function testPropertyMethods() {
     $url = $this->randomString();
+    // Prevent URL placeholders from being added.
+    $url = preg_replace('/(\/|^)%(\/|$)/', '${1}*${2}', $url);

The randomness has no part of the test. We could just use a hard coded string

anavarre’s picture

Issue tags: +VDC
pbull’s picture

Status: Needs work » Needs review
FileSize
1.13 KB
1.24 KB

The attached patch hard-codes the paths and view arguments.

(Alternately, the randomness could be retained with URL-safe strings by using TestBase::randomMachineName() in the place of TestBase::randomString() but I don't think this is necessary.)

Status: Needs review » Needs work

The last submitted patch, 6: testPropertyMethods_random_url-2409339-6.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

Thank you for your work on that!

+++ b/core/modules/views/src/Tests/ViewExecutableTest.php
@@ -320,19 +320,19 @@ public function testPropertyMethods() {
     // Test the title methods.
-    $title = $this->randomString();
+    $title = 'FooBar';
     $view->setTitle($title);

Is the random string for the title a problem?

pbull’s picture

@dawehner no, randomness in the titles doesn't appear to be an issue. Updated patch attached.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Great!

This looks fine for me.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 312aa7c and pushed to 8.0.x. Thanks!

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes.

  • alexpott committed 312aa7c on 8.0.x
    Issue #2409339 by pbull: ViewExecutableTest::testPropertyMethods fails...

Status: Fixed » Closed (fixed)

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