Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Currently tests are failing due to the external test file (https://drupal.org/files/druplicon.large_.png) no longer existing. These should be updated so that they pass.
Comment | File | Size | Author |
---|---|---|---|
#10 | imagecache_external_test_fix-2896506-10.patch | 5.25 KB | ben.kyriakou |
#9 | imagecache_external_test_fix-2896506-9.patch | 4.82 KB | ben.kyriakou |
#2 | imagecache_external_test_fix_2-2896506.patch | 4.14 KB | ben.kyriakou |
#2 | imagecache_external_test_fix_1-2896506.patch | 912 bytes | ben.kyriakou |
Comments
Comment #2
ben.kyriakou CreditAttribution: ben.kyriakou at Cogapp commentedSince 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.
Comment #3
BarisW CreditAttribution: BarisW at LimoenGroen commentedLooks good to me, thanks for this! I'll go for the complicated one.
Comment #5
BarisW CreditAttribution: BarisW at LimoenGroen commentedComment #6
BarisW CreditAttribution: BarisW at LimoenGroen commentedI'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
Comment #7
ben.kyriakou CreditAttribution: ben.kyriakou at Cogapp commentedIt 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.Comment #8
ben.kyriakou CreditAttribution: ben.kyriakou at Cogapp commentedIt 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.
Comment #9
ben.kyriakou CreditAttribution: ben.kyriakou at Cogapp commentedFinal 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.
Comment #10
ben.kyriakou CreditAttribution: ben.kyriakou at Cogapp commentedIt 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...Comment #11
BarisW CreditAttribution: BarisW at LimoenGroen commentedYou are a hero! Thanks so much.
Comment #13
BarisW CreditAttribution: BarisW at LimoenGroen commented