Problem/Motivation

This was added in #2918570: Drupal\KernelTests\Core\Image\ToolkitGdTest fails on PHP 7.1.x-dev and 7.0.x-dev following a PHP bugfix and never resolved. An attempt in #2921123: Adjust Rectangle class to calculate rotated image dimensions according to libgd 2.2.2+ stalled.

    $effect_id = $style->addImageEffect($effect);
    $style->save();
    // @todo Uncomment this once
    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
    // $this->assertEqual($this->getImageTag($variables), '<img src="' . $url . '" width="41" height="41" alt="" class="image-style-test" />');
    $this->assertFalse(file_exists($generated_uri), 'Generated file does not exist.');
    $this->drupalGet($this->getAbsoluteUrl($url));
    $this->assertResponse(200, 'Image was generated at the URL.');
    $this->assertTrue(file_exists($generated_uri), 'Generated file does exist after we accessed it.');
    $image_file = $image_factory->get($generated_uri);
    // @todo Uncomment this once
    //   https://www.drupal.org/project/drupal/issues/2670966 is resolved.
    // $this->assertEqual($image_file->getWidth(), 41);
    // $this->assertEqual($image_file->getHeight(), 41);

Steps to reproduce

Proposed resolution

Different versions of the GD library bundled or linked in PHP provide slightly different results for the image rotation function, so we can not do an exact match check here. We tolerate a max difference of two pixels.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 3514699-nr-bot.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3514699

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Issue summary: View changes
Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Seems straight forward and todo issue clearly has already been closed

quietone’s picture

Status: Reviewed & tested by the community » Needs work

Just a question in the MR to answer. I suspect that the link to the issue can be removed.

mondrake’s picture

Well that issue, albeit closed, gives a lot of detail about exactly why the change is needed, IMHO.

znerol made their first commit to this issue’s fork.

znerol’s picture

Status: Needs work » Needs review

I agree with #6, issue #2921123: Adjust Rectangle class to calculate rotated image dimensions according to libgd 2.2.2+ contains valuable research leading to the fix. However, I think we can trust people to use git blame to actually find that research (via this issue). I removed the @see reference.

smustgrave’s picture

From the other random ones can a separate MR be opened that runs 1000 times

znerol’s picture

Repeat test MR is passing. However, there is a fail in EntityReferenceWidgetTest::testWidgetPreview(). Is that an instance of a known random test fail?

https://git.drupalcode.org/issue/drupal-3514699/-/pipelines/476020/test_...

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Don't see it on https://www.drupal.org/project/drupal/issues/2829040 maybe needs to be mentioned?

Feedback here appears to be addressed though.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

Title: [consistent test failure] ImageDimensionsTest::testImageDimensions() » Add a tolerance to width and height in ImageDimensionsTest::testImageDimensions()

A title with 'consistent test failure' doesn't seem right since this doesn't fail in the daily pipelines. Maybe this one is better?