search

wp-cli / media-command

Imports files as attachments, regenerates thumbnails, or lists registered image sizes.
MIT License
44 stars 41 forks source link

Fix tests to account for current core behavior #169

Closed wojtekn closed 1 year ago

wojtekn commented 1 year ago

In this PR, I propose to fix the failing tests. It looks like the change was introduced in https://github.com/wp-cli/media-command/commit/3b25705b94dd8c798f5bcb58b65d897b857e6fe9 .

I think that at that time, there was a bug in the WP core that resulted in saving 150x150 thumbnails for 150x150 images. We modified tests in the commit shown above to ensure they check the current core behavior, even if it was considered buggy.

However, I'm unsure about the exact timing of those events, as tests for the older WP version were marked as skipped in Aug 2019 (https://github.com/wp-cli/media-command/pull/112), tests were modified for the newer WP version in 2021 (https://github.com/wp-cli/media-command/commit/3b25705b94dd8c798f5bcb58b65d897b857e6fe9), but the bug in the core was fixed in Sep 2019 (https://core.trac.wordpress.org/ticket/32437)?

danielbachhuber commented 1 year ago

Interestingly, this passes for me locally...

wojtekn commented 1 year ago

@danielbachhuber could you please check tests for the main as well? For me, tests for "Regenerate all images default behavior" scenario fail locally on main, and succeed with the fix from this branch applied. On our build server, it works for main, but not for this PR.

It seems it's something environment-related, but I couldn't find the root cause yet.

danielbachhuber commented 1 year ago

could you please check tests for the main as well? For me, tests for "Regenerate all images default behavior" scenario fail locally on main, and succeed with the fix from this branch applied. On our build server, it works for main, but not for this PR.

It seems it's something environment-related, but I couldn't find the root cause yet.

@wojtekn Yep, main is broken for me locally too. I agree that it's something environment-related, and am not sure what it might be.

danielbachhuber commented 1 year ago

It looks like the change was introduced in 3b25705 .

For reference, the associated PR is https://github.com/wp-cli/media-command/pull/140

danielbachhuber commented 1 year ago

@wojtekn I'm still flummoxed as to why white-150-square-150x150.jpg is created in the GitHub Actions environment, and isn't created locally 😞

@schlessera @swissspidy Any ideas on what might be going on?

I added a couple of debug scenarios with https://github.com/wp-cli/media-command/pull/173 to see if we can get more visibility.

danielbachhuber commented 1 year ago

@wojtekn Found it! 😁 https://core.trac.wordpress.org/ticket/57370

danielbachhuber commented 1 year ago

@wojtekn Closing this in favor of https://github.com/wp-cli/media-command/issues/174

Thanks again for your work on it!