Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ben.kyriakou created an issue. See original summary.

ben.kyriakou’s picture

Status: Active » Needs review
FileSize
912 bytes
4.14 KB

Since there are a couple of ways this can be fixed, I give you not one but two patches.

The first patch does the simple thing, and replaces the URL with one that exists (https://www.drupal.org/files/drupal_logo-blue.png), along with updated image sizes for the image style test.

The second patch is more complicated, and alters the test to use one of the images generated by Simpletest. I prefer this approach since it's less likely to break due to external factors.

BarisW’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, thanks for this! I'll go for the complicated one.

  • BarisW committed 4107703 on 7.x-2.x authored by ben.kyriakou
    Issue #2896506 by ben.kyriakou, BarisW: Fix test failures due to missing...
BarisW’s picture

Status: Reviewed & tested by the community » Fixed
BarisW’s picture

I've re-enabled tests on the project after committing this. However, the tests still fail, not sure if it related: https://www.drupal.org/pift-ci-job/722409

ben.kyriakou’s picture

Status: Fixed » Needs review
FileSize
4.29 KB

It looks like this is due to issues with how imagecache_external_validate_host escapes domains for regex checking (which may also lead to a possible security issue where domains can be unintentionally matched when the period is interpreted as a wildcard character). Attached is a patch that fixes this and adds tests, plus makes a change to my existing test to ensure that the image style is applied.

ben.kyriakou’s picture

It looks like the test environment doesn't have that image available for some reason. Let's try a more agnostic method of finding an image.

ben.kyriakou’s picture

Final roll of this patch. If this doesn't pass I suspect there's something else going on here that's specific to the test environment.

ben.kyriakou’s picture

It looks like the way I was extracting the host from the $base_url wasn't playing nicely with the subdirectory on the test environment. This should do it...

BarisW’s picture

Status: Needs review » Reviewed & tested by the community

You are a hero! Thanks so much.

  • BarisW committed a5c3a5f on 7.x-2.x authored by ben.kyriakou
    Issue #2896506 by ben.kyriakou, BarisW: Fix test failures due to missing...
BarisW’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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