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.

CommentFileSizeAuthor
#11 run-tests-2150623.patch685 bytesdawehner
rfay_screenshot_ 2013-12-05 at 9.12.54 AM.png57.32 KBrfay
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
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

pwolanin CreditAttribution: pwolanin commented

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

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
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

pwolanin CreditAttribution: pwolanin commented

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

pwolanin CreditAttribution: pwolanin commented

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

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
FileSize
run-tests-2150623.patch685 bytes

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

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Reviewed & tested by the community

Fixes the test behavior for me locally

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

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

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
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.