I think this is probably due to a poor test, but when Testbot 953 was configured with 6.x-3.x, it confirmed successfully with a full D7 test, but fails 3 particular tests on d8:

* Drupal\file\Tests\DownloadTest->checkUrl(): Generated URL matches expected URL.
* Drupal\image\Tests\ImageStylesPathAndUrlTest->doImageStyleUrlAndPathTests(): When using non-clean URLS, the system path contains the script name.

I have disabled 953 for now; I'd love to have somebody sort out this in core.

Files: 
CommentFileSizeAuthor
#11 run-tests-2150623.patch685 bytesdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,328 pass(es).
[ View ]
rfay_screenshot_ 2013-12-05 at 9.12.54 AM.png57.32 KBrfay

Comments

rfay’s picture

I tested both of these manually on both a vagrant testbot and on 953. Neither failed.

rfay’s picture

I just arranged a D8 test by 953 (which is enabled again, for now). It should probably be disabled if these same fails come back.

rfay’s picture

So this the DownloadTest failure is easily demonstrated using run-tests.sh...

sudo -u www-data /usr/bin/php ./core/scripts/run-tests.sh --concurrency 8 --php /usr/bin/php --url http://drupaltestbot953/checkout/ --verbose --class 'Drupal\file\Tests\DownloadTest'

I added an assertTrue() for debugging right before the failed test to find out what $url and $expected_url are, and they are different. But it wouldn't apparently have anything to do with the testbot.

The code in question is line 143 of https://api.drupal.org/api/drupal/core%21modules%21file%21lib%21Drupal%2...

Pass      Other      DownloadTest.php   143 Drupal\file\Tests\DownloadTest->che
    Value 'Generated URL =
    http://drupaltestbot953/checkout/system/files/%20-._%21%24%27%22%28%29%2A%40%5B%5D%3F%26%2B%25%23%2C%3B%3D%3A__%2523%2525%2526%252B%252F%253F%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E,
    Expected URL =
    http://drupaltestbot953/checkout/index.php/system/files/%20-._%21%24%27%22%28%29%2A%40%5B%5D%3F%26%2B%25%23%2C%3B%3D%3A__%2523%2525%2526%252B%252F%253F%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E'
rfay’s picture

drush si's default for clean_urls is ON. I think the default when installing via the traditional testbot was OFF.

rfay’s picture

Based on testing by pwolanin and others, this appears to be a core bug. Manually running run-tests.sh as described above with drupal in a subdirectory *breaks*. Running it without the subdir succeeds.

I can't say why the old-style 6.x-2.x testbot does not demonstrate this.

rfay’s picture

Title:6.x-3.x PIFR: 3 Failures in D8 test» Subdirectory + Clean URLs failure: 6.x-3.x PIFR: 3 Failures in D8 test
Project:Drupal.org Testbots» Drupal core
Version:» 8.x-dev
Component:malfunctioning testbot» file system
Priority:Normal» Major

I put this as major because it's blocking an important testbot improvement. Change if you will.

pwolanin’s picture

I'm guessing this is a bug with the way $GLOBALS['base_url'] is constructed and hence used in file_create_url().

I frankly don't even see at first blush how the test ever *can* work since the expected URL is generated with and without a clean URL flag, but I don't think the code in file_create_url() has any way to know about that flag.

rfay’s picture

Issue tags:+Random test failure

Tagging random test failure. However, it's not random; This is easy to demonstrate using the run-tests.sh approach on a system where drupal is in a subdirectory.

It appears random only because we have a few testbots (running PIFR 6.x-3.x) that detect it, and many which do not (for no known reason)

pwolanin’s picture

So far, I'm not able to validate that - it looks like both $GLOBALS['base_url'] and $file->getFileUri() are the smae in the top and subdir scenarios

pwolanin’s picture

Ok, so to reconfirm:

$url = file_create_url($file->getFileUri());

When the test is run from a top-level URL includes the index.php

When run in a subdirectory, it does not.

In class PrivateStream I see a call to url():

  /**
   * Implements Drupal\Core\StreamWrapper\StreamWrapperInterface::getExternalUrl().
   *
   * @return string
   *   Returns the HTML URI of a private file.
   */
  function getExternalUrl() {
    $path = str_replace('\\', '/', $this->getTarget());
    return url('system/files/' . $path, array('absolute' => TRUE));
  }

url() makes use of the request object, which the test set up in function prepareRequestForGenerator() and sets into the generator.

That function has a @todo and some code for handling the cli test runner, so I'm guessing the bug is there somewho.

dawehner’s picture

Status:Active» Needs review
StatusFileSize
new685 bytes
PASSED: [[SimpleTest]]: [MySQL] 59,328 pass(es).
[ View ]

We figured out that /checkout/ does not work while /checkout does work.

The code responsible for generating the base url takes the $_SERVER['SERVER_FILENAME']
which is set by run-tests.sh:

    $path = isset($parsed_url['path']) ? rtrim($parsed_url['path']) : '';
  $_SERVER['SCRIPT_FILENAME'] = $path .'/index.php';

so you will end up with two right slashes. Trim the path, if available fixes the problem.

pwolanin’s picture

Status:Needs review» Reviewed & tested by the community

Fixes the test behavior for me locally

rfay’s picture

Thanks so much to both of you. The fix is also now in pifr 6.x-3.x, so it generates a run-tests.sh url with no trailing /.

webchick’s picture

Status:Reviewed & tested by the community» Fixed

Great sleuthing work, you two! :D

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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